Re: [PATCH] mm/mmap.c: Remove redundent 'get_area' function pointer in get_unmapped_area()

[Chen Gang <xili_gchen_5257@hotmail.com>]

On 9/7/15 20:41, Oleg Nesterov wrote:
> On 09/05, Chen Gang wrote:
>>
>> From a1bf4726f71d6d0394b41309944646fc806a8a0c Mon Sep 17 00:00:00 2001
>> From: Chen Gang <gang.chen.5i5j@gmail.com>
>> Date: Sat, 5 Sep 2015 21:51:08 +0800
>> Subject: [PATCH] mm/mmap.c: Remove redundent 'get_area' function pointer in
>> get_unmapped_area()
>>
>> Call the function pointer directly, then let code a bit simpler.
> ^^^^^^^^^^^^^^^^^^
>
> This is subjective you know ;)
>

Oh, sorry. The comments need be improved.

> I guess the author of this code added this variable to make the code
> more readable. And to me it becomes less readable after your change.
>
> I leave this to you and maintainers.
>

OK, I can understand, every members have their own taste (my taste is
if one buffering variable is used within 2 times, I want to remove it).

For optimization, the original code maybe be a little better.

So for me, if more than 20% members still like the original code, we
should keep the original code no touch.


Thanks.

>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>> ---
>> mm/mmap.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/mm/mmap.c b/mm/mmap.c
>> index 4db7cf0..39fd727 100644
>> --- a/mm/mmap.c
>> +++ b/mm/mmap.c
>> @@ -2012,10 +2012,8 @@ unsigned long
>> get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
>> unsigned long pgoff, unsigned long flags)
>> {
>> - unsigned long (*get_area)(struct file *, unsigned long,
>> - unsigned long, unsigned long, unsigned long);
>> -
>> unsigned long error = arch_mmap_check(addr, len, flags);
>> +
>> if (error)
>> return error;
>>
>> @@ -2023,10 +2021,12 @@ get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
>> if (len> TASK_SIZE)
>> return -ENOMEM;
>>
>> - get_area = current->mm->get_unmapped_area;
>> if (file && file->f_op->get_unmapped_area)
>> - get_area = file->f_op->get_unmapped_area;
>> - addr = get_area(file, addr, len, pgoff, flags);
>> + addr = file->f_op->get_unmapped_area(file, addr, len,
>> + pgoff, flags);
>> + else
>> + addr = current->mm->get_unmapped_area(file, addr, len,
>> + pgoff, flags);
>> if (IS_ERR_VALUE(addr))
>> return addr;
>>
>> --
>> 1.9.3
>>
>>
>

--
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed