Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1460047 > unrolled thread

[PATCH 0/5] Reduce memory waste by page extension user

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] Reduce memory waste by page extension user js1304@gmail.com - 2016-08-11 00:20 +0200
    [PATCH 2/5] mm/debug_pagealloc: don't allocate page_ext if we don't use guard page js1304@gmail.com - 2016-08-11 00:50 +0200
      Re: [PATCH 2/5] mm/debug_pagealloc: don't allocate page_ext if we  don't use guard page Vlastimil Babka <vbabka@suse.cz> - 2016-08-11 12:00 +0200
    Re: [PATCH 1/5] mm/debug_pagealloc: clean-up guard page handling code Vlastimil Babka <vbabka@suse.cz> - 2016-08-11 11:40 +0200
    Re: [PATCH 1/5] mm/debug_pagealloc: clean-up guard page handling code Vlastimil Babka <vbabka@suse.cz> - 2016-08-11 11:50 +0200
      Re: [PATCH 1/5] mm/debug_pagealloc: clean-up guard page handling code Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-08-12 14:30 +0200
        Re: [PATCH 1/5] mm/debug_pagealloc: clean-up guard page handling code Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-08-16 05:00 +0200
    Re: [PATCH 3/5] mm/page_owner: move page_owner specific function to  page_owner.c Vlastimil Babka <vbabka@suse.cz> - 2016-08-11 14:40 +0200
    Re: [PATCH 4/5] mm/page_ext: support extra space allocation by  page_ext user Vlastimil Babka <vbabka@suse.cz> - 2016-08-11 15:00 +0200
    Re: [PATCH 5/5] mm/page_owner: don't define fields on struct page_ext  by hard-coding Vlastimil Babka <vbabka@suse.cz> - 2016-08-11 15:10 +0200

#1460047 — [PATCH 0/5] Reduce memory waste by page extension user

From: Joonsoo Kim <iamjoonsoo.kim@lge.com>

This patchset tries to reduce memory waste by page extension user.

First case is architecture supported debug_pagealloc. It doesn't
requires additional memory if guard page isn't used. 8 bytes per
page will be saved in this case.

Second case is related to page owner feature. Until now, if page_ext
users want to use it's own fields on page_ext, fields should be
defined in struct page_ext by hard-coding. It has a following problem.

struct page_ext {
 #ifdef CONFIG_A
	int a;
 #endif
 #ifdef CONFIG_B
	int b;
 #endif
};

Assume that kernel is built with both CONFIG_A and CONFIG_B.
Even if we enable feature A and doesn't enable feature B at runtime,
each entry of struct page_ext takes two int rather than one int.
It's undesirable waste so this patch tries to reduce it. By this patchset,
we can save 20 bytes per page dedicated for page owner feature
in some configurations.

Thanks.

Joonsoo Kim (5):
  mm/debug_pagealloc: clean-up guard page handling code
  mm/debug_pagealloc: don't allocate page_ext if we don't use guard page
  mm/page_owner: move page_owner specific function to page_owner.c
  mm/page_ext: support extra space allocation by page_ext user
  mm/page_owner: don't define fields on struct page_ext by hard-coding

 include/linux/page_ext.h   |   8 +--
 include/linux/page_owner.h |   2 +
 mm/page_alloc.c            |  44 +++++++------
 mm/page_ext.c              |  41 +++++++++---
 mm/page_owner.c            | 152 ++++++++++++++++++++++++++++++++++++++-------
 mm/vmstat.c                |  79 -----------------------
 6 files changed, 190 insertions(+), 136 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1460073 — [PATCH 2/5] mm/debug_pagealloc: don't allocate page_ext if we don't use guard page

From: Joonsoo Kim <iamjoonsoo.kim@lge.com>

What debug_pagealloc does is just mapping/unmapping page table.
Basically, it doesn't need additional memory space to memorize something.
But, with guard page feature, it requires additional memory to distinguish
if the page is for guard or not. Guard page is only used when
debug_guardpage_minorder is non-zero so this patch removes additional
memory allocation (page_ext) if debug_guardpage_minorder is zero.

It saves memory if we just use debug_pagealloc and not guard page.

Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
 mm/page_alloc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5e7944b..45cb021 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -608,6 +608,9 @@ static bool need_debug_guardpage(void)
 	if (!debug_pagealloc_enabled())
 		return false;
 
+	if (!debug_guardpage_minorder())
+		return false;
+
 	return true;
 }
 
@@ -616,6 +619,9 @@ static void init_debug_guardpage(void)
 	if (!debug_pagealloc_enabled())
 		return;
 
+	if (!debug_guardpage_minorder())
+		return;
+
 	_debug_guardpage_enabled = true;
 }
 
@@ -636,7 +642,7 @@ static int __init debug_guardpage_minorder_setup(char *buf)
 	pr_info("Setting debug_guardpage_minorder to %lu\n", res);
 	return 0;
 }
-__setup("debug_guardpage_minorder=", debug_guardpage_minorder_setup);
+early_param("debug_guardpage_minorder", debug_guardpage_minorder_setup);
 
 static inline bool set_page_guard(struct zone *zone, struct page *page,
 				unsigned int order, int migratetype)
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1460345 — Re: [PATCH 2/5] mm/debug_pagealloc: don't allocate page_ext if we don't use guard page

On 08/10/2016 08:16 AM, js1304@gmail.com wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> What debug_pagealloc does is just mapping/unmapping page table.
> Basically, it doesn't need additional memory space to memorize something.
> But, with guard page feature, it requires additional memory to distinguish
> if the page is for guard or not. Guard page is only used when
> debug_guardpage_minorder is non-zero so this patch removes additional
> memory allocation (page_ext) if debug_guardpage_minorder is zero.
>
> It saves memory if we just use debug_pagealloc and not guard page.

We could also save cycles with a static key for _debug_guardpage_enabled :)

But memory savings are likely more significant, so

> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

[toc] | [prev] | [next] | [standalone]

#1460324 — Re: [PATCH 1/5] mm/debug_pagealloc: clean-up guard page handling code

On 08/10/2016 08:16 AM, js1304@gmail.com wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> We can make code clean by moving decision condition
> for set_page_guard() into set_page_guard() itself. It will
> help code readability. There is no functional change.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

[toc] | [prev] | [next] | [standalone]

#1460344 — Re: [PATCH 1/5] mm/debug_pagealloc: clean-up guard page handling code

On 08/10/2016 10:14 AM, Sergey Senozhatsky wrote:
>> @@ -1650,18 +1655,15 @@ static inline void expand(struct zone *zone, struct page *page,
>>  		size >>= 1;
>>  		VM_BUG_ON_PAGE(bad_range(zone, &page[size]), &page[size]);
>>
>> -		if (IS_ENABLED(CONFIG_DEBUG_PAGEALLOC) &&
>> -			debug_guardpage_enabled() &&
>> -			high < debug_guardpage_minorder()) {
>> -			/*
>> -			 * Mark as guard pages (or page), that will allow to
>> -			 * merge back to allocator when buddy will be freed.
>> -			 * Corresponding page table entries will not be touched,
>> -			 * pages will stay not present in virtual address space
>> -			 */
>> -			set_page_guard(zone, &page[size], high, migratetype);
>> +		/*
>> +		 * Mark as guard pages (or page), that will allow to
>> +		 * merge back to allocator when buddy will be freed.
>> +		 * Corresponding page table entries will not be touched,
>> +		 * pages will stay not present in virtual address space
>> +		 */
>> +		if (set_page_guard(zone, &page[size], high, migratetype))
>>  			continue;
>> -		}
>
> so previously IS_ENABLED(CONFIG_DEBUG_PAGEALLOC) could have optimized out
> the entire branch -- no set_page_guard() invocation and checks, right? but
> now we would call set_page_guard() every time?

No, there's a !CONFIG_DEBUG_PAGEALLOC version of set_page_guard() that 
returns false (static inline), so this whole if will be eliminated by 
the compiler, same as before.

[toc] | [prev] | [next] | [standalone]

#1461141 — Re: [PATCH 1/5] mm/debug_pagealloc: clean-up guard page handling code

On (08/11/16 11:41), Vlastimil Babka wrote:
> On 08/10/2016 10:14 AM, Sergey Senozhatsky wrote:
> > > @@ -1650,18 +1655,15 @@ static inline void expand(struct zone *zone, struct page *page,
> > >  		size >>= 1;
> > >  		VM_BUG_ON_PAGE(bad_range(zone, &page[size]), &page[size]);
> > > 
> > > -		if (IS_ENABLED(CONFIG_DEBUG_PAGEALLOC) &&
> > > -			debug_guardpage_enabled() &&
> > > -			high < debug_guardpage_minorder()) {
> > > -			/*
> > > -			 * Mark as guard pages (or page), that will allow to
> > > -			 * merge back to allocator when buddy will be freed.
> > > -			 * Corresponding page table entries will not be touched,
> > > -			 * pages will stay not present in virtual address space
> > > -			 */
> > > -			set_page_guard(zone, &page[size], high, migratetype);
> > > +		/*
> > > +		 * Mark as guard pages (or page), that will allow to
> > > +		 * merge back to allocator when buddy will be freed.
> > > +		 * Corresponding page table entries will not be touched,
> > > +		 * pages will stay not present in virtual address space
> > > +		 */
> > > +		if (set_page_guard(zone, &page[size], high, migratetype))
> > >  			continue;
> > > -		}
> > 
> > so previously IS_ENABLED(CONFIG_DEBUG_PAGEALLOC) could have optimized out
> > the entire branch -- no set_page_guard() invocation and checks, right? but
> > now we would call set_page_guard() every time?
> 
> No, there's a !CONFIG_DEBUG_PAGEALLOC version of set_page_guard() that
> returns false (static inline), so this whole if will be eliminated by the
> compiler, same as before.

ah, indeed. didn't notice it.

	-ss

[toc] | [prev] | [next] | [standalone]

#1463361 — Re: [PATCH 1/5] mm/debug_pagealloc: clean-up guard page handling code

On Fri, Aug 12, 2016 at 09:25:37PM +0900, Sergey Senozhatsky wrote:
> On (08/11/16 11:41), Vlastimil Babka wrote:
> > On 08/10/2016 10:14 AM, Sergey Senozhatsky wrote:
> > > > @@ -1650,18 +1655,15 @@ static inline void expand(struct zone *zone, struct page *page,
> > > >  		size >>= 1;
> > > >  		VM_BUG_ON_PAGE(bad_range(zone, &page[size]), &page[size]);
> > > > 
> > > > -		if (IS_ENABLED(CONFIG_DEBUG_PAGEALLOC) &&
> > > > -			debug_guardpage_enabled() &&
> > > > -			high < debug_guardpage_minorder()) {
> > > > -			/*
> > > > -			 * Mark as guard pages (or page), that will allow to
> > > > -			 * merge back to allocator when buddy will be freed.
> > > > -			 * Corresponding page table entries will not be touched,
> > > > -			 * pages will stay not present in virtual address space
> > > > -			 */
> > > > -			set_page_guard(zone, &page[size], high, migratetype);
> > > > +		/*
> > > > +		 * Mark as guard pages (or page), that will allow to
> > > > +		 * merge back to allocator when buddy will be freed.
> > > > +		 * Corresponding page table entries will not be touched,
> > > > +		 * pages will stay not present in virtual address space
> > > > +		 */
> > > > +		if (set_page_guard(zone, &page[size], high, migratetype))
> > > >  			continue;
> > > > -		}
> > > 
> > > so previously IS_ENABLED(CONFIG_DEBUG_PAGEALLOC) could have optimized out
> > > the entire branch -- no set_page_guard() invocation and checks, right? but
> > > now we would call set_page_guard() every time?
> > 
> > No, there's a !CONFIG_DEBUG_PAGEALLOC version of set_page_guard() that
> > returns false (static inline), so this whole if will be eliminated by the
> > compiler, same as before.
> 
> ah, indeed. didn't notice it.

Hello, Sergey and Vlastimil.

I fixed all you commented and sent v2.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1460469 — Re: [PATCH 3/5] mm/page_owner: move page_owner specific function to page_owner.c

On 08/10/2016 08:16 AM, js1304@gmail.com wrote:
> +			page_mt = gfpflags_to_migratetype(page_ext->gfp_mask);
> +			if (pageblock_mt != page_mt) {
> +				count[pageblock_mt]++;
> +
> +				pfn = block_end_pfn;
> +				break;
> +			}

... is not the same as ...

> -			page_mt = gfpflags_to_migratetype(page_ext->gfp_mask);
> -			if (pageblock_mt != page_mt) {
> -				if (is_migrate_cma(pageblock_mt))
> -					count[MIGRATE_MOVABLE]++;
> -				else
> -					count[pageblock_mt]++;
> -
> -				pfn = block_end_pfn;
> -				break;
> -			}

Rebasing blunder?

Vlastimil

[toc] | [prev] | [next] | [standalone]

#1460493 — Re: [PATCH 4/5] mm/page_ext: support extra space allocation by page_ext user

On 08/10/2016 08:16 AM, js1304@gmail.com wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> Until now, if some page_ext users want to use it's own field on page_ext,
> it should be defined in struct page_ext by hard-coding. It has a problem
> that wastes memory in following situation.
>
> struct page_ext {
>  #ifdef CONFIG_A
> 	int a;
>  #endif
>  #ifdef CONFIG_B
> 	int b;
>  #endif
> };
>
> Assume that kernel is built with both CONFIG_A and CONFIG_B.
> Even if we enable feature A and doesn't enable feature B at runtime,
> each entry of struct page_ext takes two int rather than one int.
> It's undesirable result so this patch tries to fix it.
>
> To solve above problem, this patch implements to support extra space
> allocation at runtime. When need() callback returns true, it's extra
> memory requirement is summed to entry size of page_ext. Also, offset
> for each user's extra memory space is returned. With this offset,
> user can use this extra space and there is no need to define needed
> field on page_ext by hard-coding.
>
> This patch only implements an infrastructure. Following patch will use it
> for page_owner which is only user having it's own fields on page_ext.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Fine, but...

>
>  static void __init invoke_init_callbacks(void)
> @@ -91,6 +102,16 @@ static void __init invoke_init_callbacks(void)
>  	}
>  }
>
> +static unsigned long get_entry_size(void)
> +{
> +	return sizeof(struct page_ext) + extra_mem;
> +}
> +
> +static inline struct page_ext *get_entry_base(void *base, unsigned long offset)
> +{
> +	return base + get_entry_size() * offset;
> +}

Why _base()? Why not just get_entry?
Also I find it confusing that the word offset here is different than the 
offset in page_ext_operations. Maybe use "index" instead?

Vlastimil

[toc] | [prev] | [next] | [standalone]

#1460505 — Re: [PATCH 5/5] mm/page_owner: don't define fields on struct page_ext by hard-coding

On 08/10/2016 08:16 AM, js1304@gmail.com wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> There is a memory waste problem if we define field on struct page_ext
> by hard-coding. Entry size of struct page_ext includes the size of
> those fields even if it is disabled at runtime. Now, extra memory request
> at runtime is possible so page_owner don't need to define it's own fields
> by hard-coding.
>
> This patch removes hard-coded define and uses extra memory for storing
> page_owner information in page_owner. Most of code are just mechanical
> changes.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web