| Message ID | 20220401081023.37080-1-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/mremap: avoid unneeded do_munmap call | expand |
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>
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! :) >
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; }
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(-)