Re: [PATCH] mm: get rid of odd jump label in find_mergeable_anon_vma

[linmiaohe]

> On 11/15/19 4:58 AM, David Hildenbrand wrote:
>> On 15.11.19 07:36, linmiaohe wrote:
>>> From: Miaohe Lin <linmiaohe@xxxxxxxxxx>
>> 
>> I'm pro removing unnecessary jump labels.
>
>Thank you, simpler code is good--all other things being equal. 
>The  tradeoff is, as Qian points out: code churn and risk in critical code paths.
>
>In this case, I'd claim it's OK to improve this one, because we can likely get it right by visual inspection, and the pre-existing code is quite poor. And being in the kernel does not necessarily give weak code a free pass to remain there and incur maintenance and annoyance costs until the end of time. :)
>
>However, please spend equal time when you write your commit descriptions, because that's also very important. Commit logs should also be clear!
>
>> 
>> Subject: "mm: get rid of jump labels in find_mergeable_anon_vma()"
>> 
>>>
>>> The odd jump label try_prev and none is not really need
>
>s/need/needed/
>
>> 
>> s/odd jump label/jump labels/
>> 
>> s/is/are/
>> 
>>> in func find_mergeable_anon_vma, eliminate them to improve 
>>> readability.
>>>
>>> Signed-off-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>
>>> ---
>>>  mm/mmap.c | 18 +++++++-----------
>>>  1 file changed, 7 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/mm/mmap.c b/mm/mmap.c
>>> index 4d4db76a07da..ab980d468a10 100644
>> 
>> Let me suggest the following instead:
>> 
>> /* Try next first */
>> near = vma->vm_next;
>> if (near) {
>> 	anon_vma = reusable_anon_vma(near, vma, near);
>> 	if (anon_vma)
>> 		return anon_vma;
>> }
>> /* Try prev next */
>> near = vma->vm_prev;
>> if (near) {
>> 	anon_vma = reusable_anon_vma(near, vma, near);
>> 	if (anon_vma)
>> 		return anon_vma;
>> }
>> 
>
>Actually, it can be further simplified, because you don't need that last "if" statement, because you're returning NULL after this.
>
>So just return anon_vma there. (And adjust the comment block at the end, so that it's clear that anon_vma might be null.)
>
>

Many Thanks for all of your precious advice. I will fix my patch and send a v2 patch. Thanks again.