From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
To: Dev Jain <dev.jain@arm.com>
Cc: akpm@linux-foundation.org, axelrasmussen@google.com,
yuanchu@google.com, weixugc@google.com, david@kernel.org,
Liam.Howlett@oracle.com, vbabka@suse.cz, rppt@kernel.org,
surenb@google.com, mhocko@suse.com, riel@surriel.com,
harry.yoo@oracle.com, jannh@google.com, ryan.roberts@arm.com,
baohua@kernel.org, baolin.wang@linux.alibaba.com,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH mm-unstable] mm: Fix uffd-wp bit loss when batching file folio unmapping
Date: Fri, 16 Jan 2026 08:39:04 +0000 [thread overview]
Message-ID: <343f8462-44fd-4190-9fe2-8120b5403269@lucifer.local> (raw)
In-Reply-To: <20260116082721.275178-1-dev.jain@arm.com>
On Fri, Jan 16, 2026 at 01:57:21PM +0530, Dev Jain wrote:
> The recently added file folio unmap batching support forgets to update
> pte_install_uffd_wp_if_needed(), which still updates a single pte.
> We end up jumping to the end of the folio in page_vma_mapped_walk(), thus
> setting the uffd-wp marker only on a single pte in the batch. Fix this by
> passing nr_pages into the function, and set the uffd-wp marker on all ptes.
>
> Note that, since the nr_pages passed to this function is always derived by
> some sort of batching, it is guaranteed that the set of old ptevals of the
> batch have uffd-wp bit on all ptes or no ptes, therefore it is safe to derive
> the value of the local variable "arm_uffd_pte" from only the particular
> pteval passed to this function, but apply the result on all ptes of the batch.
>
> Use set_pte_at() in a loop to set the markers - we cannot use set_ptes()
> as that will increment the PFN, but we don't have any PFN to update here.
>
> The userspace visible effect of the bug is inaccuracy observed by workloads
> relying on uffd-wp regions to install their own pages.
>
> Fixes: 8798e255b5ec ("mm: rmap: support batched unmapping for file large folios")
Hmm this patch isn't upstream yet, so wouldn't this patch be better as a comment
replying to the series so it can be respun?
You only do a fixes tag for either upstream things that obviously have to stay
in place or perhaps things in mm-stable immediately prior to merge window that
can't be adjusted.
> Signed-off-by: Dev Jain <dev.jain@arm.com>
Let's replace this with a comment on the series at [0] please.
[0]: https://lore.kernel.org/linux-mm/cover.1766631066.git.baolin.wang@linux.alibaba.com/
Also the original series really shouldn't go in without signoff from me/David.
Thanks, Lorenzo
> ---
> Patch applies on mm-unstable, commit f8ed52ac0cfb.
>
> I observed this bug during code inspection, but it turns out that the uffd-wp-mremap
> selftest will skip some tests with a bogus complain that "MADV_PAGEOUT didn't work,
> is swap enabled?" even when swap is enabled. It first sets the region uffd-wp,
> then swaps it out, then checks through pagemap whether it got swapped out. For
> file folios, this check makes no sense since the ptes are simply cleared, but in
> this particular case, because of uffd-wp preservation, we need to store PTE_MARKER_UFFD_WP,
> which is stored as a swap entry, that is why the test works out on a non-buggy kernel.
>
> include/linux/mm_inline.h | 7 ++++---
> mm/memory.c | 14 +-------------
> mm/rmap.c | 2 +-
> 3 files changed, 6 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> index fa2d6ba811b5..adec1dcb8793 100644
> --- a/include/linux/mm_inline.h
> +++ b/include/linux/mm_inline.h
> @@ -568,7 +568,7 @@ static inline pte_marker copy_pte_marker(
> */
> static inline bool
> pte_install_uffd_wp_if_needed(struct vm_area_struct *vma, unsigned long addr,
> - pte_t *pte, pte_t pteval)
> + pte_t *pte, pte_t pteval, unsigned int nr_pages)
> {
> bool arm_uffd_pte = false;
>
> @@ -599,8 +599,9 @@ pte_install_uffd_wp_if_needed(struct vm_area_struct *vma, unsigned long addr,
> arm_uffd_pte = true;
>
> if (unlikely(arm_uffd_pte)) {
> - set_pte_at(vma->vm_mm, addr, pte,
> - make_pte_marker(PTE_MARKER_UFFD_WP));
> + for (int i = 0; i < nr_pages; ++i, ++pte, addr += PAGE_SIZE)
> + set_pte_at(vma->vm_mm, addr, pte,
> + make_pte_marker(PTE_MARKER_UFFD_WP));
> return true;
> }
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 6b22dd72ebc8..35ac86d29e77 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1588,8 +1588,6 @@ zap_install_uffd_wp_if_needed(struct vm_area_struct *vma,
> unsigned long addr, pte_t *pte, int nr,
> struct zap_details *details, pte_t pteval)
> {
> - bool was_installed = false;
> -
> if (!uffd_supports_wp_marker())
> return false;
>
> @@ -1600,17 +1598,7 @@ zap_install_uffd_wp_if_needed(struct vm_area_struct *vma,
> if (zap_drop_markers(details))
> return false;
>
> - for (;;) {
> - /* the PFN in the PTE is irrelevant. */
> - if (pte_install_uffd_wp_if_needed(vma, addr, pte, pteval))
> - was_installed = true;
> - if (--nr == 0)
> - break;
> - pte++;
> - addr += PAGE_SIZE;
> - }
> -
> - return was_installed;
> + return pte_install_uffd_wp_if_needed(vma, addr, pte, pteval, nr);
> }
>
> static __always_inline void zap_present_folio_ptes(struct mmu_gather *tlb,
> diff --git a/mm/rmap.c b/mm/rmap.c
> index f13480cb9f2e..d6ca002bf79c 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -2171,7 +2171,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> * we may want to replace a none pte with a marker pte if
> * it's file-backed, so we don't lose the tracking info.
> */
> - pte_install_uffd_wp_if_needed(vma, address, pvmw.pte, pteval);
> + pte_install_uffd_wp_if_needed(vma, address, pvmw.pte, pteval, nr_pages);
>
> /* Update high watermark before we lower rss */
> update_hiwater_rss(mm);
> --
> 2.34.1
>
next prev parent reply other threads:[~2026-01-16 8:39 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-16 8:27 Dev Jain
2026-01-16 8:39 ` Lorenzo Stoakes [this message]
2026-01-16 9:40 ` Dev Jain
2026-01-16 9:48 ` Lorenzo Stoakes
2026-01-16 9:54 ` Dev Jain
2026-01-16 10:51 ` David Hildenbrand (Red Hat)
2026-01-17 17:46 ` Andrew Morton
2026-01-16 10:57 ` David Hildenbrand (Red Hat)
2026-01-16 11:12 ` Lorenzo Stoakes
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=343f8462-44fd-4190-9fe2-8120b5403269@lucifer.local \
--to=lorenzo.stoakes@oracle.com \
--cc=Liam.Howlett@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=axelrasmussen@google.com \
--cc=baohua@kernel.org \
--cc=baolin.wang@linux.alibaba.com \
--cc=david@kernel.org \
--cc=dev.jain@arm.com \
--cc=harry.yoo@oracle.com \
--cc=jannh@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=riel@surriel.com \
--cc=rppt@kernel.org \
--cc=ryan.roberts@arm.com \
--cc=surenb@google.com \
--cc=vbabka@suse.cz \
--cc=weixugc@google.com \
--cc=yuanchu@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