Messages in this thread | | | Date | Wed, 2 Mar 2022 17:51:32 +0100 | Subject | Re: [PATCH 2/5] mm/slub: use stackdepot to save stack trace in objects | From | Vlastimil Babka <> |
| |
On 2/27/22 10:44, Hyeonggon Yoo wrote: > On Fri, Feb 25, 2022 at 07:03:15PM +0100, Vlastimil Babka wrote: >> From: Oliver Glitta <glittao@gmail.com> >> >> Many stack traces are similar so there are many similar arrays. >> Stackdepot saves each unique stack only once. >> >> Replace field addrs in struct track with depot_stack_handle_t handle. Use >> stackdepot to save stack trace. >> > > I think it's not a replacement?
It is, for the array 'addrs':
-#ifdef CONFIG_STACKTRACE - unsigned long addrs[TRACK_ADDRS_COUNT]; /* Called from address */ +#ifdef CONFIG_STACKDEPOT + depot_stack_handle_t handle;
Not confuse with 'addr' which is the immediate caller and indeed stays for redundancy/kernels without stack trace enabled.
>> The benefits are smaller memory overhead and possibility to aggregate >> per-cache statistics in the following patch using the stackdepot handle >> instead of matching stacks manually. >> >> [ vbabka@suse.cz: rebase to 5.17-rc1 and adjust accordingly ] >> >> This was initially merged as commit 788691464c29 and reverted by commit >> ae14c63a9f20 due to several issues, that should now be fixed. >> The problem of unconditional memory overhead by stackdepot has been >> addressed by commit 2dba5eb1c73b ("lib/stackdepot: allow optional init >> and stack_table allocation by kvmalloc()"), so the dependency on >> stackdepot will result in extra memory usage only when a slab cache >> tracking is actually enabled, and not for all CONFIG_SLUB_DEBUG builds. >> The build failures on some architectures were also addressed, and the >> reported issue with xfs/433 test did not reproduce on 5.17-rc1 with this >> patch. > > This is just an idea and beyond this patch. > > After this patch, now we have external storage that records stack traces.
Well, we had it before this patch too.
> It's possible that some rare stack traces are in stack depot, but > not reachable because track is overwritten.
Yes.
> I think it's worth implementing a way to iterate through stacks in stack depot?
The question is for what use case? We might even not know who stored them - could have been page_owner, or other stack depot users. But the point is usually not to learn about all existing traces, but to determine which ones cause an object lifetime bug, or memory leak.
>> >> Signed-off-by: Oliver Glitta <glittao@gmail.com> >> Signed-off-by: Vlastimil Babka <vbabka@suse.cz> >> Cc: David Rientjes <rientjes@google.com> >> Cc: Christoph Lameter <cl@linux.com> >> Cc: Pekka Enberg <penberg@kernel.org> >> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> >
| |