Message-ID: <53BCB910.4070709@intel.com>
Date:	Wed, 09 Jul 2014 11:37:52 +0800
From:	"xinhui.pan" <xinhuix.pan@...el.com>
To:	"Zhang, Yanmin" <yanmin_zhang@...ux.intel.com>,
	Mikulas Patocka <mpatocka@...hat.com>
CC:	linux-kernel@...r.kernel.org, agk@...hat.com, snitzer@...hat.com,
	dm-devel@...hat.com, "Liu, ShuoX" <shuox.liu@...el.com>
Subject: Re: [dm-devel] [PATCH] md/dm-ioctl.c: optimize memory allocation
 in copy_params



于 2014年07月09日 10:01, Zhang, Yanmin 写道:
> 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.
> 
> We are enabling Android on mobiles and tablets. We hit OOM often at
> Monkey and other stress testing. Sometimes, kernel prints big memory allocation
> warning.
> 
> Theoretically, it's a good idea to call kmalloc firstly. If it fails, use vmalloc.
> However, kernel assumes drivers do the right thing. When drivers call kmalloc to
> allocate a big memory, memory management would do the best to fulfill it. When memory
> is fragmented and most memory is used up, such allocation would cause big memory
> recollection. Some processes might be killed. The worse scenario is after a process
> is killed, it is restarted and allocates memory again. That causes system hang,
> or mobile users feel a big stall. We did fix a similar issue in one of our drivers.
> 
> Usually, kernel and drivers can allocate big continuous memory at booting or
> initialization stage. After that, they need allocate small order memory. The best
> is to just allocate order-0 memory.
> 
> 
>>
>> If you have some other driver that fails because of large kmalloc failure,
>> you should fix it to use scatter-gather DMA or (if the hardware can't do
>> it) preallocate the memory when the driver is loaded.
> 
> I agree with you. Here the patch fixes the issue, where dm might allocate
> big continuous memory after booting. Monkey testing triggers it by operating
> menu Setting=>Security=>InstallfromSDcard.
> 

hi, Yanmin && Mikulas
	Thanks for your nice comments. And sorry for confusing you as my comment is not clear enough.
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.
> We are catching all places in kernel where big memory allocation happens.
> This patch is just to fix one case.
> 
> Thanks,
> Yanmin
> 
>>
>> Mikulas
>>
>>
>>
>> On Fri, 4 Jul 2014, xinhui.pan wrote:
>>
>>> KMALLOC_MAX_SIZE is too big, it easily cause memory fragmented and low memory when param_kernel->data_size
>>> is also big. That's not nice. So use vmalloc instead of kmalloc when size is larger than (PAGE_SIZE << 2).
>>> There are other drivers using kmalloc to gain a big size memory. And that cause our devices to hit hang and
>>> many worse issues. We don't benefit much when using kmalloc in such scenario.
>>>
>>> Signed-off-by: xinhui.pan <xinhuiX.pan@...el.com>
>>> Signed-off-by: yanmin.zhang <yanmin_zhang@...ux.intel.com>
>>> ---
>>>   drivers/md/dm-ioctl.c |    2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
>>> index 5152142..31c3af9 100644
>>> --- a/drivers/md/dm-ioctl.c
>>> +++ b/drivers/md/dm-ioctl.c
>>> @@ -1709,7 +1709,7 @@ static int copy_params(struct dm_ioctl __user *user, struct dm_ioctl *param_kern
>>>           * Use kmalloc() rather than vmalloc() when we can.
>>>           */
>>>          dmi = NULL;
>>> -       if (param_kernel->data_size <= KMALLOC_MAX_SIZE) {
>>> +       if (param_kernel->data_size <= (PAGE_SIZE << 2)) {
>>>                  dmi = kmalloc(param_kernel->data_size, GFP_NOIO | __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN);
>>>                  if (dmi)
>>>                          *param_flags |= DM_PARAMS_KMALLOC;
>>> -- 
>>> 1.7.9.5
>>>
>>> -- 
>>> dm-devel mailing list
>>> dm-devel@...hat.com
>>> https://www.redhat.com/mailman/listinfo/dm-devel
>>>
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists