linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Brian Geffon <bgeffon@google.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	 Arnd Bergmann <arnd@arndb.de>,
	LKML <linux-kernel@vger.kernel.org>,
	 linux-mm <linux-mm@kvack.org>,
	Linux API <linux-api@vger.kernel.org>,
	 Andy Lutomirski <luto@amacapital.net>,
	Will Deacon <will@kernel.org>,
	 Andrea Arcangeli <aarcange@redhat.com>,
	Sonny Rao <sonnyrao@google.com>,
	 Joel Fernandes <joel@joelfernandes.org>,
	Yu Zhao <yuzhao@google.com>,  Jesse Barnes <jsbarnes@google.com>,
	Florian Weimer <fweimer@redhat.com>,
	 "Kirill A . Shutemov" <kirill@shutemov.name>
Subject: Re: [PATCH v6 1/2] mm: Add MREMAP_DONTUNMAP to mremap().
Date: Thu, 20 Feb 2020 10:36:38 -0800	[thread overview]
Message-ID: <CADyq12zUEq9kcyuR_Qm9MrU1ii-+9n8T2hK6QNzj=kH5zn0VrA@mail.gmail.com> (raw)
In-Reply-To: <20200220171554.GA44866@google.com>

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


  reply	other threads:[~2020-02-20 18:37 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-18 17:32 [PATCH v6 1/2] mm: Add MREMAP_DONTUNMAP to mremap() Brian Geffon
2020-02-18 17:32 ` [PATCH v6 2/2] selftest: Add MREMAP_DONTUNMAP selftest Brian Geffon
2020-02-19 20:39 ` [PATCH v6 1/2] mm: Add MREMAP_DONTUNMAP to mremap() Andrew Morton
2020-02-19 21:38   ` Lokesh Gidra
2020-02-19 23:23 ` Minchan Kim
2020-02-20 11:57 ` Kirill A. Shutemov
2020-02-20 23:55   ` Brian Geffon
2020-02-21 12:23     ` Kirill A. Shutemov
2020-02-20 17:15 ` Minchan Kim
2020-02-20 18:36   ` Brian Geffon [this message]
2020-02-20 18:45     ` Brian Geffon
2020-02-20 19:14     ` Minchan Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CADyq12zUEq9kcyuR_Qm9MrU1ii-+9n8T2hK6QNzj=kH5zn0VrA@mail.gmail.com' \
    --to=bgeffon@google.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=fweimer@redhat.com \
    --cc=joel@joelfernandes.org \
    --cc=jsbarnes@google.com \
    --cc=kirill@shutemov.name \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@amacapital.net \
    --cc=minchan@kernel.org \
    --cc=mst@redhat.com \
    --cc=sonnyrao@google.com \
    --cc=will@kernel.org \
    --cc=yuzhao@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).