[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADyq12y2uJh2ROBajA1D5H6OLEyFrKKEXH=yQC-8ELF0+UiNzw@mail.gmail.com>
Date: Thu, 20 Feb 2020 15:55:53 -0800
From: Brian Geffon <bgeffon@...gle.com>
To: "Kirill A. Shutemov" <kirill@...temov.name>
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>,
Minchan Kim <minchan@...nel.org>,
Joel Fernandes <joel@...lfernandes.org>,
Yu Zhao <yuzhao@...gle.com>,
Jesse Barnes <jsbarnes@...gle.com>,
Florian Weimer <fweimer@...hat.com>
Subject: Re: [PATCH v6 1/2] mm: Add MREMAP_DONTUNMAP to mremap().
Hi Kirill,
> I have hard time understanding the case when new_len != old_len.
>
> Correct me if I'm wrong, but looks like that you change the size of old
> mapping to be the new_len and then create a new of the same new_len.
>
> This doesn't look right to me.
>
> In my opinion, MREMAP_DONTUNMAP has to leave the old mapping intact. And
> create the new mapping adjusted to the new_len.
>
> Other option is to force new_len == old_len if MREMAP_DONTUNMAP is
> specified. It would simplify the implementation. And I don't see why
> anybody would really want anything else.
I had been approaching this as, "do what mremap would have done in
this situation except skip the last step." Meaning, whatever the final
state of the old mapping was MREMAP_DONTUNMAP meant that you should
just not do the unmap operation on the old mapping at the end. But I
understand why it's confusing, especially when in the case of the VMA
growing you're left with the old vma of size old_len and the new_vma
of size new_len but only containing old_len worth of pages.
Personally, I don't think this is a problem having that behavior
because it can be documented and it just adds a small amount of
flexibility.
Nonetheless, I agree with you and I also cannot come up with a
situation where you'd actually want to do this so I'm willing to
restrict it to old_len == new_len and return -EINVAL if not, it
simplifies it a bit and accounting becomes a easier because the
outcome is always the same two mappings of size old_len and the size
of the locked_vm never changes. We can always allow the resize
operation later if there becomes a need. If everyone is okay with this
restriction I can send a new patch.
Thank you again,
Brian
Powered by blists - more mailing lists