* [PATCH 0/1] mm/page_ext: remove unnecessary stack_trace field @ 2015-01-26 21:52 Sergei Rogachev 2015-01-26 21:52 ` [PATCH 1/1] " Sergei Rogachev 0 siblings, 1 reply; 5+ messages in thread From: Sergei Rogachev @ 2015-01-26 21:52 UTC (permalink / raw) To: akpm, iamjoonsoo.kim; +Cc: LKML, Sergei Rogachev The functionality named "page owner" has resurrected the structure page_ext which is used for keeping additional page-related information. Every page in the system has its own corresponding page_ext structure. Page owner uses this structure to keep allocation order, gfp_mask and backtrace entries; moreover page_ext includes a field of type "struct stack_trace". And this field is used for collecting backtraces: a pointer to struct stack_trace is passed to save_stack_trace in the function __set_page_owner. But it is really not necessary to keep dedicated stack_trace structures for all physical pages in the system, only an amount of saved backtrace entries is needed to be saved. The stack_trace structure can be allocated on the stack (it is relatively small) and used for save_stack_trace invocation. Such change can save 12 bytes of memory per page if page owner functionality is enabled. Sergei Rogachev (1): mm/page_ext: remove unnecessary stack_trace field include/linux/page_ext.h | 2 +- mm/page_owner.c | 21 ++++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] mm/page_ext: remove unnecessary stack_trace field 2015-01-26 21:52 [PATCH 0/1] mm/page_ext: remove unnecessary stack_trace field Sergei Rogachev @ 2015-01-26 21:52 ` Sergei Rogachev 2015-01-27 8:36 ` Joonsoo Kim 2015-01-27 22:43 ` Andrew Morton 0 siblings, 2 replies; 5+ messages in thread From: Sergei Rogachev @ 2015-01-26 21:52 UTC (permalink / raw) To: akpm, iamjoonsoo.kim; +Cc: LKML, Sergei Rogachev Page owner uses the page_ext structure to keep meta-information for every page in the system. The structure also contains a field of type 'struct stack_trace', page owner uses this field during invocation of the function save_stack_trace. It is easy to notice that keeping a copy of this structure for every page in the system is very inefficiently in terms of memory. The patch removes this unnecessary field of page_ext and forces page owner to use a stack_trace structure allocated on the stack. Signed-off-by: Sergei Rogachev <rogachevsergei@gmail.com> --- include/linux/page_ext.h | 2 +- mm/page_owner.c | 21 ++++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h index d2a2c84..c42981c 100644 --- a/include/linux/page_ext.h +++ b/include/linux/page_ext.h @@ -40,7 +40,7 @@ struct page_ext { #ifdef CONFIG_PAGE_OWNER unsigned int order; gfp_t gfp_mask; - struct stack_trace trace; + unsigned int nr_entries; unsigned long trace_entries[8]; #endif }; diff --git a/mm/page_owner.c b/mm/page_owner.c index 9ab4a9b..f7f3318 100644 --- a/mm/page_owner.c +++ b/mm/page_owner.c @@ -60,19 +60,19 @@ void __reset_page_owner(struct page *page, unsigned int order) void __set_page_owner(struct page *page, unsigned int order, gfp_t gfp_mask) { struct page_ext *page_ext; - struct stack_trace *trace; + struct stack_trace trace; page_ext = lookup_page_ext(page); - trace = &page_ext->trace; - trace->nr_entries = 0; - trace->max_entries = ARRAY_SIZE(page_ext->trace_entries); - trace->entries = &page_ext->trace_entries[0]; - trace->skip = 3; - save_stack_trace(&page_ext->trace); + trace.nr_entries = 0; + trace.max_entries = ARRAY_SIZE(page_ext->trace_entries); + trace.entries = &page_ext->trace_entries[0]; + trace.skip = 3; + save_stack_trace(&trace); page_ext->order = order; page_ext->gfp_mask = gfp_mask; + page_ext->nr_entries = trace.nr_entries; __set_bit(PAGE_EXT_OWNER, &page_ext->flags); } @@ -83,6 +83,7 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn, { int ret; int pageblock_mt, page_mt; + struct stack_trace trace; char *kbuf; kbuf = kmalloc(count, GFP_KERNEL); @@ -121,8 +122,10 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn, if (ret >= count) goto err; - ret += snprint_stack_trace(kbuf + ret, count - ret, - &page_ext->trace, 0); + trace.nr_entries = page_ext->nr_entries; + trace.entries = &page_ext->trace_entries[0]; + + ret += snprint_stack_trace(kbuf + ret, count - ret, &trace, 0); if (ret >= count) goto err; -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] mm/page_ext: remove unnecessary stack_trace field 2015-01-26 21:52 ` [PATCH 1/1] " Sergei Rogachev @ 2015-01-27 8:36 ` Joonsoo Kim 2015-01-27 22:43 ` Andrew Morton 1 sibling, 0 replies; 5+ messages in thread From: Joonsoo Kim @ 2015-01-27 8:36 UTC (permalink / raw) To: Sergei Rogachev; +Cc: akpm, LKML On Tue, Jan 27, 2015 at 12:52:12AM +0300, Sergei Rogachev wrote: > Page owner uses the page_ext structure to keep meta-information > for every page in the system. The structure also contains a field > of type 'struct stack_trace', page owner uses this field during > invocation of the function save_stack_trace. It is easy to notice > that keeping a copy of this structure for every page in the system > is very inefficiently in terms of memory. > > The patch removes this unnecessary field of page_ext and forces > page owner to use a stack_trace structure allocated on the stack. > > Signed-off-by: Sergei Rogachev <rogachevsergei@gmail.com> > --- > include/linux/page_ext.h | 2 +- > mm/page_owner.c | 21 ++++++++++++--------- > 2 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h > index d2a2c84..c42981c 100644 > --- a/include/linux/page_ext.h > +++ b/include/linux/page_ext.h > @@ -40,7 +40,7 @@ struct page_ext { > #ifdef CONFIG_PAGE_OWNER > unsigned int order; > gfp_t gfp_mask; > - struct stack_trace trace; > + unsigned int nr_entries; > unsigned long trace_entries[8]; > #endif > }; > diff --git a/mm/page_owner.c b/mm/page_owner.c > index 9ab4a9b..f7f3318 100644 > --- a/mm/page_owner.c > +++ b/mm/page_owner.c > @@ -60,19 +60,19 @@ void __reset_page_owner(struct page *page, unsigned int order) > void __set_page_owner(struct page *page, unsigned int order, gfp_t gfp_mask) > { > struct page_ext *page_ext; > - struct stack_trace *trace; > + struct stack_trace trace; > > page_ext = lookup_page_ext(page); > > - trace = &page_ext->trace; > - trace->nr_entries = 0; > - trace->max_entries = ARRAY_SIZE(page_ext->trace_entries); > - trace->entries = &page_ext->trace_entries[0]; > - trace->skip = 3; > - save_stack_trace(&page_ext->trace); > + trace.nr_entries = 0; > + trace.max_entries = ARRAY_SIZE(page_ext->trace_entries); > + trace.entries = &page_ext->trace_entries[0]; > + trace.skip = 3; > + save_stack_trace(&trace); > > page_ext->order = order; > page_ext->gfp_mask = gfp_mask; > + page_ext->nr_entries = trace.nr_entries; > > __set_bit(PAGE_EXT_OWNER, &page_ext->flags); > } > @@ -83,6 +83,7 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn, > { > int ret; > int pageblock_mt, page_mt; > + struct stack_trace trace; > char *kbuf; > > kbuf = kmalloc(count, GFP_KERNEL); > @@ -121,8 +122,10 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn, > if (ret >= count) > goto err; > > - ret += snprint_stack_trace(kbuf + ret, count - ret, > - &page_ext->trace, 0); > + trace.nr_entries = page_ext->nr_entries; > + trace.entries = &page_ext->trace_entries[0]; > + > + ret += snprint_stack_trace(kbuf + ret, count - ret, &trace, 0); > if (ret >= count) > goto err; Acked-by: Joonsoo Kim <iamjoonsoo.kim@lge.com> Thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] mm/page_ext: remove unnecessary stack_trace field 2015-01-26 21:52 ` [PATCH 1/1] " Sergei Rogachev 2015-01-27 8:36 ` Joonsoo Kim @ 2015-01-27 22:43 ` Andrew Morton 2015-01-28 19:08 ` Sergei Rogachev 1 sibling, 1 reply; 5+ messages in thread From: Andrew Morton @ 2015-01-27 22:43 UTC (permalink / raw) To: Sergei Rogachev; +Cc: iamjoonsoo.kim, LKML On Tue, 27 Jan 2015 00:52:12 +0300 Sergei Rogachev <rogachevsergei@gmail.com> wrote: > Page owner uses the page_ext structure to keep meta-information > for every page in the system. The structure also contains a field > of type 'struct stack_trace', page owner uses this field during > invocation of the function save_stack_trace. It is easy to notice > that keeping a copy of this structure for every page in the system > is very inefficiently in terms of memory. > > The patch removes this unnecessary field of page_ext and forces > page owner to use a stack_trace structure allocated on the stack. > Fair enough. What do you think of the below tweaks? Seems a bit neater, and it will cause all unmentioned fields to be zeroed out. It's not a bug to leave .max_entries uninitialised in print_page_owner(), but it's nice not to leave loose ends. --- a/mm/page_owner.c~mm-page_ext-remove-unnecessary-stack_trace-field-fix +++ a/mm/page_owner.c @@ -59,15 +59,14 @@ void __reset_page_owner(struct page *pag void __set_page_owner(struct page *page, unsigned int order, gfp_t gfp_mask) { - struct page_ext *page_ext; - struct stack_trace trace; + struct page_ext *page_ext = lookup_page_ext(page); + struct stack_trace trace = { + .nr_entries = 0, + .max_entries = ARRAY_SIZE(page_ext->trace_entries), + .entries = &page_ext->trace_entries[0], + .skip = 3, + }; - page_ext = lookup_page_ext(page); - - trace.nr_entries = 0; - trace.max_entries = ARRAY_SIZE(page_ext->trace_entries); - trace.entries = &page_ext->trace_entries[0]; - trace.skip = 3; save_stack_trace(&trace); page_ext->order = order; @@ -83,8 +82,11 @@ print_page_owner(char __user *buf, size_ { int ret; int pageblock_mt, page_mt; - struct stack_trace trace; char *kbuf; + struct stack_trace trace = { + .nr_entries = page_ext->nr_entries, + .entries = &page_ext->trace_entries[0], + }; kbuf = kmalloc(count, GFP_KERNEL); if (!kbuf) @@ -122,9 +124,6 @@ print_page_owner(char __user *buf, size_ if (ret >= count) goto err; - trace.nr_entries = page_ext->nr_entries; - trace.entries = &page_ext->trace_entries[0]; - ret += snprint_stack_trace(kbuf + ret, count - ret, &trace, 0); if (ret >= count) goto err; _ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] mm/page_ext: remove unnecessary stack_trace field 2015-01-27 22:43 ` Andrew Morton @ 2015-01-28 19:08 ` Sergei Rogachev 0 siblings, 0 replies; 5+ messages in thread From: Sergei Rogachev @ 2015-01-28 19:08 UTC (permalink / raw) To: Andrew Morton; +Cc: iamjoonsoo.kim, LKML On Tue, 27 Jan 2015 14:43:19 -0800 Andrew Morton <akpm@linux-foundation.org> wrote: > On Tue, 27 Jan 2015 00:52:12 +0300 Sergei Rogachev <rogachevsergei@gmail.com> wrote: > > > Page owner uses the page_ext structure to keep meta-information > > for every page in the system. The structure also contains a field > > of type 'struct stack_trace', page owner uses this field during > > invocation of the function save_stack_trace. It is easy to notice > > that keeping a copy of this structure for every page in the system > > is very inefficiently in terms of memory. > > > > The patch removes this unnecessary field of page_ext and forces > > page owner to use a stack_trace structure allocated on the stack. > > > > Fair enough. > > What do you think of the below tweaks? Seems a bit neater, and it will > cause all unmentioned fields to be zeroed out. It's not a bug to leave > .max_entries uninitialised in print_page_owner(), but it's nice not to > leave loose ends. Yes, with your fix the code looks much better! > > --- a/mm/page_owner.c~mm-page_ext-remove-unnecessary-stack_trace-field-fix > +++ a/mm/page_owner.c > @@ -59,15 +59,14 @@ void __reset_page_owner(struct page *pag > > void __set_page_owner(struct page *page, unsigned int order, gfp_t gfp_mask) > { > - struct page_ext *page_ext; > - struct stack_trace trace; > + struct page_ext *page_ext = lookup_page_ext(page); > + struct stack_trace trace = { > + .nr_entries = 0, > + .max_entries = ARRAY_SIZE(page_ext->trace_entries), > + .entries = &page_ext->trace_entries[0], > + .skip = 3, > + }; > > - page_ext = lookup_page_ext(page); > - > - trace.nr_entries = 0; > - trace.max_entries = ARRAY_SIZE(page_ext->trace_entries); > - trace.entries = &page_ext->trace_entries[0]; > - trace.skip = 3; > save_stack_trace(&trace); > > page_ext->order = order; > @@ -83,8 +82,11 @@ print_page_owner(char __user *buf, size_ > { > int ret; > int pageblock_mt, page_mt; > - struct stack_trace trace; > char *kbuf; > + struct stack_trace trace = { > + .nr_entries = page_ext->nr_entries, > + .entries = &page_ext->trace_entries[0], > + }; > > kbuf = kmalloc(count, GFP_KERNEL); > if (!kbuf) > @@ -122,9 +124,6 @@ print_page_owner(char __user *buf, size_ > if (ret >= count) > goto err; > > - trace.nr_entries = page_ext->nr_entries; > - trace.entries = &page_ext->trace_entries[0]; > - > ret += snprint_stack_trace(kbuf + ret, count - ret, &trace, 0); > if (ret >= count) > goto err; > _ > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-01-28 20:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-01-26 21:52 [PATCH 0/1] mm/page_ext: remove unnecessary stack_trace field Sergei Rogachev 2015-01-26 21:52 ` [PATCH 1/1] " Sergei Rogachev 2015-01-27 8:36 ` Joonsoo Kim 2015-01-27 22:43 ` Andrew Morton 2015-01-28 19:08 ` Sergei Rogachev
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.