From: Peter Xu <peterx@redhat.com>
To: David Hildenbrand <david@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Ives van Hoorne <ives@codesandbox.io>,
stable@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>,
Hugh Dickins <hugh@veritas.com>,
Alistair Popple <apopple@nvidia.com>,
Mike Rapoport <rppt@linux.vnet.ibm.com>,
Nadav Amit <nadav.amit@gmail.com>,
Andrea Arcangeli <aarcange@redhat.com>
Subject: Re: [PATCH RFC] mm/userfaultfd: enable writenotify while userfaultfd-wp is enabled for a VMA
Date: Tue, 6 Dec 2022 16:27:31 -0500 [thread overview]
Message-ID: <Y4+zw4JU7JMlDHbM@x1n> (raw)
In-Reply-To: <92173bad-caa3-6b43-9d1e-9a471fdbc184@redhat.com>
On Tue, Dec 06, 2022 at 05:28:07PM +0100, David Hildenbrand wrote:
> > If no one is using mprotect() with uffd-wp like that, then the reproducer
> > may not be valid - the reproducer is defining how it should work, but does
> > that really stand? That's why I said it's ambiguous, because the
> > definition in this case is unclear.
>
> There are interesting variations like:
>
> mmap(PROT_READ, MAP_POPULATE|MAP_SHARED)
> uffd_wp()
> mprotect(PROT_READ|PROT_WRITE)
>
> Where we start out with all-write permissions before we enable selective
> write permissions.
Could you elaborate what's the difference of above comparing to:
mmap(PROT_READ|PROT_WRITE, MAP_POPULATE|MAP_SHARED)
uffd_wp()
?
[...]
> Yes, you are correct. I added that to the patch description:
>
> "
> Note that we don't optimize for the actual migration case:
> (1) When migration succeeds the new PTE will not be writable because
> the source PTE was not writable (protnone); in the future we
> might just optimize that case similarly by reusing
> can_change_pte_writable()/can_change_pmd_writable() when
> removing migration PTEs.
> (2) When migration fails, we'd have to recalculate the "writable"
> flag because we temporarily dropped the PT lock; for now keep it
> simple and set "writable=false".
> "
>
> Case (1) would, with your current patch, always lose the write bit during
> migration, even if vma->vm_page_prot included it. We most might want to
> optimize that in the future.
>
> Case (2) is rather a corner case, and unless people complain about it being
> a real performance issue, it felt cleaner (less code) to not optimize for
> that now.
As I didn't have a closer look on the savedwrite removal patchset so I may
not speak anything sensible here.. What I hope is that we don't lose write
bits easily, after all we tried to even safe the dirty and young bits to
avoid the machine cycles in the MMUs.
>
> Again Peter, I am not against you, not at all. Sorry if I gave you the
> impression. I highly appreciate your work and this discussion.
No worry on that part. You're doing great in this email explaining things
and write things up, especially I'm happy Hugh confirmed it so it's good to
have those. Let's start with something like this when you NAK something
next time. :)
Thanks,
--
Peter Xu
next prev parent reply other threads:[~2022-12-06 21:27 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-02 12:27 David Hildenbrand
2022-12-02 16:33 ` Peter Xu
2022-12-02 16:56 ` David Hildenbrand
2022-12-02 17:11 ` David Hildenbrand
2022-12-05 21:08 ` Peter Xu
2022-12-06 0:46 ` [PATCH] mm/uffd: Always wr-protect pte in pte_mkuffd_wp() kernel test robot
2022-12-06 16:21 ` Peter Xu
2022-12-06 11:43 ` kernel test robot
2022-12-06 16:28 ` [PATCH RFC] mm/userfaultfd: enable writenotify while userfaultfd-wp is enabled for a VMA David Hildenbrand
2022-12-06 19:09 ` Hugh Dickins
2022-12-06 21:18 ` Peter Xu
2022-12-07 15:32 ` David Hildenbrand
2022-12-07 17:43 ` Peter Xu
2022-12-07 19:53 ` David Hildenbrand
2022-12-07 20:14 ` Peter Xu
2022-12-06 21:27 ` Peter Xu [this message]
2022-12-07 13:33 ` David Hildenbrand
2022-12-07 15:59 ` Peter Xu
2022-12-07 20:10 ` David Hildenbrand
2022-12-08 15:17 ` Peter Xu
2022-12-06 18:38 ` [PATCH] mm/uffd: Always wr-protect pte in pte_mkuffd_wp() kernel test robot
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=Y4+zw4JU7JMlDHbM@x1n \
--to=peterx@redhat.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=apopple@nvidia.com \
--cc=david@redhat.com \
--cc=hugh@veritas.com \
--cc=ives@codesandbox.io \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=nadav.amit@gmail.com \
--cc=rppt@linux.vnet.ibm.com \
--cc=stable@vger.kernel.org \
/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