[PATCH] mm/mremap: avoid unneeded do_munmap call

[PATCH] mm/mremap: avoid unneeded do_munmap call

[Miaohe Lin]

When old_len == new_len, do_munmap will return -EINVAL due to len == 0.
This errno will be simply ignored because of old_len != new_len check.
So it is unnecessary to call do_munmap when old_len == new_len because
nothing is actually done.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/mremap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/mremap.c b/mm/mremap.c
index e776d4c2345c..dd966621a056 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -817,9 +817,9 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
 			goto out;
 	}
 
-	if (old_len >= new_len) {
+	if (old_len > new_len) {
 		ret = do_munmap(mm, addr+new_len, old_len - new_len, uf_unmap);
-		if (ret && old_len != new_len)
+		if (ret)
 			goto out;
 		old_len = new_len;
 	}
-- 
2.23.0

Re: [PATCH] mm/mremap: avoid unneeded do_munmap call

[David Hildenbrand]

On 01.04.22 10:10, Miaohe Lin wrote:
> When old_len == new_len, do_munmap will return -EINVAL due to len == 0.
> This errno will be simply ignored because of old_len != new_len check.
> So it is unnecessary to call do_munmap when old_len == new_len because
> nothing is actually done.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/mremap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/mremap.c b/mm/mremap.c
> index e776d4c2345c..dd966621a056 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -817,9 +817,9 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
>  			goto out;
>  	}
>  
> -	if (old_len >= new_len) {
> +	if (old_len > new_len) {
>  		ret = do_munmap(mm, addr+new_len, old_len - new_len, uf_unmap);
> -		if (ret && old_len != new_len)
> +		if (ret)
>  			goto out;
>  		old_len = new_len;
>  	}

I remember stumbling over that myself a year ago or so but dig not
deeper. But indeed, both variants (mmu, nommu) return -EINVAL in case
len (old_len - new_len) == 0.

Maybe that used to be different before ecc1a8993751 ("do_mremap()
untangling, part 2"), but it doesn't look like it.

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH] mm/mremap: avoid unneeded do_munmap call

[Miaohe Lin]

On 2022/4/1 18:38, David Hildenbrand wrote:
> On 01.04.22 10:10, Miaohe Lin wrote:
>> When old_len == new_len, do_munmap will return -EINVAL due to len == 0.
>> This errno will be simply ignored because of old_len != new_len check.
>> So it is unnecessary to call do_munmap when old_len == new_len because
>> nothing is actually done.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>  mm/mremap.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/mremap.c b/mm/mremap.c
>> index e776d4c2345c..dd966621a056 100644
>> --- a/mm/mremap.c
>> +++ b/mm/mremap.c
>> @@ -817,9 +817,9 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
>>  			goto out;
>>  	}
>>  
>> -	if (old_len >= new_len) {
>> +	if (old_len > new_len) {
>>  		ret = do_munmap(mm, addr+new_len, old_len - new_len, uf_unmap);
>> -		if (ret && old_len != new_len)
>> +		if (ret)
>>  			goto out;
>>  		old_len = new_len;
>>  	}
> 
> I remember stumbling over that myself a year ago or so but dig not
> deeper. But indeed, both variants (mmu, nommu) return -EINVAL in case
> len (old_len - new_len) == 0.
> 
> Maybe that used to be different before ecc1a8993751 ("do_mremap()
> untangling, part 2"), but it doesn't look like it.

It could be really hard to trace back to the reason. But it's clear that
this change does the same thing now and simplifies the code a bit.

> 
> Acked-by: David Hildenbrand <david@redhat.com>

Many thanks for your comment and Acked-by tag! Have a nice weekend! :)

>