[PATCH] mm/rmap: don't discard lazyfree THPs at PMD level

[PATCH] mm/rmap: don't discard lazyfree THPs at PMD level

[Zi Yan]

__discard_anon_folio_pmd_locked() clears a lazyfree THP PMD before it knows
whether the folio can be discarded, and restores it if the folio was
redirtied or has extra references. A concurrent munmap() or
MREMAP_DONTUNMAP skips the temporary none PMD and unlinks the VMA from its
anon_vma, so the folio stays mapped after the anon_vma is freed and a later
rmap walk uses the freed anon_vma.

Using an invalidated PMD instead of a cleared one requires additional arch
code fixes. Instead, disable the PMD level discard of lazyfree THPs, as
before commit 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during
shrink_folio_list()").

Fixes: 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during shrink_folio_list()")
Reported-by: Kyle Zeng <kylebot@openai.com>
Closes: https://lore.kernel.org/r/20261009165214.40212-2-kylebot@openai.com
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
The proposed fix[1] requires additional arch code fixes[2]. Disable it
first for stable fix. The function will be re-enabled along with arch code
fixes.

Link: https://lore.kernel.org/all/20261009165214.40212-2-kylebot@openai.com/ [1]
Link: https://lore.kernel.org/all/DM0II2QHWWS1.2RXEJXC8S0101@nvidia.com/ [2]
---
 mm/rmap.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index 805db93fe0428..1131b76bbbc28 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -2275,17 +2275,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
 		}
 
 		if (!pvmw.pte) {
-			if (folio_test_lazyfree(folio)) {
-				if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, folio))
-					goto walk_done;
-				/*
-				 * unmap_huge_pmd_locked has either already marked
-				 * the folio as swap-backed or decided to retain it
-				 * due to GUP or speculative references.
-				 */
-				goto walk_abort;
-			}
-
 			if (flags & TTU_SPLIT_HUGE_PMD) {
 				/*
 				 * We temporarily have to drop the PTL and

---
base-commit: 8b38ed9ab5b09c8ba168cbcc49524e9b380ee5c4
change-id: 20261009-lazyfree-stable-fix-67712b2049dd

Best regards,
--  
Yan, Zi

Re: [PATCH] mm/rmap: don't discard lazyfree THPs at PMD level

[Kyle Zeng]

On Fri, Oct 09, 2026 at 10:08:18PM -0400, Zi Yan wrote:
> __discard_anon_folio_pmd_locked() clears a lazyfree THP PMD before it knows
> whether the folio can be discarded, and restores it if the folio was
> redirtied or has extra references. A concurrent munmap() or
> MREMAP_DONTUNMAP skips the temporary none PMD and unlinks the VMA from its
> anon_vma, so the folio stays mapped after the anon_vma is freed and a later
> rmap walk uses the freed anon_vma.
> 
> Using an invalidated PMD instead of a cleared one requires additional arch
> code fixes. Instead, disable the PMD level discard of lazyfree THPs, as
> before commit 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during
> shrink_folio_list()").
> 
> Fixes: 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during shrink_folio_list()")
> Reported-by: Kyle Zeng <kylebot@openai.com>
> Closes: https://lore.kernel.org/r/20261009165214.40212-2-kylebot@openai.com
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
> The proposed fix[1] requires additional arch code fixes[2]. Disable it
> first for stable fix. The function will be re-enabled along with arch code
> fixes.
> 
> Link: https://lore.kernel.org/all/20261009165214.40212-2-kylebot@openai.com/ [1]
> Link: https://lore.kernel.org/all/DM0II2QHWWS1.2RXEJXC8S0101@nvidia.com/ [2]
> ---
>  mm/rmap.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 805db93fe0428..1131b76bbbc28 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -2275,17 +2275,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>  		}
>  
>  		if (!pvmw.pte) {
> -			if (folio_test_lazyfree(folio)) {
> -				if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, folio))
> -					goto walk_done;
> -				/*
> -				 * unmap_huge_pmd_locked has either already marked
> -				 * the folio as swap-backed or decided to retain it
> -				 * due to GUP or speculative references.
> -				 */
> -				goto walk_abort;
> -			}
> -
>  			if (flags & TTU_SPLIT_HUGE_PMD) {
>  				/*
>  				 * We temporarily have to drop the PTL and
> 
> ---
> base-commit: 8b38ed9ab5b09c8ba168cbcc49524e9b380ee5c4
> change-id: 20261009-lazyfree-stable-fix-67712b2049dd
> 
> Best regards,
> --  
> Yan, Zi
> 

Reviewed-by: Kyle Zeng <kylebot@openai.com>

Re: [PATCH] mm/rmap: don't discard lazyfree THPs at PMD level

[Lance Yang]

On 2026/10/10 10:08, Zi Yan wrote:
> __discard_anon_folio_pmd_locked() clears a lazyfree THP PMD before it knows
> whether the folio can be discarded, and restores it if the folio was
> redirtied or has extra references. A concurrent munmap() or
> MREMAP_DONTUNMAP skips the temporary none PMD and unlinks the VMA from its
> anon_vma, so the folio stays mapped after the anon_vma is freed and a later
> rmap walk uses the freed anon_vma.
> 
> Using an invalidated PMD instead of a cleared one requires additional arch
> code fixes. Instead, disable the PMD level discard of lazyfree THPs, as
> before commit 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during
> shrink_folio_list()").
> 
> Fixes: 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during shrink_folio_list()")
> Reported-by: Kyle Zeng <kylebot@openai.com>
> Closes: https://lore.kernel.org/r/20261009165214.40212-2-kylebot@openai.com
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---

LGTM!

Reviewed-by: Lance Yang <lance.yang@linux.dev>

Re: [PATCH] mm/rmap: don't discard lazyfree THPs at PMD level

[David Hildenbrand (Arm)]

On 10/10/26 04:08, Zi Yan wrote:
> __discard_anon_folio_pmd_locked() clears a lazyfree THP PMD before it knows
> whether the folio can be discarded, and restores it if the folio was
> redirtied or has extra references. A concurrent munmap() or
> MREMAP_DONTUNMAP skips the temporary none PMD and unlinks the VMA from its
> anon_vma, so the folio stays mapped after the anon_vma is freed and a later
> rmap walk uses the freed anon_vma.
> 
> Using an invalidated PMD instead of a cleared one requires additional arch
> code fixes. Instead, disable the PMD level discard of lazyfree THPs, as
> before commit 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during
> shrink_folio_list()").
> 

Maybe mention here that we leave the infrastructure around to re-enable it once
fixes?

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH] mm/rmap: don't discard lazyfree THPs at PMD level

[Lorenzo Stoakes (ARM)]

On Fri, Oct 09, 2026 at 10:08:18PM -0400, Zi Yan wrote:
> __discard_anon_folio_pmd_locked() clears a lazyfree THP PMD before it knows
> whether the folio can be discarded, and restores it if the folio was
> redirtied or has extra references. A concurrent munmap() or
> MREMAP_DONTUNMAP skips the temporary none PMD and unlinks the VMA from its
> anon_vma, so the folio stays mapped after the anon_vma is freed and a later

Yeah this seems pretty horrible, instating a page table entry under the rmap
lock is pretty gross in general, is there anywhere else where we actually
_install_ (even if 'reinstate') page table entries under the rmap lock?

I think this commit message needs to be clearer/more detailed though, I've been
sat here working this through in my head for a while.

I'm confused about the locking, which is really the key point about this change
isn't it?

unlink_anon_vmas() takes the anon rmap lock so that can't happen concurrently,
so is it _just_ the case that:

- The folio is not properly freed/unmapped and ends up with a dangling anon_vma
  pointer

In which case, nobody can actually access that folio since it's not mapped
anywhere, or is it the far more terrible case of the above and:

- set_pmd_at() in __discard_anon_folio_pmd_locked() reinstates the pmd entry
  regardless of the fact that the VMA and surrounding page tables could go away
  concurrently

?

But I'm confused here because unlink_anon_vmas() takes the anon_vma lock?

EDIT: See below, I figured out what it was.

Looking at the code:

<anon_vma lock>
[reclaim]
< PMD lock >
-> try_to_unmap_one()
    -> unmap_huge_pmd_locked()
      -> __discard_anon_folio_pmd_locked()
        [ clears PMD entry ]
	...
	[ reinstates PMD entry later ]

Concurrent munmap():

<mmap write lock, downgraded to mmap read lock later>
do_vmi_align_munmap()
  -> vms_complete_munmap_vmas()
    -> vms_clear_ptes()
      -> unmap_region()
        -> unmap_vmas() < zaps > <----- EDIT: the real issue, see below
	-> free_pgtables()
	   -> unlink_anon_vmas()
	     -> < anon_vma lock taken! >
	   ... clears down page tables (no rmap lock) ...

Is the issue actually the zapping in unmap_vmas(), which doesn't seem to be
protected by rmap lock at all, meaning that _that_ can happen with
concurrent munmap()?

So unmap_vmas() doesn't correctly populate the mmu_gather struct for the
range, meaning that the underlying page isn't freed and the refcount,
mapcount etc. are all left as they were?

MREMAP_DONTUNMAP seems a bit of a bizarre callout, but I suppose it is the
other case in which (under certain, likely, circumstances
e.g. non-partial/self-merging remap) an unlink_anon_vmas()'s call is done.

With normal mremap() that'd happen via an unmap of the original VMA.

> rmap walk uses the freed anon_vma.
>
> Using an invalidated PMD instead of a cleared one requires additional arch
> code fixes. Instead, disable the PMD level discard of lazyfree THPs, as
> before commit 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during
> shrink_folio_list()").

Again it feels like this commit message assumes a bunch of information that
isn't presented here.

Why would you need to invalidate the PMD? Is the issue actually TLB flush?
How does that get around the fact that a PMD entry can be 'reinstated'
concurrent with a zap?

Or is it that you put something not-pmd_none() there? Hmm yeah I think so.

Doesn't all this cause pmd_none_or_clear_bad() to fire too?

All-round terrible.

I've not dug into the reports in detail, but I think the commit message
should also talk about what exactly is missing from pmdp_invalidate() that
is broken here? Are some arches not doing things quite right?

OK reading the code more and it seems the _real_ issue is actually
pmd_is_huge() in zap_pmd_range() right?

It's the -> pmd_none() in general. By doing that, you are breaking a
contract with PMD leaf entries.

So you get:

zap_pmd_range()
  -> pmd_is_huge() ? [ no ]
     -> zap_huge_pmd() [ WOULD HAVE taken PMD lock that blocks against reclaim ]

So using something like pmdp_invalidate() avoids this, because it keeps the
entry huge but invalid.

But I guess not every arch does this properly somehow?

>
> Fixes: 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during shrink_folio_list()")
> Reported-by: Kyle Zeng <kylebot@openai.com>
> Closes: https://lore.kernel.org/r/20261009165214.40212-2-kylebot@openai.com
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Zi Yan <ziy@nvidia.com>

With the commit message made a lot clearer and calling out the specific
locking scenarios that allow this + getting rid of the dead code:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
> The proposed fix[1] requires additional arch code fixes[2]. Disable it
> first for stable fix. The function will be re-enabled along with arch code
> fixes.
>
> Link: https://lore.kernel.org/all/20261009165214.40212-2-kylebot@openai.com/ [1]
> Link: https://lore.kernel.org/all/DM0II2QHWWS1.2RXEJXC8S0101@nvidia.com/ [2]
> ---
>  mm/rmap.c | 11 -----------
>  1 file changed, 11 deletions(-)
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 805db93fe0428..1131b76bbbc28 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -2275,17 +2275,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>  		}
>
>  		if (!pvmw.pte) {
> -			if (folio_test_lazyfree(folio)) {
> -				if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, folio))

This is the only caller of unmap_huge_pmd_locked(), making that and
__discard_anon_folio_pmd_locked() dead code, shouldn't we remove all of it?

Otherwise we're leaving dead code around.


> -					goto walk_done;
> -				/*
> -				 * unmap_huge_pmd_locked has either already marked
> -				 * the folio as swap-backed or decided to retain it
> -				 * due to GUP or speculative references.
> -				 */
> -				goto walk_abort;
> -			}
> -
>  			if (flags & TTU_SPLIT_HUGE_PMD) {
>  				/*
>  				 * We temporarily have to drop the PTL and
>
> ---
> base-commit: 8b38ed9ab5b09c8ba168cbcc49524e9b380ee5c4
> change-id: 20261009-lazyfree-stable-fix-67712b2049dd
>
> Best regards,
> --
> Yan, Zi
>

--
Cheers, Lorenzo

Re: [PATCH] mm/rmap: don't discard lazyfree THPs at PMD level

[Zi Yan]

On 12 Oct 2026, at 8:07, Lorenzo Stoakes (ARM) wrote:

> On Fri, Oct 09, 2026 at 10:08:18PM -0400, Zi Yan wrote:
>> __discard_anon_folio_pmd_locked() clears a lazyfree THP PMD before it knows
>> whether the folio can be discarded, and restores it if the folio was
>> redirtied or has extra references. A concurrent munmap() or
>> MREMAP_DONTUNMAP skips the temporary none PMD and unlinks the VMA from its
>> anon_vma, so the folio stays mapped after the anon_vma is freed and a later
>
> Yeah this seems pretty horrible, instating a page table entry under the rmap
> lock is pretty gross in general, is there anywhere else where we actually
> _install_ (even if 'reinstate') page table entries under the rmap lock?
>
> I think this commit message needs to be clearer/more detailed though, I've been
> sat here working this through in my head for a while.

Sorry for being vague here, let me clarify it below.

>
> I'm confused about the locking, which is really the key point about this change
> isn't it?
>
> unlink_anon_vmas() takes the anon rmap lock so that can't happen concurrently,
> so is it _just_ the case that:
>
> - The folio is not properly freed/unmapped and ends up with a dangling anon_vma
>   pointer
>
> In which case, nobody can actually access that folio since it's not mapped
> anywhere, or is it the far more terrible case of the above and:
>
> - set_pmd_at() in __discard_anon_folio_pmd_locked() reinstates the pmd entry
>   regardless of the fact that the VMA and surrounding page tables could go away
>   concurrently
>
> ?
>
> But I'm confused here because unlink_anon_vmas() takes the anon_vma lock?
>
> EDIT: See below, I figured out what it was.
>
> Looking at the code:
>
> <anon_vma lock>
> [reclaim]
> < PMD lock >
> -> try_to_unmap_one()
>     -> unmap_huge_pmd_locked()
>       -> __discard_anon_folio_pmd_locked()
>         [ clears PMD entry ]

The PMD becomes pmd_none().

> 	...

There are two readers making decisions based on “pmd_none()” locklessly:

1. munmap case is via zap_pmd_range() in mm/memory.c: it skips pmd_none(),
so the above folio is not zapped;

2. MREMAP_DONTUNMAP case is via move_page_tables() in mm/mremap.c calls
get_old_pmd(): get_old_pmd() returns NULL for pmd_none(),
so move_page_tables() skips the above folio.

> 	[ reinstates PMD entry later ]


1. for munmap, unmap_region() calls free_pgtables(). In free_pmd_range(),
the restored PMD is treated as pmd_bad() and cleared. And the folio’s
rmap is not removed.

2. for meremap, dontunmap_complete() unlinks anon_vma but the source VMA
still maps the folio.

>
> Concurrent munmap():
>
> <mmap write lock, downgraded to mmap read lock later>
> do_vmi_align_munmap()
>   -> vms_complete_munmap_vmas()
>     -> vms_clear_ptes()
>       -> unmap_region()
>         -> unmap_vmas() < zaps > <----- EDIT: the real issue, see below
> 	-> free_pgtables()
> 	   -> unlink_anon_vmas()
> 	     -> < anon_vma lock taken! >
> 	   ... clears down page tables (no rmap lock) ...
>
> Is the issue actually the zapping in unmap_vmas(), which doesn't seem to be
> protected by rmap lock at all, meaning that _that_ can happen with
> concurrent munmap()?

No. zap and rmap walkers are serialized by PTL. The reason is described above.

>
> So unmap_vmas() doesn't correctly populate the mmu_gather struct for the
> range, meaning that the underlying page isn't freed and the refcount,
> mapcount etc. are all left as they were?

Yes to the underlying page isn’t freed.

>
> MREMAP_DONTUNMAP seems a bit of a bizarre callout, but I suppose it is the
> other case in which (under certain, likely, circumstances
> e.g. non-partial/self-merging remap) an unlink_anon_vmas()'s call is done.
>
> With normal mremap() that'd happen via an unmap of the original VMA.
>
>> rmap walk uses the freed anon_vma.
>>
>> Using an invalidated PMD instead of a cleared one requires additional arch
>> code fixes. Instead, disable the PMD level discard of lazyfree THPs, as
>> before commit 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during
>> shrink_folio_list()").
>
> Again it feels like this commit message assumes a bunch of information that
> isn't presented here.

1. SPARC has thp_pte_count-- in its pmdp_invalidate(), but set_pmd_at()
does not add it back, since the code assumes pmdp_invalidate() is only
used in THP splitting code.

2. GUP-fast accepts an invalidate PMD on s390, risk, loongarch. They
need to fix it to prevent GUP-fast from succeeding for this new
pmdp_invalidate().

>
> Why would you need to invalidate the PMD? Is the issue actually TLB flush?
> How does that get around the fact that a PMD entry can be 'reinstated'
> concurrent with a zap?
>
> Or is it that you put something not-pmd_none() there? Hmm yeah I think so.
>
> Doesn't all this cause pmd_none_or_clear_bad() to fire too?
>
> All-round terrible.
>
> I've not dug into the reports in detail, but I think the commit message
> should also talk about what exactly is missing from pmdp_invalidate() that
> is broken here? Are some arches not doing things quite right?
>
> OK reading the code more and it seems the _real_ issue is actually
> pmd_is_huge() in zap_pmd_range() right?
>
> It's the -> pmd_none() in general. By doing that, you are breaking a
> contract with PMD leaf entries.
>
> So you get:
>
> zap_pmd_range()
>   -> pmd_is_huge() ? [ no ]
>      -> zap_huge_pmd() [ WOULD HAVE taken PMD lock that blocks against reclaim ]
>
> So using something like pmdp_invalidate() avoids this, because it keeps the
> entry huge but invalid.
>
> But I guess not every arch does this properly somehow?
>
>>
>> Fixes: 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during shrink_folio_list()")
>> Reported-by: Kyle Zeng <kylebot@openai.com>
>> Closes: https://lore.kernel.org/r/20261009165214.40212-2-kylebot@openai.com
>> Cc: stable@vger.kernel.org
>> Assisted-by: LLM
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>
> With the commit message made a lot clearer and calling out the specific
> locking scenarios that allow this + getting rid of the dead code:
>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Thanks.

>
>> ---
>> The proposed fix[1] requires additional arch code fixes[2]. Disable it
>> first for stable fix. The function will be re-enabled along with arch code
>> fixes.
>>
>> Link: https://lore.kernel.org/all/20261009165214.40212-2-kylebot@openai.com/ [1]
>> Link: https://lore.kernel.org/all/DM0II2QHWWS1.2RXEJXC8S0101@nvidia.com/ [2]
>> ---
>>  mm/rmap.c | 11 -----------
>>  1 file changed, 11 deletions(-)
>>
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index 805db93fe0428..1131b76bbbc28 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -2275,17 +2275,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>  		}
>>
>>  		if (!pvmw.pte) {
>> -			if (folio_test_lazyfree(folio)) {
>> -				if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, folio))
>
> This is the only caller of unmap_huge_pmd_locked(), making that and
> __discard_anon_folio_pmd_locked() dead code, shouldn't we remove all of it?
>
> Otherwise we're leaving dead code around.

1) it keeps the backport small, 2) I am going to restore the function and
this reduce the code churn.

I can remove the dead code if you think that is better.

>
>
>> -					goto walk_done;
>> -				/*
>> -				 * unmap_huge_pmd_locked has either already marked
>> -				 * the folio as swap-backed or decided to retain it
>> -				 * due to GUP or speculative references.
>> -				 */
>> -				goto walk_abort;
>> -			}
>> -
>>  			if (flags & TTU_SPLIT_HUGE_PMD) {
>>  				/*
>>  				 * We temporarily have to drop the PTL and
>>
>> ---
>> base-commit: 8b38ed9ab5b09c8ba168cbcc49524e9b380ee5c4
>> change-id: 20261009-lazyfree-stable-fix-67712b2049dd
>>
>> Best regards,
>> --
>> Yan, Zi
>>
>
> --
> Cheers, Lorenzo


Best Regards,
Yan, Zi

Re: [PATCH] mm/rmap: don't discard lazyfree THPs at PMD level

[David Hildenbrand (Arm)]

>>> --- a/mm/rmap.c
>>> +++ b/mm/rmap.c
>>> @@ -2275,17 +2275,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>>  		}
>>>
>>>  		if (!pvmw.pte) {
>>> -			if (folio_test_lazyfree(folio)) {
>>> -				if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, folio))
>>
>> This is the only caller of unmap_huge_pmd_locked(), making that and
>> __discard_anon_folio_pmd_locked() dead code, shouldn't we remove all of it?
>>
>> Otherwise we're leaving dead code around.
> 
> 1) it keeps the backport small, 2) I am going to restore the function and
> this reduce the code churn.
> 
> I can remove the dead code if you think that is better.

Leave it in and let's clarify that we'll restore the functionality shortly.

-- 
Cheers,

David

Re: [PATCH] mm/rmap: don't discard lazyfree THPs at PMD level

[Lorenzo Stoakes (ARM)]

On Mon, Oct 12, 2026 at 04:39:03PM +0200, David Hildenbrand (Arm) wrote:

> > I can remove the dead code if you think that is better.
>
> Leave it in and let's clarify that we'll restore the functionality shortly.

Yup as long as there's something in the commit msg that's fine.

--
Cheers, Lorenzo

Re: [PATCH] mm/rmap: don't discard lazyfree THPs at PMD level

[Lorenzo Stoakes (ARM)]

On Mon, Oct 12, 2026 at 10:33:34AM -0400, Zi Yan wrote:
> > Looking at the code:
> >
> > <anon_vma lock>
> > [reclaim]
> > < PMD lock >
> > -> try_to_unmap_one()
> >     -> unmap_huge_pmd_locked()
> >       -> __discard_anon_folio_pmd_locked()
> >         [ clears PMD entry ]
>
> The PMD becomes pmd_none().

Yup.

>
> > 	...
>
> There are two readers making decisions based on “pmd_none()” locklessly:
>
> 1. munmap case is via zap_pmd_range() in mm/memory.c: it skips pmd_none(),
> so the above folio is not zapped;

Yup I get at that below.

It also doesn't serialise on the PMD lock as a result.

That's really key isn't it?

		if (pmd_is_huge(*pmd)) { 				<- returns false
			if (next - addr != HPAGE_PMD_SIZE)
				__split_huge_pmd(vma, pmd, addr, false);
			else if (zap_huge_pmd(tlb, vma, pmd, addr)) {	<- doesn't lock the PMD
				addr = next;
				continue;
			}

So it skips it and nothing happens to the PMD nor is there any serialisation
between unmap_huge_pmd_locked() and this code.

try_to_unmap_one() will be called under the PMD lock because of this In
page_vma_mapped_walk(), I believe:

bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
{
	...
		if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
		    (pmd_trans_huge(pmde) || pmd_is_migration_entry(pmde) ||
		    pmd_is_device_private_entry(pmde))) {
			pvmw->ptl = pmd_lock(mm, pvmw->pmd);
			...
			if (likely(pmd_trans_huge(pmde))) {
				...
				return true;
			}
			...
		}
	...
}

>
> 2. MREMAP_DONTUNMAP case is via move_page_tables() in mm/mremap.c calls
> get_old_pmd(): get_old_pmd() returns NULL for pmd_none(),
> so move_page_tables() skips the above folio.

move_page_tables() is called for _all_ mremap() operations. So the same issue
would occur across mremap().

That bit isn't special to MREMAP_DONTUNMAP.

>
> > 	[ reinstates PMD entry later ]
>
>
> 1. for munmap, unmap_region() calls free_pgtables(). In free_pmd_range(),
> the restored PMD is treated as pmd_bad() and cleared. And the folio’s
> rmap is not removed.

Yup I got to that point below :)

>
> 2. for meremap, dontunmap_complete() unlinks anon_vma but the source VMA
> still maps the folio.

Well, only if the set_pmd_at() managed to reinstall it AND the PUD page table
that mapped it still exists.

That set pmd could also UAF on the PMD page table in theory couldn't it?

It's all really horrible.

>
> >
> > Concurrent munmap():
> >
> > <mmap write lock, downgraded to mmap read lock later>
> > do_vmi_align_munmap()
> >   -> vms_complete_munmap_vmas()
> >     -> vms_clear_ptes()
> >       -> unmap_region()
> >         -> unmap_vmas() < zaps > <----- EDIT: the real issue, see below
> > 	-> free_pgtables()
> > 	   -> unlink_anon_vmas()
> > 	     -> < anon_vma lock taken! >
> > 	   ... clears down page tables (no rmap lock) ...
> >
> > Is the issue actually the zapping in unmap_vmas(), which doesn't seem to be
> > protected by rmap lock at all, meaning that _that_ can happen with
> > concurrent munmap()?
>
> No. zap and rmap walkers are serialized by PTL. The reason is described above.
>

Well, no, that's not quite right AFAICT - as above the PMD PTL _is not taken_
because the PMD entry is cleared.

So the serialisation that would otherwise exist between the try_to_unmap_one()
and zap_pmd_range() doesn't happen.

> >
> > So unmap_vmas() doesn't correctly populate the mmu_gather struct for the
> > range, meaning that the underlying page isn't freed and the refcount,
> > mapcount etc. are all left as they were?
>
> Yes to the underlying page isn’t freed.

The refcount and mapcount etc. would be correct? I think the refcount would be
such that it permanently leaks and the mapcount would be incorrect or only
accidentally correct?

>
> >
> > MREMAP_DONTUNMAP seems a bit of a bizarre callout, but I suppose it is the
> > other case in which (under certain, likely, circumstances
> > e.g. non-partial/self-merging remap) an unlink_anon_vmas()'s call is done.
> >
> > With normal mremap() that'd happen via an unmap of the original VMA.
> >
> >> rmap walk uses the freed anon_vma.
> >>
> >> Using an invalidated PMD instead of a cleared one requires additional arch
> >> code fixes. Instead, disable the PMD level discard of lazyfree THPs, as
> >> before commit 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during
> >> shrink_folio_list()").
> >
> > Again it feels like this commit message assumes a bunch of information that
> > isn't presented here.
>
> 1. SPARC has thp_pte_count-- in its pmdp_invalidate(), but set_pmd_at()
> does not add it back, since the code assumes pmdp_invalidate() is only
> used in THP splitting code.
>
> 2. GUP-fast accepts an invalidate PMD on s390, risk, loongarch. They
> need to fix it to prevent GUP-fast from succeeding for this new
> pmdp_invalidate().

Thanks that's useful to know! And ugh.

They seem like they are existing bugs effectively in pmdp_invalidate()? Or stuff
that we would want to fix anyway I guess?

In general the point of all the above discussion is to encourage a commit
message that conveys this information clearly.

When I read the patch first there was a lot of 'huh' and that's a sign that the
commit msg doesn't say enough.

> >> diff --git a/mm/rmap.c b/mm/rmap.c
> >> index 805db93fe0428..1131b76bbbc28 100644
> >> --- a/mm/rmap.c
> >> +++ b/mm/rmap.c
> >> @@ -2275,17 +2275,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> >>  		}
> >>
> >>  		if (!pvmw.pte) {
> >> -			if (folio_test_lazyfree(folio)) {
> >> -				if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, folio))
> >
> > This is the only caller of unmap_huge_pmd_locked(), making that and
> > __discard_anon_folio_pmd_locked() dead code, shouldn't we remove all of it?
> >
> > Otherwise we're leaving dead code around.
>
> 1) it keeps the backport small, 2) I am going to restore the function and
> this reduce the code churn.
>
> I can remove the dead code if you think that is better.

No, if you plan to reinstate it (soon!) then that's fine, but please
explain that you're intentionally doing that in the commit message please!

--
Cheers, Lorenzo

Re: [PATCH] mm/rmap: don't discard lazyfree THPs at PMD level

[Zi Yan]

On 12 Oct 2026, at 11:22, Lorenzo Stoakes (ARM) wrote:

> On Mon, Oct 12, 2026 at 10:33:34AM -0400, Zi Yan wrote:
>>> Looking at the code:
>>>
>>> <anon_vma lock>
>>> [reclaim]
>>> < PMD lock >
>>> -> try_to_unmap_one()
>>>     -> unmap_huge_pmd_locked()
>>>       -> __discard_anon_folio_pmd_locked()
>>>         [ clears PMD entry ]
>>
>> The PMD becomes pmd_none().
>
> Yup.
>
>>
>>> 	...
>>
>> There are two readers making decisions based on “pmd_none()” locklessly:
>>
>> 1. munmap case is via zap_pmd_range() in mm/memory.c: it skips pmd_none(),
>> so the above folio is not zapped;
>
> Yup I get at that below.
>
> It also doesn't serialise on the PMD lock as a result.
>
> That's really key isn't it?
>
> 		if (pmd_is_huge(*pmd)) { 				<- returns false
> 			if (next - addr != HPAGE_PMD_SIZE)
> 				__split_huge_pmd(vma, pmd, addr, false);
> 			else if (zap_huge_pmd(tlb, vma, pmd, addr)) {	<- doesn't lock the PMD
> 				addr = next;
> 				continue;
> 			}
>
> So it skips it and nothing happens to the PMD nor is there any serialisation
> between unmap_huge_pmd_locked() and this code.

Right.

>
> try_to_unmap_one() will be called under the PMD lock because of this In
> page_vma_mapped_walk(), I believe:
>
> bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
> {
> 	...
> 		if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
> 		    (pmd_trans_huge(pmde) || pmd_is_migration_entry(pmde) ||
> 		    pmd_is_device_private_entry(pmde))) {
> 			pvmw->ptl = pmd_lock(mm, pvmw->pmd);
> 			...
> 			if (likely(pmd_trans_huge(pmde))) {
> 				...
> 				return true;
> 			}
> 			...
> 		}
> 	...
> }

Yes.

My wording was not precise.

zap_pmd_range() doesn’t take the PTL since it sees pmd_none(), while
try_to_unmap_one() takes the PTL.

>
>>
>> 2. MREMAP_DONTUNMAP case is via move_page_tables() in mm/mremap.c calls
>> get_old_pmd(): get_old_pmd() returns NULL for pmd_none(),
>> so move_page_tables() skips the above folio.
>
> move_page_tables() is called for _all_ mremap() operations. So the same issue
> would occur across mremap().
>
> That bit isn't special to MREMAP_DONTUNMAP.

Right.


>
>>
>>> 	[ reinstates PMD entry later ]
>>
>>
>> 1. for munmap, unmap_region() calls free_pgtables(). In free_pmd_range(),
>> the restored PMD is treated as pmd_bad() and cleared. And the folio’s
>> rmap is not removed.
>
> Yup I got to that point below :)
>
>>
>> 2. for meremap, dontunmap_complete() unlinks anon_vma but the source VMA
>> still maps the folio.
>
> Well, only if the set_pmd_at() managed to reinstall it AND the PUD page table
> that mapped it still exists.
>
> That set pmd could also UAF on the PMD page table in theory couldn't it?

No, in __discard_anon_folio_pmd_locked(), the caller holds anon_vma read lock;
when mremap() or munmap() calls free_pgtables() to get rid of page tables,
they first call unlink_anon_vmas() and get blocked by the aforementioned read
lock. set_pmd_at() is called while free_pgtables() is blocked, so no UAF here.

>
> It's all really horrible.

And kills my brain.

>
>>
>>>
>>> Concurrent munmap():
>>>
>>> <mmap write lock, downgraded to mmap read lock later>
>>> do_vmi_align_munmap()
>>>   -> vms_complete_munmap_vmas()
>>>     -> vms_clear_ptes()
>>>       -> unmap_region()
>>>         -> unmap_vmas() < zaps > <----- EDIT: the real issue, see below
>>> 	-> free_pgtables()
>>> 	   -> unlink_anon_vmas()
>>> 	     -> < anon_vma lock taken! >
>>> 	   ... clears down page tables (no rmap lock) ...
>>>
>>> Is the issue actually the zapping in unmap_vmas(), which doesn't seem to be
>>> protected by rmap lock at all, meaning that _that_ can happen with
>>> concurrent munmap()?
>>
>> No. zap and rmap walkers are serialized by PTL. The reason is described above.
>>
>
> Well, no, that's not quite right AFAICT - as above the PMD PTL _is not taken_
> because the PMD entry is cleared.
>
> So the serialisation that would otherwise exist between the try_to_unmap_one()
> and zap_pmd_range() doesn't happen.
>
>>>
>>> So unmap_vmas() doesn't correctly populate the mmu_gather struct for the
>>> range, meaning that the underlying page isn't freed and the refcount,
>>> mapcount etc. are all left as they were?
>>
>> Yes to the underlying page isn’t freed.
>
> The refcount and mapcount etc. would be correct? I think the refcount would be
> such that it permanently leaks and the mapcount would be incorrect or only
> accidentally correct?

refcount and mapcount would be wrong in the munmap case. Both are correct
in MREMAP_DONTUNMAP case, since the folio is still mapped in the source VMA
but its anon_vma says it belongs to dst VMA. When dst VMA is unmapped,
the anon_vma is freed, but the folio is still mapped and its ->mapping
points to a freed anon_vma. A rmap code checking folio->mapping is a UAF.


>
>>
>>>
>>> MREMAP_DONTUNMAP seems a bit of a bizarre callout, but I suppose it is the
>>> other case in which (under certain, likely, circumstances
>>> e.g. non-partial/self-merging remap) an unlink_anon_vmas()'s call is done.
>>>
>>> With normal mremap() that'd happen via an unmap of the original VMA.
>>>
>>>> rmap walk uses the freed anon_vma.
>>>>
>>>> Using an invalidated PMD instead of a cleared one requires additional arch
>>>> code fixes. Instead, disable the PMD level discard of lazyfree THPs, as
>>>> before commit 735ecdfaf4e8 ("mm/vmscan: avoid split lazyfree THP during
>>>> shrink_folio_list()").
>>>
>>> Again it feels like this commit message assumes a bunch of information that
>>> isn't presented here.
>>
>> 1. SPARC has thp_pte_count-- in its pmdp_invalidate(), but set_pmd_at()
>> does not add it back, since the code assumes pmdp_invalidate() is only
>> used in THP splitting code.
>>
>> 2. GUP-fast accepts an invalidate PMD on s390, risk, loongarch. They
>> need to fix it to prevent GUP-fast from succeeding for this new
>> pmdp_invalidate().
>
> Thanks that's useful to know! And ugh.
>
> They seem like they are existing bugs effectively in pmdp_invalidate()? Or stuff
> that we would want to fix anyway I guess?

SPARC’s thp_pte_count might cause other issues, since its code only considers
huge pmd splitting. powerpc’s page table check might have an issue.

>
> In general the point of all the above discussion is to encourage a commit
> message that conveys this information clearly.
>
> When I read the patch first there was a lot of 'huh' and that's a sign that the
> commit msg doesn't say enough.
>
>>>> diff --git a/mm/rmap.c b/mm/rmap.c
>>>> index 805db93fe0428..1131b76bbbc28 100644
>>>> --- a/mm/rmap.c
>>>> +++ b/mm/rmap.c
>>>> @@ -2275,17 +2275,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>>>  		}
>>>>
>>>>  		if (!pvmw.pte) {
>>>> -			if (folio_test_lazyfree(folio)) {
>>>> -				if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, folio))
>>>
>>> This is the only caller of unmap_huge_pmd_locked(), making that and
>>> __discard_anon_folio_pmd_locked() dead code, shouldn't we remove all of it?
>>>
>>> Otherwise we're leaving dead code around.
>>
>> 1) it keeps the backport small, 2) I am going to restore the function and
>> this reduce the code churn.
>>
>> I can remove the dead code if you think that is better.
>
> No, if you plan to reinstate it (soon!) then that's fine, but please
> explain that you're intentionally doing that in the commit message please!

Sure. I will try to come up with a concise but comprehensive message to cover
both the issue and why pmdp_invalidate() does not work itself.


Best Regards,
Yan, Zi