[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADyq12zUEq9kcyuR_Qm9MrU1ii-+9n8T2hK6QNzj=kH5zn0VrA@mail.gmail.com>
Date: Thu, 20 Feb 2020 10:36:38 -0800
From: Brian Geffon <bgeffon@...gle.com>
To: Minchan Kim <minchan@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
"Michael S . Tsirkin" <mst@...hat.com>,
Arnd Bergmann <arnd@...db.de>,
LKML <linux-kernel@...r.kernel.org>,
linux-mm <linux-mm@...ck.org>,
Linux API <linux-api@...r.kernel.org>,
Andy Lutomirski <luto@...capital.net>,
Will Deacon <will@...nel.org>,
Andrea Arcangeli <aarcange@...hat.com>,
Sonny Rao <sonnyrao@...gle.com>,
Joel Fernandes <joel@...lfernandes.org>,
Yu Zhao <yuzhao@...gle.com>,
Jesse Barnes <jsbarnes@...gle.com>,
Florian Weimer <fweimer@...hat.com>,
"Kirill A . Shutemov" <kirill@...temov.name>
Subject: Re: [PATCH v6 1/2] mm: Add MREMAP_DONTUNMAP to mremap().
Hi Minchan,
> And here we got error if the addr is in non-anonymous-private vma so the
> syscall will fail but old vma is gone? I guess it's not your intention?
This is exactly what happens today in several situations, because
vma_to_resize is called unconditionally. For example if the old vma
has VM_HUGETLB and old_len < new_len it would have unmapped a portion
and then in vma_to_resize returned -EINVAL, similarly when old_len = 0
with a non-sharable mapping it will have called do_munmap only to fail
in vma_to_resize, if the vma has VM_DONTEXPAND set and you shrink the
size with old_len < new_len it would return -EFAULT after having done
the unmap on the decreased portion. So I followed the pattern to keep
the change simple and maintain consistency with existing behavior.
But with that being said, Kirill made the point that resizing a VMA
while also using MREMAP_DONTUNMAP doesn't have any clear use case and
I agree with that, I'm unable to think of a situation where you'd want
to resize a VMA and use MREMAP_DONTUNMAP. So I'm tempted to mail a new
version which returns -EINVAL if old_len != new_len that would resolve
this concern here as nothing would be unmapped ever at the old
position add it would clean up the change to very few lines of code.
What do you think?
Thank you for taking the time to review.
Brian
Powered by blists - more mailing lists