On 4/5/22 23:40, David Rientjes wrote: >> -static void set_track(struct kmem_cache *s, void *object, >> +static void noinline set_track(struct kmem_cache *s, void *object, >> enum track_item alloc, unsigned long addr) >> { >> struct track *p = get_track(s, object, alloc); >> >> -#ifdef CONFIG_STACKTRACE >> +#ifdef CONFIG_STACKDEPOT >> + unsigned long entries[TRACK_ADDRS_COUNT]; >> unsigned int nr_entries; >> >> - metadata_access_enable(); >> - nr_entries = stack_trace_save(kasan_reset_tag(p->addrs), >> - TRACK_ADDRS_COUNT, 3); >> - metadata_access_disable(); >> - >> - if (nr_entries < TRACK_ADDRS_COUNT) >> - p->addrs[nr_entries] = 0; >> + nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 3); >> + p->handle = stack_depot_save(entries, nr_entries, GFP_NOWAIT); > > I think this should also have __GFP_NOWARN set since this allocation could > easily fail and it would unnecessarily spam the kernel log unless we > actually care about the stack trace being printed later (and the patch > already indicates the allocation failed in print_track() when it matters). Good point. But turns out __stack_depot_save() adds it for us already. > Otherwise: > > Acked-by: David Rientjes <rientjes@xxxxxxxxxx> Thanks!