Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1375549 > unrolled thread

Re: [PATCH v7 5/7] mm, kasan: Stackdepot implementation. Enable stackdepot for SLAB

Started byJoonsoo Kim <iamjoonsoo.kim@lge.com>
First post2016-04-11 09:50 +0200
Last post2016-04-12 06:50 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v7 5/7] mm, kasan: Stackdepot implementation. Enable  stackdepot for SLAB Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-04-11 09:50 +0200
    Re: [PATCH v7 5/7] mm, kasan: Stackdepot implementation. Enable  stackdepot for SLAB Alexander Potapenko <glider@google.com> - 2016-04-11 16:40 +0200
      Re: [PATCH v7 5/7] mm, kasan: Stackdepot implementation. Enable  stackdepot for SLAB Alexander Potapenko <glider@google.com> - 2016-04-11 17:00 +0200
        Re: [PATCH v7 5/7] mm, kasan: Stackdepot implementation. Enable  stackdepot for SLAB Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-04-12 06:50 +0200

#1375549 — Re: [PATCH v7 5/7] mm, kasan: Stackdepot implementation. Enable stackdepot for SLAB

FromJoonsoo Kim <iamjoonsoo.kim@lge.com>
Date2016-04-11 09:50 +0200
SubjectRe: [PATCH v7 5/7] mm, kasan: Stackdepot implementation. Enable stackdepot for SLAB
Message-ID<rmELE-2aR-21@gated-at.bofh.it>
On Mon, Mar 14, 2016 at 11:43:43AM +0100, Alexander Potapenko wrote:
> +depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
> +				    gfp_t alloc_flags)
> +{
> +	u32 hash;
> +	depot_stack_handle_t retval = 0;
> +	struct stack_record *found = NULL, **bucket;
> +	unsigned long flags;
> +	struct page *page = NULL;
> +	void *prealloc = NULL;
> +	bool *rec;
> +
> +	if (unlikely(trace->nr_entries == 0))
> +		goto fast_exit;
> +
> +	rec = this_cpu_ptr(&depot_recursion);
> +	/* Don't store the stack if we've been called recursively. */
> +	if (unlikely(*rec))
> +		goto fast_exit;
> +	*rec = true;
> +
> +	hash = hash_stack(trace->entries, trace->nr_entries);
> +	/* Bad luck, we won't store this stack. */
> +	if (hash == 0)
> +		goto exit;

Hello,

why is hash == 0 skipped?

Thanks.

[toc] | [next] | [standalone]

#1376031

FromAlexander Potapenko <glider@google.com>
Date2016-04-11 16:40 +0200
Message-ID<rmLaq-7r8-21@gated-at.bofh.it>
In reply to#1375549
On Mon, Apr 11, 2016 at 9:44 AM, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> On Mon, Mar 14, 2016 at 11:43:43AM +0100, Alexander Potapenko wrote:
>> +depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
>> +                                 gfp_t alloc_flags)
>> +{
>> +     u32 hash;
>> +     depot_stack_handle_t retval = 0;
>> +     struct stack_record *found = NULL, **bucket;
>> +     unsigned long flags;
>> +     struct page *page = NULL;
>> +     void *prealloc = NULL;
>> +     bool *rec;
>> +
>> +     if (unlikely(trace->nr_entries == 0))
>> +             goto fast_exit;
>> +
>> +     rec = this_cpu_ptr(&depot_recursion);
>> +     /* Don't store the stack if we've been called recursively. */
>> +     if (unlikely(*rec))
>> +             goto fast_exit;
>> +     *rec = true;
>> +
>> +     hash = hash_stack(trace->entries, trace->nr_entries);
>> +     /* Bad luck, we won't store this stack. */
>> +     if (hash == 0)
>> +             goto exit;
>
> Hello,
>
> why is hash == 0 skipped?
>
> Thanks.
We have to keep a special value to distinguish allocations for which
we don't have the stack trace for some reason.
Making 0 such a value seems natural.


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

[toc] | [prev] | [next] | [standalone]

#1376045

FromAlexander Potapenko <glider@google.com>
Date2016-04-11 17:00 +0200
Message-ID<rmLtM-7zE-21@gated-at.bofh.it>
In reply to#1376031
On Mon, Apr 11, 2016 at 4:39 PM, Alexander Potapenko <glider@google.com> wrote:
> On Mon, Apr 11, 2016 at 9:44 AM, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
>> On Mon, Mar 14, 2016 at 11:43:43AM +0100, Alexander Potapenko wrote:
>>> +depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
>>> +                                 gfp_t alloc_flags)
>>> +{
>>> +     u32 hash;
>>> +     depot_stack_handle_t retval = 0;
>>> +     struct stack_record *found = NULL, **bucket;
>>> +     unsigned long flags;
>>> +     struct page *page = NULL;
>>> +     void *prealloc = NULL;
>>> +     bool *rec;
>>> +
>>> +     if (unlikely(trace->nr_entries == 0))
>>> +             goto fast_exit;
>>> +
>>> +     rec = this_cpu_ptr(&depot_recursion);
>>> +     /* Don't store the stack if we've been called recursively. */
>>> +     if (unlikely(*rec))
>>> +             goto fast_exit;
>>> +     *rec = true;
>>> +
>>> +     hash = hash_stack(trace->entries, trace->nr_entries);
>>> +     /* Bad luck, we won't store this stack. */
>>> +     if (hash == 0)
>>> +             goto exit;
>>
>> Hello,
>>
>> why is hash == 0 skipped?
>>
>> Thanks.
> We have to keep a special value to distinguish allocations for which
> we don't have the stack trace for some reason.
> Making 0 such a value seems natural.
Well, the above statement is false.
Because we only compare the hash to the records that are already in
the depot, there's no point in reserving this value.
>
> --
> Alexander Potapenko
> Software Engineer
>
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
>
> Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

[toc] | [prev] | [next] | [standalone]

#1376445

FromJoonsoo Kim <iamjoonsoo.kim@lge.com>
Date2016-04-12 06:50 +0200
Message-ID<rmYr0-1aa-7@gated-at.bofh.it>
In reply to#1376045
On Mon, Apr 11, 2016 at 04:51:47PM +0200, Alexander Potapenko wrote:
> On Mon, Apr 11, 2016 at 4:39 PM, Alexander Potapenko <glider@google.com> wrote:
> > On Mon, Apr 11, 2016 at 9:44 AM, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> >> On Mon, Mar 14, 2016 at 11:43:43AM +0100, Alexander Potapenko wrote:
> >>> +depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
> >>> +                                 gfp_t alloc_flags)
> >>> +{
> >>> +     u32 hash;
> >>> +     depot_stack_handle_t retval = 0;
> >>> +     struct stack_record *found = NULL, **bucket;
> >>> +     unsigned long flags;
> >>> +     struct page *page = NULL;
> >>> +     void *prealloc = NULL;
> >>> +     bool *rec;
> >>> +
> >>> +     if (unlikely(trace->nr_entries == 0))
> >>> +             goto fast_exit;
> >>> +
> >>> +     rec = this_cpu_ptr(&depot_recursion);
> >>> +     /* Don't store the stack if we've been called recursively. */
> >>> +     if (unlikely(*rec))
> >>> +             goto fast_exit;
> >>> +     *rec = true;
> >>> +
> >>> +     hash = hash_stack(trace->entries, trace->nr_entries);
> >>> +     /* Bad luck, we won't store this stack. */
> >>> +     if (hash == 0)
> >>> +             goto exit;
> >>
> >> Hello,
> >>
> >> why is hash == 0 skipped?
> >>
> >> Thanks.
> > We have to keep a special value to distinguish allocations for which
> > we don't have the stack trace for some reason.
> > Making 0 such a value seems natural.
> Well, the above statement is false.
> Because we only compare the hash to the records that are already in
> the depot, there's no point in reserving this value.

So, could you make a patch for it?

Thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web