[dm-devel] [PATCH] md/dm-ioctl.c: optimize memory allocation in copy_params

[Zhang, Yanmin]

On 2014/7/9 22:53, Mikulas Patocka wrote:
>
> On Wed, 9 Jul 2014, Zhang, Yanmin wrote:
>
>> On 2014/7/9 6:39, Mikulas Patocka wrote:
>>
>>> Hi
>> Mikulas,
>>
>> Thanks for your kind comments.
>>
>>> I don't really know what is the purpose of this patch. In existing device
>>> mapper code, if kmalloc fails, the allocation is retried with __vmalloc.
>>> So there is no need to avoid kmalloc aritifically.
>>>
>>> kmalloc doesn't cause memory fragmentation. If the memory is too
>>> fragmented, kmalloc fails. If it isn't, it succeeds. But it doesn't cause
>>> memory being fragmented.
>> I agree with you. The patch's original description is not appropriate.
>> Basically, memory fragmentation is not caused by this kmalloc.
>>
>> The right description is: When memory is fragmented and most memory is used
>> up,
>> kmalloc a big memory might cause lots of OutOFMemory and system might kill
>> lots of processes. Then, system might hang.

Sorry for replying you too late. I am very busy in some other critical issues.

> The question is - does this particular kmalloc in device mapper cause out
> of memory or killing of other tasks? It has flags __GFP_NORETRY,

When memory is fragmented, drivers need allocate small pages instead of big
memory. Even with __GFP_NORETRY, driver might get a big memory by luck. That means
other drivers would get fewer chances to fulfill their memory requests, such like
allocating 2 pages for task_struct. Later on, OOM might happen.

> __GFP_NOMEMALLOC, __GFP_NOWARN so it shouldn't cause any trouble. It
> should just fail silently if memory is fragmented.

It's hard to say this call causes out of memory. There are many such places
in kernel to allocate big continuous memory. One is in seq_read, where we
created a patch to use vmalloc instead of kmalloc to fix it, but got far
worse comments as it's very old code. Another is in our own gfx driver.
We want to fix all. We can't blame the OOM to just one place.

Monkey testing is popular for Android development. We run the testing frequently.
It might start lots of applications. Eventually, it is a comprehensive testing.

>
> Do you have some stacktrace that identifies this kmalloc as a problem?

Sometimes, when OOM happens, kernel log shows some backtrace of big continuous
memory allocation failure. Sometimes, when board can't respond and watchdog might
reset the board after saving thread callchain into disk.

>
> Do this test - prepare two kernels that are identical, except that one
> kernel has that one-line change in dm-ioctl. Boot each kernel 10 times, do
> exactly the same operation after boot. Does the kernel with the patch
> always behave correctly and does the kernel without the patch always fail?

No. Instead of just one, many places have impact on the OOM issue.

> Report the result - how many failures did you get with or without that
> one-line patch. Without such a test - I just don't believe that your patch
> makes any difference.
>
> Another question - your patch only makes change if some device mapper
> ioctl has more than 16kB arugments. Which ioctl with more than 16kB
> arguments do you use? Do you load such a big table to device mapper? How
> often do you call that ioctl with such big arguments?

Xinhui's email mentions the ioctl details. In android, there is a command
"dumpstate", it run many other commands to collect information. In our
scenario, it run command "vdc dump", and vdc uses socket to pass some
parameters to "vold", then vold generates ioctl.

Thanks for your patience.