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

[Andrey Ryabinin]

On 03/11/2016 02:18 PM, Alexander Potapenko wrote:
> 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), 

Probably. kmem_cache_init() goes before vmalloc_init().


> or is this a bug in kmem_cache_alloc_node_trace()
> itself?
>