From: Alistair Popple <apopple@nvidia.com>
To: Peter Xu <peterx@redhat.com>
Cc: <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>,
Andrea Arcangeli <aarcange@redhat.com>,
"Kirill A . Shutemov" <kirill@shutemov.name>,
Axel Rasmussen <axelrasmussen@google.com>,
Nadav Amit <nadav.amit@gmail.com>,
Hugh Dickins <hughd@google.com>,
Jerome Glisse <jglisse@redhat.com>,
Jason Gunthorpe <jgg@ziepe.ca>,
Andrew Morton <akpm@linux-foundation.org>,
Miaohe Lin <linmiaohe@huawei.com>,
Mike Rapoport <rppt@linux.vnet.ibm.com>,
Matthew Wilcox <willy@infradead.org>,
Mike Kravetz <mike.kravetz@oracle.com>
Subject: Re: [PATCH v3 06/27] shmem/userfaultfd: Handle uffd-wp special pte in page fault handler
Date: Thu, 17 Jun 2021 18:59:09 +1000 [thread overview]
Message-ID: <5906045.o7RC9TDvkT@nvdebian> (raw)
In-Reply-To: <20210527202122.30739-1-peterx@redhat.com>
On Friday, 28 May 2021 6:21:22 AM AEST Peter Xu wrote:
> File-backed memories are prone to unmap/swap so the ptes are always unstable.
> This could lead to userfaultfd-wp information got lost when unmapped or swapped
> out on such types of memory, for example, shmem. To keep such an information
> persistent, we will start to use the newly introduced swap-like special ptes to
> replace a null pte when those ptes were removed.
>
> Prepare this by handling such a special pte first before it is applied in the
> general page fault handler.
>
> The handling of this special pte page fault is similar to missing fault, but it
> should happen after the pte missing logic since the special pte is designed to
> be a swap-like pte. Meanwhile it should be handled before do_swap_page() so
> that the swap core logic won't be confused to see such an illegal swap pte.
>
> This is a slow path of uffd-wp handling, because unmap of wr-protected shmem
> ptes should be rare. So far it should only trigger in two conditions:
>
> (1) When trying to punch holes in shmem_fallocate(), there will be a
> pre-unmap optimization before evicting the page. That will create
> unmapped shmem ptes with wr-protected pages covered.
>
> (2) Swapping out of shmem pages
>
> Because of this, the page fault handling is simplifed too by not sending the
> wr-protect message in the 1st page fault, instead the page will be installed
> read-only, so the message will be generated until the next write, which will
> trigger the do_wp_page() path of general uffd-wp handling.
>
> Disable fault-around for all uffd-wp registered ranges for extra safety, and
> clean the code up a bit after we introduced MINOR fault.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> include/linux/userfaultfd_k.h | 12 +++++
> mm/memory.c | 88 +++++++++++++++++++++++++++++++----
> 2 files changed, 90 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> index 93f932b53a71..ca3f794d07e9 100644
> --- a/include/linux/userfaultfd_k.h
> +++ b/include/linux/userfaultfd_k.h
> @@ -94,6 +94,18 @@ static inline bool uffd_disable_huge_pmd_share(struct vm_area_struct *vma)
> return vma->vm_flags & (VM_UFFD_WP | VM_UFFD_MINOR);
> }
>
> +/*
> + * Don't do fault around for either WP or MINOR registered uffd range. For
> + * MINOR registered range, fault around will be a total disaster and ptes can
> + * be installed without notifications; for WP it should mostly be fine as long
> + * as the fault around checks for pte_none() before the installation, however
> + * to be super safe we just forbid it.
> + */
> +static inline bool uffd_disable_fault_around(struct vm_area_struct *vma)
> +{
> + return vma->vm_flags & (VM_UFFD_WP | VM_UFFD_MINOR);
> +}
> +
> static inline bool userfaultfd_missing(struct vm_area_struct *vma)
> {
> return vma->vm_flags & VM_UFFD_MISSING;
> diff --git a/mm/memory.c b/mm/memory.c
> index 2b24af4616df..45a2f71e447a 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3917,6 +3917,7 @@ vm_fault_t do_set_pmd(struct vm_fault *vmf, struct page *page)
> void do_set_pte(struct vm_fault *vmf, struct page *page, unsigned long addr)
> {
> struct vm_area_struct *vma = vmf->vma;
> + bool uffd_wp = pte_swp_uffd_wp_special(vmf->orig_pte);
> bool write = vmf->flags & FAULT_FLAG_WRITE;
> bool prefault = vmf->address != addr;
> pte_t entry;
> @@ -3929,6 +3930,8 @@ void do_set_pte(struct vm_fault *vmf, struct page *page, unsigned long addr)
>
> if (write)
> entry = maybe_mkwrite(pte_mkdirty(entry), vma);
> + if (unlikely(uffd_wp))
> + entry = pte_mkuffd_wp(pte_wrprotect(entry));
> /* copy-on-write page */
> if (write && !(vma->vm_flags & VM_SHARED)) {
> inc_mm_counter_fast(vma->vm_mm, MM_ANONPAGES);
> @@ -3996,8 +3999,12 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
> vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
> vmf->address, &vmf->ptl);
> ret = 0;
> - /* Re-check under ptl */
> - if (likely(pte_none(*vmf->pte)))
> +
> + /*
> + * Re-check under ptl. Note: this will cover both none pte and
> + * uffd-wp-special swap pte
> + */
> + if (likely(pte_same(*vmf->pte, vmf->orig_pte)))
> do_set_pte(vmf, page, vmf->address);
> else
> ret = VM_FAULT_NOPAGE;
> @@ -4101,9 +4108,21 @@ static vm_fault_t do_fault_around(struct vm_fault *vmf)
> return vmf->vma->vm_ops->map_pages(vmf, start_pgoff, end_pgoff);
> }
>
> +/* Return true if we should do read fault-around, false otherwise */
> +static inline bool should_fault_around(struct vm_fault *vmf)
> +{
> + /* No ->map_pages? No way to fault around... */
> + if (!vmf->vma->vm_ops->map_pages)
> + return false;
> +
> + if (uffd_disable_fault_around(vmf->vma))
> + return false;
> +
> + return fault_around_bytes >> PAGE_SHIFT > 1;
> +}
> +
> static vm_fault_t do_read_fault(struct vm_fault *vmf)
> {
> - struct vm_area_struct *vma = vmf->vma;
> vm_fault_t ret = 0;
>
> /*
> @@ -4111,12 +4130,10 @@ static vm_fault_t do_read_fault(struct vm_fault *vmf)
> * if page by the offset is not ready to be mapped (cold cache or
> * something).
> */
> - if (vma->vm_ops->map_pages && fault_around_bytes >> PAGE_SHIFT > 1) {
> - if (likely(!userfaultfd_minor(vmf->vma))) {
> - ret = do_fault_around(vmf);
> - if (ret)
> - return ret;
> - }
> + if (should_fault_around(vmf)) {
> + ret = do_fault_around(vmf);
> + if (ret)
> + return ret;
> }
>
> ret = __do_fault(vmf);
> @@ -4435,6 +4452,57 @@ static vm_fault_t wp_huge_pud(struct vm_fault *vmf, pud_t orig_pud)
> return VM_FAULT_FALLBACK;
> }
>
> +static vm_fault_t uffd_wp_clear_special(struct vm_fault *vmf)
> +{
> + vmf->pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd,
> + vmf->address, &vmf->ptl);
> + /*
> + * Be careful so that we will only recover a special uffd-wp pte into a
> + * none pte. Otherwise it means the pte could have changed, so retry.
> + */
> + if (pte_swp_uffd_wp_special(*vmf->pte))
> + pte_clear(vmf->vma->vm_mm, vmf->address, vmf->pte);
> + pte_unmap_unlock(vmf->pte, vmf->ptl);
> + return 0;
> +}
> +
> +/*
> + * This is actually a page-missing access, but with uffd-wp special pte
> + * installed. It means this pte was wr-protected before being unmapped.
> + */
> +static vm_fault_t uffd_wp_handle_special(struct vm_fault *vmf)
> +{
> + /* Careful! vmf->pte unmapped after return */
> + if (!pte_unmap_same(vmf))
> + return 0;
> +
> + /*
> + * Just in case there're leftover special ptes even after the region
> + * got unregistered - we can simply clear them.
> + */
> + if (unlikely(!userfaultfd_wp(vmf->vma) || vma_is_anonymous(vmf->vma)))
> + return uffd_wp_clear_special(vmf);
> +
> + /*
> + * Here we share most code with do_fault(), in which we can identify
> + * whether this is "none pte fault" or "uffd-wp-special fault" by
> + * checking the vmf->orig_pte.
> + */
> + return do_fault(vmf);
> +}
> +
> +static vm_fault_t do_swap_pte(struct vm_fault *vmf)
> +{
> + /*
> + * We need to handle special swap ptes before handling ptes that
> + * contain swap entries, always.
> + */
> + if (unlikely(pte_swp_uffd_wp_special(vmf->orig_pte)))
> + return uffd_wp_handle_special(vmf);
> +
> + return do_swap_page(vmf);
Probably pretty minor in the scheme of things but why not add this special
case directly to do_swap_page()? Your earlier "shmem/userfaultfd: Handle
uffd-wp special pte in page fault handler" adds this to do_swap_page()
anyway:
/*
* We should never call do_swap_page upon a swap special pte; just be
* safe to bail out if it happens.
*/
if (WARN_ON_ONCE(is_swap_special_pte(vmf->orig_pte)))
goto out;
So this patch could instead replace the warning with the call to
uffd_wp_handle_special(), which also means you can remove the extra
pte_unmap_same(vmf) check in uffd_wp_handle_special().
I suppose you might have to worry about other callers of do_swap_page(),
but the only other one I could see was __collapse_huge_page_swapin().
Initially I thought that might be able to trigger the warning here but I
see it checks pte_has_swap_entry() first which should skip it if it finds
the special pte.
- Alistair
> +}
> +
> /*
> * These routines also need to handle stuff like marking pages dirty
> * and/or accessed for architectures that don't do it in hardware (most
> @@ -4509,7 +4577,7 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
> }
>
> if (!pte_present(vmf->orig_pte))
> - return do_swap_page(vmf);
> + return do_swap_pte(vmf);
>
> if (pte_protnone(vmf->orig_pte) && vma_is_accessible(vmf->vma))
> return do_numa_page(vmf);
>
next prev parent reply other threads:[~2021-06-17 8:59 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-27 20:19 [PATCH v3 00/27] userfaultfd-wp: Support shmem and hugetlbfs Peter Xu
2021-05-27 20:19 ` [PATCH v3 01/27] mm/shmem: Unconditionally set pte dirty in mfill_atomic_install_pte Peter Xu
2021-05-27 20:19 ` [PATCH v3 02/27] shmem/userfaultfd: Take care of UFFDIO_COPY_MODE_WP Peter Xu
2021-05-27 20:19 ` [PATCH v3 03/27] mm: Clear vmf->pte after pte_unmap_same() returns Peter Xu
2021-05-27 20:19 ` [PATCH v3 04/27] mm/userfaultfd: Introduce special pte for unmapped file-backed mem Peter Xu
2021-05-28 8:32 ` Alistair Popple
2021-05-28 12:56 ` Peter Xu
2021-06-03 11:53 ` Alistair Popple
2021-06-03 14:51 ` Peter Xu
2021-06-04 0:55 ` Alistair Popple
2021-06-04 3:14 ` Hugh Dickins
2021-06-04 6:16 ` Alistair Popple
2021-06-04 16:01 ` Peter Xu
2021-06-08 13:18 ` Alistair Popple
2021-06-09 13:06 ` Alistair Popple
2021-06-09 14:43 ` Peter Xu
2021-05-27 20:21 ` [PATCH v3 05/27] mm/swap: Introduce the idea of special swap ptes Peter Xu
2021-05-27 20:21 ` [PATCH v3 06/27] shmem/userfaultfd: Handle uffd-wp special pte in page fault handler Peter Xu
2021-06-17 8:59 ` Alistair Popple [this message]
2021-06-17 15:10 ` Peter Xu
2021-05-27 20:21 ` [PATCH v3 07/27] mm: Drop first_index/last_index in zap_details Peter Xu
2021-06-21 12:20 ` Alistair Popple
2021-05-27 20:21 ` [PATCH v3 08/27] mm: Introduce zap_details.zap_flags Peter Xu
2021-06-21 12:09 ` Alistair Popple
2021-06-21 16:16 ` Peter Xu
2021-06-22 2:07 ` Alistair Popple
2021-05-27 20:21 ` [PATCH v3 09/27] mm: Introduce ZAP_FLAG_SKIP_SWAP Peter Xu
2021-06-21 12:36 ` Alistair Popple
2021-06-21 16:26 ` Peter Xu
2021-06-22 2:11 ` Alistair Popple
2021-05-27 20:21 ` [PATCH v3 10/27] mm: Pass zap_flags into unmap_mapping_pages() Peter Xu
2021-05-27 20:22 ` [PATCH v3 11/27] shmem/userfaultfd: Persist uffd-wp bit across zapping for file-backed Peter Xu
2021-06-21 8:41 ` Alistair Popple
2021-06-22 0:40 ` Peter Xu
2021-06-22 12:47 ` Alistair Popple
2021-06-22 15:44 ` Peter Xu
2021-06-23 6:04 ` Alistair Popple
2021-06-23 15:31 ` Peter Xu
2021-07-06 5:40 ` Alistair Popple
2021-07-06 15:35 ` Peter Xu
2021-07-08 2:49 ` Alistair Popple
2021-05-27 20:22 ` [PATCH v3 12/27] shmem/userfaultfd: Allow wr-protect none pte for file-backed mem Peter Xu
2021-05-27 20:22 ` [PATCH v3 13/27] shmem/userfaultfd: Allows file-back mem to be uffd wr-protected on thps Peter Xu
2021-05-27 20:22 ` [PATCH v3 14/27] shmem/userfaultfd: Handle the left-overed special swap ptes Peter Xu
2021-05-27 20:22 ` [PATCH v3 15/27] shmem/userfaultfd: Pass over uffd-wp special swap pte when fork() Peter Xu
2021-05-27 20:23 ` [PATCH v3 16/27] mm/hugetlb: Drop __unmap_hugepage_range definition from hugetlb.h Peter Xu
2021-05-27 20:23 ` [PATCH v3 17/27] mm/hugetlb: Introduce huge pte version of uffd-wp helpers Peter Xu
2021-05-27 20:23 ` [PATCH v3 18/27] hugetlb/userfaultfd: Hook page faults for uffd write protection Peter Xu
2021-05-27 20:23 ` [PATCH v3 19/27] hugetlb/userfaultfd: Take care of UFFDIO_COPY_MODE_WP Peter Xu
2021-05-27 20:23 ` [PATCH v3 20/27] hugetlb/userfaultfd: Handle UFFDIO_WRITEPROTECT Peter Xu
2021-05-27 20:23 ` [PATCH v3 21/27] mm/hugetlb: Introduce huge version of special swap pte helpers Peter Xu
2021-05-27 20:23 ` [PATCH v3 22/27] hugetlb/userfaultfd: Handle uffd-wp special pte in hugetlb pf handler Peter Xu
2021-05-27 20:23 ` [PATCH v3 23/27] hugetlb/userfaultfd: Allow wr-protect none ptes Peter Xu
2021-05-27 20:23 ` [PATCH v3 24/27] hugetlb/userfaultfd: Only drop uffd-wp special pte if required Peter Xu
2021-05-27 20:23 ` [PATCH v3 25/27] mm/pagemap: Recognize uffd-wp bit for shmem/hugetlbfs Peter Xu
2021-05-27 20:23 ` [PATCH v3 26/27] mm/userfaultfd: Enable write protection for shmem & hugetlbfs Peter Xu
2021-05-27 20:23 ` [PATCH v3 27/27] userfaultfd/selftests: Enable uffd-wp for shmem/hugetlbfs Peter Xu
2021-06-02 14:40 ` [PATCH v3 00/27] userfaultfd-wp: Support shmem and hugetlbfs Peter Xu
2021-06-02 22:36 ` Andrew Morton
2021-06-03 0:09 ` 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=5906045.o7RC9TDvkT@nvdebian \
--to=apopple@nvidia.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=axelrasmussen@google.com \
--cc=hughd@google.com \
--cc=jgg@ziepe.ca \
--cc=jglisse@redhat.com \
--cc=kirill@shutemov.name \
--cc=linmiaohe@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mike.kravetz@oracle.com \
--cc=nadav.amit@gmail.com \
--cc=peterx@redhat.com \
--cc=rppt@linux.vnet.ibm.com \
--cc=willy@infradead.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;
as well as URLs for NNTP newsgroup(s).