* [PATCH 1/1] mm/page_owner: fix recursion bug after changing skip entries [not found] <CGME20180321043818epcas5p176fe0e0bbfce685420df2bfb7a421acd@epcas5p1.samsung.com> @ 2018-03-21 4:37 ` Maninder Singh 2018-03-21 8:25 ` Vlastimil Babka 0 siblings, 1 reply; 2+ messages in thread From: Maninder Singh @ 2018-03-21 4:37 UTC (permalink / raw) To: akpm, vbabka, mhocko, osalvador, gregkh, ayush.m, guptap, vinmenon, gomonovych Cc: linux-mm, linux-kernel, a.sahrawat, pankaj.m, Maninder Singh, Vaneet Narang This patch fixes "5f48f0bd4e368425db4424b9afd1bd251d32367a". (mm, page_owner: skip unnecessary stack_trace entries) Because if we skip first two entries then logic of checking count value as 2 for recursion is broken and code will go in one depth recursion. so we need to check only one call of _RET_IP(__set_page_owner) while checking for recursion. Current Backtrace while checking for recursion:- (save_stack) from (__set_page_owner) // (But recursion returns true here) (__set_page_owner) from (get_page_from_freelist) (get_page_from_freelist) from (__alloc_pages_nodemask) (__alloc_pages_nodemask) from (depot_save_stack) (depot_save_stack) from (save_stack) // recursion should return true here (save_stack) from (__set_page_owner) (__set_page_owner) from (get_page_from_freelist) (get_page_from_freelist) from (__alloc_pages_nodemask+) (__alloc_pages_nodemask) from (depot_save_stack) (depot_save_stack) from (save_stack) (save_stack) from (__set_page_owner) (__set_page_owner) from (get_page_from_freelist) Correct Backtrace with fix: (save_stack) from (__set_page_owner) // recursion returned true here (__set_page_owner) from (get_page_from_freelist) (get_page_from_freelist) from (__alloc_pages_nodemask+) (__alloc_pages_nodemask) from (depot_save_stack) (depot_save_stack) from (save_stack) (save_stack) from (__set_page_owner) (__set_page_owner) from (get_page_from_freelist) Signed-off-by: Maninder Singh <maninder1.s@samsung.com> Signed-off-by: Vaneet Narang <v.narang@samsung.com> --- mm/page_owner.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mm/page_owner.c b/mm/page_owner.c index 8592543..46ab1c4 100644 --- a/mm/page_owner.c +++ b/mm/page_owner.c @@ -123,13 +123,13 @@ void __reset_page_owner(struct page *page, unsigned int order) static inline bool check_recursive_alloc(struct stack_trace *trace, unsigned long ip) { - int i, count; + int i; if (!trace->nr_entries) return false; - for (i = 0, count = 0; i < trace->nr_entries; i++) { - if (trace->entries[i] == ip && ++count == 2) + for (i = 0; i < trace->nr_entries; i++) { + if (trace->entries[i] == ip) return true; } -- 1.7.1 ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH 1/1] mm/page_owner: fix recursion bug after changing skip entries 2018-03-21 4:37 ` [PATCH 1/1] mm/page_owner: fix recursion bug after changing skip entries Maninder Singh @ 2018-03-21 8:25 ` Vlastimil Babka 0 siblings, 0 replies; 2+ messages in thread From: Vlastimil Babka @ 2018-03-21 8:25 UTC (permalink / raw) To: Maninder Singh, akpm, mhocko, osalvador, gregkh, ayush.m, guptap, vinmenon, gomonovych Cc: linux-mm, linux-kernel, a.sahrawat, pankaj.m, Vaneet Narang On 03/21/2018 05:37 AM, Maninder Singh wrote: > This patch fixes "5f48f0bd4e368425db4424b9afd1bd251d32367a". > (mm, page_owner: skip unnecessary stack_trace entries) > > Because if we skip first two entries then logic of checking count > value as 2 for recursion is broken and code will go in one depth > recursion. > > so we need to check only one call of _RET_IP(__set_page_owner) > while checking for recursion. > > Current Backtrace while checking for recursion:- > > (save_stack) from (__set_page_owner) // (But recursion returns true here) > (__set_page_owner) from (get_page_from_freelist) > (get_page_from_freelist) from (__alloc_pages_nodemask) > (__alloc_pages_nodemask) from (depot_save_stack) > (depot_save_stack) from (save_stack) // recursion should return true here > (save_stack) from (__set_page_owner) > (__set_page_owner) from (get_page_from_freelist) > (get_page_from_freelist) from (__alloc_pages_nodemask+) > (__alloc_pages_nodemask) from (depot_save_stack) > (depot_save_stack) from (save_stack) > (save_stack) from (__set_page_owner) > (__set_page_owner) from (get_page_from_freelist) > > Correct Backtrace with fix: > > (save_stack) from (__set_page_owner) // recursion returned true here > (__set_page_owner) from (get_page_from_freelist) > (get_page_from_freelist) from (__alloc_pages_nodemask+) > (__alloc_pages_nodemask) from (depot_save_stack) > (depot_save_stack) from (save_stack) > (save_stack) from (__set_page_owner) > (__set_page_owner) from (get_page_from_freelist) > > Signed-off-by: Maninder Singh <maninder1.s@samsung.com> > Signed-off-by: Vaneet Narang <v.narang@samsung.com> Fixes: 5f48f0bd4e36 ("mm, page_owner: skip unnecessary stack_trace entries") Good catch. Acked-by: Vlastimil Babka <vbabka@suse.cz> > --- > mm/page_owner.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/mm/page_owner.c b/mm/page_owner.c > index 8592543..46ab1c4 100644 > --- a/mm/page_owner.c > +++ b/mm/page_owner.c > @@ -123,13 +123,13 @@ void __reset_page_owner(struct page *page, unsigned int order) > static inline bool check_recursive_alloc(struct stack_trace *trace, > unsigned long ip) > { > - int i, count; > + int i; > > if (!trace->nr_entries) > return false; > > - for (i = 0, count = 0; i < trace->nr_entries; i++) { > - if (trace->entries[i] == ip && ++count == 2) > + for (i = 0; i < trace->nr_entries; i++) { > + if (trace->entries[i] == ip) > return true; > } > > ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-03-21 8:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20180321043818epcas5p176fe0e0bbfce685420df2bfb7a421acd@epcas5p1.samsung.com>
2018-03-21 4:37 ` [PATCH 1/1] mm/page_owner: fix recursion bug after changing skip entries Maninder Singh
2018-03-21 8:25 ` Vlastimil Babka
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox