lkml.org 
[lkml]   [2016]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v1 5/8] mm, kasan: Stackdepot implementation. Enable stackdepot for SLAB
    From
    2016-02-17 3:37 GMT+09:00 Alexander Potapenko <glider@google.com>:
    > On Mon, Feb 1, 2016 at 3:55 AM, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
    >> On Thu, Jan 28, 2016 at 02:27:44PM +0100, Alexander Potapenko wrote:
    >>> On Thu, Jan 28, 2016 at 1:51 PM, Alexander Potapenko <glider@google.com> wrote:
    >>> >
    >>> > On Jan 28, 2016 8:40 AM, "Joonsoo Kim" <iamjoonsoo.kim@lge.com> wrote:
    >>> >>
    >>> >> Hello,
    >>> >>
    >>> >> On Wed, Jan 27, 2016 at 07:25:10PM +0100, Alexander Potapenko wrote:
    >>> >> > 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.
    >>> >
    >>> >> 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.
    >>
    >>> > 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.

    >> 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.

    \
     
     \ /
      Last update: 2016-02-18 09:21    [W:5.352 / U:0.440 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site