Re: [PATCH v1 5/8] mm, kasan: Stackdepot implementation. Enable stackdepot for SLAB
From: Joonsoo Kim <hidden>
Date: 2016-02-18 07:58:52
Also in:
lkml
2016-02-17 3:37 GMT+09:00 Alexander Potapenko [off-list ref]:
On Mon, Feb 1, 2016 at 3:55 AM, Joonsoo Kim [off-list ref] wrote:quoted
On Thu, Jan 28, 2016 at 02:27:44PM +0100, Alexander Potapenko wrote:quoted
On Thu, Jan 28, 2016 at 1:51 PM, Alexander Potapenko [off-list ref] wrote:quoted
On Jan 28, 2016 8:40 AM, "Joonsoo Kim" [off-list ref] wrote:quoted
Hello, On Wed, Jan 27, 2016 at 07:25:10PM +0100, Alexander Potapenko wrote:quoted
Stack depot will allow KASAN store allocation/deallocation stack traces for memory chunks. The stack traces are stored in a hash table and referenced by handles which reside in the kasan_alloc_meta and kasan_free_meta structures in the allocated memory chunks.Looks really nice! Could it be more generalized to be used by other feature that need to store stack trace such as tracepoint or page owner?Certainly yes, but see below.quoted
If it could be, there is one more requirement. I understand the fact that entry is never removed from depot makes things very simpler, but, for general usecases, it's better to use reference count and allow to remove. Is it possible?For our use case reference counting is not really necessary, and it would introduce unwanted contention.Okay.quoted
quoted
There are two possible options, each having its advantages and drawbacks: we can let the clients store the refcounters directly in their stacks (more universal, but harder to use for the clients), or keep the counters in the depot but add an API that does not change them (easier for the clients, but potentially error-prone). I'd say it's better to actually find at least one more user for the stack depot in order to understand the requirements, and refactor the code after that.I re-think the page owner case and it also may not need refcount. For now, just moving this stuff to /lib would be helpful for other future user.I agree this code may need to be moved to /lib someday, but I wouldn't hurry with that. Right now it is quite KASAN-specific, and it's unclear yet whether anyone else is going to use it. I suggest we keep it in mm/kasan for now, and factor the common parts into /lib when the need arises.
Please consider it one more time. I really have a plan to use it on page owner,
because using page owner requires too many memory for stack trace and
it changes system behaviour a lot.
Page owner uses following structure to store stack trace.
struct page_ext {
unsigned long flags;
#ifdef CONFIG_PAGE_OWNER
unsigned int order;
gfp_t gfp_mask;
unsigned int nr_entries;
int last_migrate_reason;
unsigned long trace_entries[8];
#endif
};
Using stack depot in page owner would be straight forward if stack depot
is in /lib. It is possible to move it when needed but it requires moving
a file and it would not be desirable.
quoted
BTW, is there any performance number? I guess that it could affect the performance.I've compared the performance of KASAN with SLAB allocator on a small synthetic benchmark in two modes: with stack depot enabled and with kasan_save_stack() unconditionally returning 0. In the former case 8% more time was spent in the kernel than in the latter case. If I am not mistaking, for SLUB allocator the bookkeeping (enabled with the slub_debug=UZ boot options) take only 1.5 time, so the difference is worth looking into (at least before we switch SLUB to stack depot).
Okay. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>