Re: [RESEND PATCH v5 2/4] mm/vmalloc: revert "mm/vmalloc.c: emit the failure message before return"

[Wanpeng Li <liwanp@linux.vnet.ibm.com>]

Hi KOSAKI,
On Tue, Sep 17, 2013 at 01:39:13AM -0400, KOSAKI Motohiro wrote:
>On Mon, Sep 16, 2013 at 7:41 PM, Wanpeng Li <liwanp@linux.vnet.ibm.com> wrote:
>> Hi KOSAKI,
>> On Mon, Sep 16, 2013 at 04:15:29PM -0400, KOSAKI Motohiro wrote:
>>>On 9/14/2013 7:45 PM, Wanpeng Li wrote:
>>>> Changelog:
>>>>  *v2 -> v3: revert commit 46c001a2 directly
>>>>
>>>> Don't warning twice in __vmalloc_area_node and __vmalloc_node_range if
>>>> __vmalloc_area_node allocation failure. This patch revert commit 46c001a2
>>>> (mm/vmalloc.c: emit the failure message before return).
>>>>
>>>> Reviewed-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
>>>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>>>> ---
>>>>  mm/vmalloc.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>>>> index d78d117..e3ec8b4 100644
>>>> --- a/mm/vmalloc.c
>>>> +++ b/mm/vmalloc.c
>>>> @@ -1635,7 +1635,7 @@ void *__vmalloc_node_range(unsigned long size, unsigned long align,
>>>>
>>>>      addr = __vmalloc_area_node(area, gfp_mask, prot, node, caller);
>>>>      if (!addr)
>>>> -            goto fail;
>>>> +            return NULL;
>>
>> The goto fail is introduced by commit (mm/vmalloc.c: emit the failure message
>> before return), and the commit author ignore there has already have warning in
>> __vmalloc_area_node.
>>
>> http://marc.info/?l=linux-mm&m=137818671125209&w=2
>
>But, module_alloc() directly calls __vmalloc_node_range(). Your fix
>makes another regression.

I'm not sure what's the regression you mentioned.

Before patch:

module_alloc
 -> __vmalloc_node_range (waring for the second time)   <-|
  -> __vmalloc_area_node  (warning for the first time)  --|

After patch:

module_alloc
 -> __vmalloc_node_range                  <-|
  -> __vmalloc_area_node (warning once)   --|

>
>
>>>This is not right fix. Now we have following call stack.
>>>
>>> __vmalloc_node
>>>       __vmalloc_node_range
>>>               __vmalloc_node
>>>
>>>Even if we remove a warning of __vmalloc_node_range, we still be able to see double warning
>>>because we call __vmalloc_node recursively.
>>
>> Different size allocation failure in your example actually.
>
>But, when we can not allocate small size memory, almost always we
>can't allocate large size too.
>
>You need some refactoring and make right fix.

There is warning in __vmalloc_area_node for different size which you
metioned, could you point out what need refactor? ;-)

Regards,
Wanpeng Li 




--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>