From: Peter Xu <peterx@redhat.com>
To: David Hildenbrand <david@redhat.com>
Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>,
Andrew Morton <akpm@linux-foundation.org>,
kernel@collabora.com, Paul Gofman <pgofman@codeweavers.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 1/2] mm/userfaultfd: Support WP on multiple VMAs
Date: Thu, 16 Feb 2023 15:25:54 -0500 [thread overview]
Message-ID: <Y+6RUjv/K6RkwKQ7@x1n> (raw)
In-Reply-To: <f4b6cd03-28d7-b041-6dd4-46ac0812fc9b@redhat.com>
On Thu, Feb 16, 2023 at 10:37:36AM +0100, David Hildenbrand wrote:
> On 16.02.23 10:16, Muhammad Usama Anjum wrote:
> > mwriteprotect_range() errors out if [start, end) doesn't fall in one
> > VMA. We are facing a use case where multiple VMAs are present in one
> > range of interest. For example, the following pseudocode reproduces the
> > error which we are trying to fix:
> > - Allocate memory of size 16 pages with PROT_NONE with mmap
> > - Register userfaultfd
> > - Change protection of the first half (1 to 8 pages) of memory to
> > PROT_READ | PROT_WRITE. This breaks the memory area in two VMAs.
> > - Now UFFDIO_WRITEPROTECT_MODE_WP on the whole memory of 16 pages errors
> > out.
>
> I think, in QEMU, with partial madvise()/mmap(MAP_FIXED) while handling
> memory remapping during reboot to discard pages with memory errors, it would
> be possible that we get multiple VMAs and could not enable uffd-wp for
> background snapshots anymore. So this change makes sense to me.
Any pointer for this one?
>
> Especially, because userfaultfd_register() seems to already properly handle
> multi-VMA ranges correctly. It traverses the VMA list twice ... but also
> holds the mmap lock in write mode.
>
> >
> > This is a simple use case where user may or may not know if the memory
> > area has been divided into multiple VMAs.
> >
> > We need an implementation which doesn't disrupt the already present
> > users. So keeping things simple, stop going over all the VMAs if any one
> > of the VMA hasn't been registered in WP mode. While at it, remove the
> > un-needed error check as well.
> >
> > Reported-by: Paul Gofman <pgofman@codeweavers.com>
> > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> > ---
>
>
> Acked-by: David Hildenbrand <david@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
Thanks,
--
Peter Xu
next prev parent reply other threads:[~2023-02-16 20:26 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-16 9:16 [PATCH v4 1/2] mm/userfaultfd: Support WP on multiple VMAs Muhammad Usama Anjum
2023-02-16 9:16 ` [PATCH v4 2/2] mm/userfaultfd: add VM_WARN_ONCE() Muhammad Usama Anjum
2023-02-16 9:24 ` David Hildenbrand
2023-02-16 9:48 ` Muhammad Usama Anjum
2023-02-16 20:26 ` Peter Xu
2023-02-17 10:40 ` Muhammad Usama Anjum
2023-02-16 9:37 ` [PATCH v4 1/2] mm/userfaultfd: Support WP on multiple VMAs David Hildenbrand
2023-02-16 20:25 ` Peter Xu [this message]
2023-02-17 8:53 ` David Hildenbrand
2023-02-17 17:35 ` Peter Xu
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=Y+6RUjv/K6RkwKQ7@x1n \
--to=peterx@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=david@redhat.com \
--cc=kernel@collabora.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=pgofman@codeweavers.com \
--cc=usama.anjum@collabora.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.