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

[Alexander Potapenko]

On Thu, Mar 10, 2016 at 5:58 PM, Andrey Ryabinin <ryabinin.a.a@xxxxxxxxx> wrote:
> 2016-03-08 14:42 GMT+03:00 Alexander Potapenko <glider@xxxxxxxxxx>:
>> On Tue, Mar 1, 2016 at 12:57 PM, Andrey Ryabinin <ryabinin.a.a@xxxxxxxxx> wrote:
>>>>>
>>>>>> +                     page = alloc_pages(alloc_flags, STACK_ALLOC_ORDER);
>>>>>
>>>>> STACK_ALLOC_ORDER = 4 - that's a lot. Do you really need that much?
>>>>
>>>> Part of the issue the atomic context above. When we can't allocate
>>>> memory we still want to save the stack trace. When we have less than
>>>> STACK_ALLOC_ORDER memory, we try to preallocate another
>>>> STACK_ALLOC_ORDER in advance. So in the worst case, we have
>>>> STACK_ALLOC_ORDER memory and that should be enough to handle all
>>>> kmalloc/kfree in the atomic context. 1 page does not look enough. I
>>>> think Alex did some measuring of the failure race (when we are out of
>>>> memory and can't allocate more).
>>>>
>>>
>>> A lot of 4-order pages will lead to high fragmentation. You don't need physically contiguous memory here,
>>> so try to use vmalloc(). It is slower, but fragmentation won't be problem.
>> I've tried using vmalloc(), but turned out it's calling KASAN hooks
>> again. Dealing with reentrancy in this case sounds like an overkill.
>
> We'll have to deal with recursion eventually. Using stackdepot for
> page owner will cause recursion.
>
>> Given that we only require 9 Mb most of the time, is allocating
>> physical pages still a problem?
>>
>
> This is not about size, this about fragmentation. vmalloc allows to
> utilize available low-order pages,
> hence reduce the fragmentation.
I've attempted to add __vmalloc(STACK_ALLOC_SIZE, alloc_flags,
PAGE_KERNEL) (also tried vmalloc(STACK_ALLOC_SIZE)) instead of
page_alloc() and am now getting a crash in
kmem_cache_alloc_node_trace() in mm/slab.c, because it doesn't allow
the kmem_cache pointer to be NULL (it's dereferenced when calling
trace_kmalloc_node()).

Steven, do you know if this because of my code violating some contract
(e.g. I'm calling vmalloc() too early, when kmalloc_caches[] haven't
been initialized), or is this a bug in kmem_cache_alloc_node_trace()
itself?

>>> And one more thing. Take a look at mempool, because it's generally used to solve the problem you have here
>>> (guaranteed allocation in atomic context).
>> As far as I understood the docs, mempools have a drawback of
>> allocating too much memory which won't be available for any other use.
>
> As far as I understood your code, it has a drawback of
> allocating too much memory which won't be available for any other use ;)
>
> However, now I think that mempool doesn't fit here. We never free
> memory => never return it to pool.
> And this will cause 5sec delays between allocation retries in mempool_alloc().
>
>
>> O'Reily's "Linux Device Drivers" even suggests not using mempools in
>> any case when it's easier to deal with allocation failures (that
>> advice is for device drivers, not sure if that stands for other
>> subsystems though).
>>
>>
>> --
>> 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