| Message ID | 20210902000154.1096484-3-imran.f.khan@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | lib, stackdepot: check stackdepot handle before accessing slabs | expand |
On 9/2/21 02:01, Imran Khan wrote: > To print a stack entries, users of stackdepot, first > use stack_depot_fetch to get a list of stack entries > and then use stack_trace_print to print this list. > Provide a helper in stackdepot to print stack entries > based on stackdepot handle. > Also change above mentioned users to use this helper. > > Signed-off-by: Imran Khan <imran.f.khan@oracle.com> > Suggested-by: Vlastimil Babka <vbabka@suse.cz> Acked-by: Vlastimil Babka <vbabka@suse.cz>
On Thu, Sep 2, 2021 at 9:55 AM Vlastimil Babka <vbabka@suse.cz> wrote: > > On 9/2/21 02:01, Imran Khan wrote: > > To print a stack entries, users of stackdepot, first > > use stack_depot_fetch to get a list of stack entries > > and then use stack_trace_print to print this list. > > Provide a helper in stackdepot to print stack entries > > based on stackdepot handle. > > Also change above mentioned users to use this helper. > > > > Signed-off-by: Imran Khan <imran.f.khan@oracle.com> > > Suggested-by: Vlastimil Babka <vbabka@suse.cz> > > Acked-by: Vlastimil Babka <vbabka@suse.cz> Reviewed-by: Alexander Potapenko <glider@google.com>
On 9/2/21 02:01, Imran Khan wrote: > To print a stack entries, users of stackdepot, first > use stack_depot_fetch to get a list of stack entries > and then use stack_trace_print to print this list. > Provide a helper in stackdepot to print stack entries > based on stackdepot handle. > Also change above mentioned users to use this helper. > > Signed-off-by: Imran Khan <imran.f.khan@oracle.com> > Suggested-by: Vlastimil Babka <vbabka@suse.cz> > --- > include/linux/stackdepot.h | 2 ++ > lib/stackdepot.c | 17 +++++++++++++++++ > mm/kasan/report.c | 15 +++------------ > mm/page_owner.c | 13 ++++--------- > 4 files changed, 26 insertions(+), 21 deletions(-) > > diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h > index 6bb4bc1a5f54..d77a30543dd4 100644 > --- a/include/linux/stackdepot.h > +++ b/include/linux/stackdepot.h > @@ -19,6 +19,8 @@ depot_stack_handle_t stack_depot_save(unsigned long *entries, > unsigned int stack_depot_fetch(depot_stack_handle_t handle, > unsigned long **entries); > > +void stack_depot_print(depot_stack_handle_t stack); > + > unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries); > > #ifdef CONFIG_STACKDEPOT > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 67439c082490..873aeb152f52 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -214,6 +214,23 @@ static inline struct stack_record *find_stack(struct stack_record *bucket, > return NULL; > } > > +/** > + * stack_depot_print - print stack entries from a depot > + * > + * @handle: Stack depot handle which was returned from > + * stack_depot_save(). > + * > + */ > +void stack_depot_print(depot_stack_handle_t stack) > +{ > + unsigned long *entries; > + unsigned int nr_entries; > + > + nr_entries = stack_depot_fetch(stack, &entries); Maybe this should also skip stack_trace_print when nr_entries is 0, to avoid the warning. While the callers added by this patch check handle != 0, future ones might not.
On 14/9/21 7:01 pm, Vlastimil Babka wrote: > On 9/2/21 02:01, Imran Khan wrote: >> To print a stack entries, users of stackdepot, first >> use stack_depot_fetch to get a list of stack entries >> and then use stack_trace_print to print this list. >> Provide a helper in stackdepot to print stack entries >> based on stackdepot handle. >> Also change above mentioned users to use this helper. >> [...] >> >> +/** >> + * stack_depot_print - print stack entries from a depot >> + * >> + * @handle: Stack depot handle which was returned from >> + * stack_depot_save(). >> + * >> + */ >> +void stack_depot_print(depot_stack_handle_t stack) >> +{ >> + unsigned long *entries; >> + unsigned int nr_entries; >> + >> + nr_entries = stack_depot_fetch(stack, &entries); > > Maybe this should also skip stack_trace_print when nr_entries is 0, to avoid > the warning. While the callers added by this patch check handle != 0, future > ones might not. > Agree. I have incorporated this suggestion in latest version [1] [1] https://lore.kernel.org/lkml/20210915014806.3206938-1-imran.f.khan@oracle.com/ Thanks -- Imran
diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h index 6bb4bc1a5f54..d77a30543dd4 100644 --- a/include/linux/stackdepot.h +++ b/include/linux/stackdepot.h @@ -19,6 +19,8 @@ depot_stack_handle_t stack_depot_save(unsigned long *entries, unsigned int stack_depot_fetch(depot_stack_handle_t handle, unsigned long **entries); +void stack_depot_print(depot_stack_handle_t stack); + unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries); #ifdef CONFIG_STACKDEPOT diff --git a/lib/stackdepot.c b/lib/stackdepot.c index 67439c082490..873aeb152f52 100644 --- a/lib/stackdepot.c +++ b/lib/stackdepot.c @@ -214,6 +214,23 @@ static inline struct stack_record *find_stack(struct stack_record *bucket, return NULL; } +/** + * stack_depot_print - print stack entries from a depot + * + * @handle: Stack depot handle which was returned from + * stack_depot_save(). + * + */ +void stack_depot_print(depot_stack_handle_t stack) +{ + unsigned long *entries; + unsigned int nr_entries; + + nr_entries = stack_depot_fetch(stack, &entries); + stack_trace_print(entries, nr_entries, 0); +} +EXPORT_SYMBOL_GPL(stack_depot_print); + /** * stack_depot_fetch - Fetch stack entries from a depot * diff --git a/mm/kasan/report.c b/mm/kasan/report.c index 884a950c7026..3239fd8f8747 100644 --- a/mm/kasan/report.c +++ b/mm/kasan/report.c @@ -132,20 +132,11 @@ static void end_report(unsigned long *flags, unsigned long addr) kasan_enable_current(); } -static void print_stack(depot_stack_handle_t stack) -{ - unsigned long *entries; - unsigned int nr_entries; - - nr_entries = stack_depot_fetch(stack, &entries); - stack_trace_print(entries, nr_entries, 0); -} - static void print_track(struct kasan_track *track, const char *prefix) { pr_err("%s by task %u:\n", prefix, track->pid); if (track->stack) { - print_stack(track->stack); + stack_depot_print(track->stack); } else { pr_err("(stack is not available)\n"); } @@ -214,12 +205,12 @@ static void describe_object_stacks(struct kmem_cache *cache, void *object, return; if (alloc_meta->aux_stack[0]) { pr_err("Last potentially related work creation:\n"); - print_stack(alloc_meta->aux_stack[0]); + stack_depot_print(alloc_meta->aux_stack[0]); pr_err("\n"); } if (alloc_meta->aux_stack[1]) { pr_err("Second to last potentially related work creation:\n"); - print_stack(alloc_meta->aux_stack[1]); + stack_depot_print(alloc_meta->aux_stack[1]); pr_err("\n"); } #endif diff --git a/mm/page_owner.c b/mm/page_owner.c index d24ed221357c..7918770c2b2b 100644 --- a/mm/page_owner.c +++ b/mm/page_owner.c @@ -394,8 +394,6 @@ void __dump_page_owner(const struct page *page) struct page_ext *page_ext = lookup_page_ext(page); struct page_owner *page_owner; depot_stack_handle_t handle; - unsigned long *entries; - unsigned int nr_entries; gfp_t gfp_mask; int mt; @@ -423,20 +421,17 @@ void __dump_page_owner(const struct page *page) page_owner->pid, page_owner->ts_nsec, page_owner->free_ts_nsec); handle = READ_ONCE(page_owner->handle); - if (!handle) { + if (!handle) pr_alert("page_owner allocation stack trace missing\n"); - } else { - nr_entries = stack_depot_fetch(handle, &entries); - stack_trace_print(entries, nr_entries, 0); - } + else + stack_depot_print(handle); handle = READ_ONCE(page_owner->free_handle); if (!handle) { pr_alert("page_owner free stack trace missing\n"); } else { - nr_entries = stack_depot_fetch(handle, &entries); pr_alert("page last free stack trace:\n"); - stack_trace_print(entries, nr_entries, 0); + stack_depot_print(handle); } if (page_owner->last_migrate_reason != -1)
To print a stack entries, users of stackdepot, first use stack_depot_fetch to get a list of stack entries and then use stack_trace_print to print this list. Provide a helper in stackdepot to print stack entries based on stackdepot handle. Also change above mentioned users to use this helper. Signed-off-by: Imran Khan <imran.f.khan@oracle.com> Suggested-by: Vlastimil Babka <vbabka@suse.cz> --- include/linux/stackdepot.h | 2 ++ lib/stackdepot.c | 17 +++++++++++++++++ mm/kasan/report.c | 15 +++------------ mm/page_owner.c | 13 ++++--------- 4 files changed, 26 insertions(+), 21 deletions(-)