[PATCH] mm/madvise: keep the hardware dirty bit in MADV_COLD's THP path

[PATCH] mm/madvise: keep the hardware dirty bit in MADV_COLD's THP path

[samarth.colleges]

From: Samarth Narang <samarth.colleges@gmail.com>

For a young, PMD-mapped THP, madvise_cold_or_pageout_pte_range() handles
MADV_COLD by reading orig_pmd = *pmd, calling pmdp_invalidate(),
discarding its return value, and writing back pmd_mkold(orig_pmd):

	if (!pageout && pmd_young(orig_pmd)) {
		pmdp_invalidate(vma, addr, pmd);
		orig_pmd = pmd_mkold(orig_pmd);
		set_pmd_at(mm, addr, pmd, orig_pmd);
		tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
	}

pmdp_invalidate() atomically clears the present bit and returns the PMD
as it was in the page table at that instant.  A store from another CPU
that lands after orig_pmd is read but before the invalidate sets the
hardware dirty bit in the live entry.  Because the written-back value is
derived from the stale orig_pmd, that dirty bit is erased.

If the THP had been marked lazyfree (MADV_FREE), reclaim later finds a
clean PMD and a clean folio and discards it rather than swapping it out
(__discard_anon_folio_pmd_locked()).  The store is then silently lost: a
subsequent read returns stale/zero data, although the MADV_FREE contract
is that a write after the advice call cancels the free.

pmdp_invalidate() returns the pre-invalidate entry precisely so callers
can carry the accessed/dirty bits hardware may have set concurrently
(d52605d7cb30).  change_huge_pmd() and set_pmd_migration_entry()
(8a8683ad9ba4) already do this; the MADV_COLD path does not.  Rebuild
orig_pmd from the returned entry.

Reproduced unprivileged on x86-64: a 2 MiB anon THP is marked MADV_FREE,
then MADV_COLD races a store from another CPU, then MADV_PAGEOUT; the
stored byte reads back zero.  On current mainline built and booted under
KVM, the race lost 131 of 8000 stores before this patch and 0 of 8000
after; a store-before-COLD control and a MADV_NORMAL control lose none.

Fixes: 9c276cc65a58 ("mm: introduce MADV_COLD")
Cc: stable@vger.kernel.org
Signed-off-by: Samarth Narang <samarth.colleges@gmail.com>
---
 mm/madvise.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -426,8 +426,14 @@
 		}
 
 		if (!pageout && pmd_young(orig_pmd)) {
-			pmdp_invalidate(vma, addr, pmd);
-			orig_pmd = pmd_mkold(orig_pmd);
+			/*
+			 * Rebuild from the entry pmdp_invalidate() exchanged
+			 * out, not from orig_pmd: hardware may have set the
+			 * dirty bit since orig_pmd was read, and a MADV_FREE'd
+			 * (lazyfree) THP whose dirty bit is lost is discarded
+			 * by reclaim, silently dropping the store.
+			 */
+			orig_pmd = pmd_mkold(pmdp_invalidate(vma, addr, pmd));
 
 			set_pmd_at(mm, addr, pmd, orig_pmd);
 			tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
-- 
2.40.0

Re: [PATCH] mm/madvise: keep the hardware dirty bit in MADV_COLD's THP path

[Lorenzo Stoakes (ARM)]

On Sun, Oct 11, 2026 at 06:22:27AM -0700, samarth.colleges@gmail.com wrote:
> From: Samarth Narang <samarth.colleges@gmail.com>
>
> For a young, PMD-mapped THP, madvise_cold_or_pageout_pte_range() handles
> MADV_COLD by reading orig_pmd = *pmd, calling pmdp_invalidate(),
> discarding its return value, and writing back pmd_mkold(orig_pmd):
>
> 	if (!pageout && pmd_young(orig_pmd)) {
> 		pmdp_invalidate(vma, addr, pmd);
> 		orig_pmd = pmd_mkold(orig_pmd);
> 		set_pmd_at(mm, addr, pmd, orig_pmd);
> 		tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> 	}
>
> pmdp_invalidate() atomically clears the present bit and returns the PMD
> as it was in the page table at that instant.  A store from another CPU
> that lands after orig_pmd is read but before the invalidate sets the
> hardware dirty bit in the live entry.  Because the written-back value is
> derived from the stale orig_pmd, that dirty bit is erased.
>
> If the THP had been marked lazyfree (MADV_FREE), reclaim later finds a
> clean PMD and a clean folio and discards it rather than swapping it out
> (__discard_anon_folio_pmd_locked()).  The store is then silently lost: a
> subsequent read returns stale/zero data, although the MADV_FREE contract
> is that a write after the advice call cancels the free.
>
> pmdp_invalidate() returns the pre-invalidate entry precisely so callers
> can carry the accessed/dirty bits hardware may have set concurrently
> (d52605d7cb30).  change_huge_pmd() and set_pmd_migration_entry()
> (8a8683ad9ba4) already do this; the MADV_COLD path does not.  Rebuild
> orig_pmd from the returned entry.
>
> Reproduced unprivileged on x86-64: a 2 MiB anon THP is marked MADV_FREE,
> then MADV_COLD races a store from another CPU, then MADV_PAGEOUT; the
> stored byte reads back zero.  On current mainline built and booted under
> KVM, the race lost 131 of 8000 stores before this patch and 0 of 8000
> after; a store-before-COLD control and a MADV_NORMAL control lose none.
>
> Fixes: 9c276cc65a58 ("mm: introduce MADV_COLD")
> Cc: stable@vger.kernel.org
> Signed-off-by: Samarth Narang <samarth.colleges@gmail.com>

Thanks for the report.

As this seems to be AI-generated, a member of the core team will take over
this.

> ---
>  mm/madvise.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -426,8 +426,14 @@
>  		}
>
>  		if (!pageout && pmd_young(orig_pmd)) {
> -			pmdp_invalidate(vma, addr, pmd);
> -			orig_pmd = pmd_mkold(orig_pmd);
> +			/*
> +			 * Rebuild from the entry pmdp_invalidate() exchanged
> +			 * out, not from orig_pmd: hardware may have set the
> +			 * dirty bit since orig_pmd was read, and a MADV_FREE'd
> +			 * (lazyfree) THP whose dirty bit is lost is discarded
> +			 * by reclaim, silently dropping the store.
> +			 */
> +			orig_pmd = pmd_mkold(pmdp_invalidate(vma, addr, pmd));
>
>  			set_pmd_at(mm, addr, pmd, orig_pmd);
>  			tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> --
> 2.40.0

--
Cheers, Lorenzo

Re: [PATCH] mm/madvise: keep the hardware dirty bit in MADV_COLD's THP path

[David Hildenbrand (Arm)]

On 10/11/26 15:22, samarth.colleges@gmail.com wrote:
> From: Samarth Narang <samarth.colleges@gmail.com>
> 
> For a young, PMD-mapped THP, madvise_cold_or_pageout_pte_range() handles
> MADV_COLD by reading orig_pmd = *pmd, calling pmdp_invalidate(),
> discarding its return value, and writing back pmd_mkold(orig_pmd):
> 
> 	if (!pageout && pmd_young(orig_pmd)) {
> 		pmdp_invalidate(vma, addr, pmd);
> 		orig_pmd = pmd_mkold(orig_pmd);
> 		set_pmd_at(mm, addr, pmd, orig_pmd);
> 		tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> 	}
> 
> pmdp_invalidate() atomically clears the present bit and returns the PMD
> as it was in the page table at that instant.  A store from another CPU
> that lands after orig_pmd is read but before the invalidate sets the
> hardware dirty bit in the live entry.  Because the written-back value is
> derived from the stale orig_pmd, that dirty bit is erased.
> 
> If the THP had been marked lazyfree (MADV_FREE), reclaim later finds a
> clean PMD and a clean folio and discards it rather than swapping it out
> (__discard_anon_folio_pmd_locked()).  The store is then silently lost: a
> subsequent read returns stale/zero data, although the MADV_FREE contract
> is that a write after the advice call cancels the free.
> 
> pmdp_invalidate() returns the pre-invalidate entry precisely so callers
> can carry the accessed/dirty bits hardware may have set concurrently
> (d52605d7cb30).  change_huge_pmd() and set_pmd_migration_entry()
> (8a8683ad9ba4) already do this; the MADV_COLD path does not.  Rebuild
> orig_pmd from the returned entry.
> 
> Reproduced unprivileged on x86-64: a 2 MiB anon THP is marked MADV_FREE,
> then MADV_COLD races a store from another CPU, then MADV_PAGEOUT; the
> stored byte reads back zero.  On current mainline built and booted under
> KVM, the race lost 131 of 8000 stores before this patch and 0 of 8000
> after; a store-before-COLD control and a MADV_NORMAL control lose none.
> 
> Fixes: 9c276cc65a58 ("mm: introduce MADV_COLD")
> Cc: stable@vger.kernel.org
> Signed-off-by: Samarth Narang <samarth.colleges@gmail.com>
> ---
>  mm/madvise.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -426,8 +426,14 @@
>  		}
>  
>  		if (!pageout && pmd_young(orig_pmd)) {
> -			pmdp_invalidate(vma, addr, pmd);
> -			orig_pmd = pmd_mkold(orig_pmd);
> +			/*
> +			 * Rebuild from the entry pmdp_invalidate() exchanged
> +			 * out, not from orig_pmd: hardware may have set the
> +			 * dirty bit since orig_pmd was read, and a MADV_FREE'd
> +			 * (lazyfree) THP whose dirty bit is lost is discarded
> +			 * by reclaim, silently dropping the store.
> +			 */
> +			orig_pmd = pmd_mkold(pmdp_invalidate(vma, addr, pmd));

Okay, you are saying we can lose the dirty bit that could get set concurrently.
And if we did an MADV_FREE before, we might not detect that the content was changed.


I suspect this found through the help of an LLM and not an actual use case in
practice?

Using MADV_COLD following MADV_FREE sounds odd at thirst. In particular, when
having MADV_COLD race with actual access.

On the PTE path we use clear_young_dirty_ptes(CYDP_CLEAR_YOUNG) which achieves
the same through ptep_test_and_clear_young().


... where we also just do a

pte = ptep_get(ptep);
set_pte_at(vma->vm_mm, address, ptep, pte_mkold(pte));

in the generic variant and rely on the per-arch variant to do the right thing.

Which makes me wonder whether we should actually have per-architecture PMD
helpers for that purpose?


-- 
Cheers,

David

Re: [PATCH] mm/madvise: keep the hardware dirty bit in MADV_COLD's THP path

[Lorenzo Stoakes (ARM)]

-cc slopper.

On Mon, Oct 12, 2026 at 12:09:07PM +0200, David Hildenbrand (Arm) wrote:
> On 10/11/26 15:22, samarth.colleges@gmail.com wrote:

> > diff --git a/mm/madvise.c b/mm/madvise.c
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -426,8 +426,14 @@
> >  		}
> >
> >  		if (!pageout && pmd_young(orig_pmd)) {
> > -			pmdp_invalidate(vma, addr, pmd);
> > -			orig_pmd = pmd_mkold(orig_pmd);
> > +			/*
> > +			 * Rebuild from the entry pmdp_invalidate() exchanged
> > +			 * out, not from orig_pmd: hardware may have set the
> > +			 * dirty bit since orig_pmd was read, and a MADV_FREE'd
> > +			 * (lazyfree) THP whose dirty bit is lost is discarded
> > +			 * by reclaim, silently dropping the store.
> > +			 */
> > +			orig_pmd = pmd_mkold(pmdp_invalidate(vma, addr, pmd));
>
> Okay, you are saying we can lose the dirty bit that could get set concurrently.
> And if we did an MADV_FREE before, we might not detect that the content was changed.

Yeah, lazyfree -> write means you don't actually free it.

As per the manpage:

	For each of the pages that has been marked to be freed but has not yet
	been freed, the free operation will be canceled if the caller writes
	into the page.

This is anon memory only:

	The MADV_FREE operation can be applied only to private anonymous
	pages (see mmap(2)).

>
>
> I suspect this found through the help of an LLM and not an actual use case in
> practice?

Seems very likely.

>
> Using MADV_COLD following MADV_FREE sounds odd at thirst. In particular, when
> having MADV_COLD race with actual access.

It's odd to MADV_FREE (lazyfree) then MADV_COLD (deactivate) yes,
especially with racing accesses.

I wonder if there are other cases where you could get weird races like
this...

In any case it seems a really hard thing to hit in reality. But I think we
probably need to fix this because of the stated contract of MADV_FREE.

I think probably we shouldn't have added that contract...

>
> On the PTE path we use clear_young_dirty_ptes(CYDP_CLEAR_YOUNG) which achieves
> the same through ptep_test_and_clear_young().
>
>
> ... where we also just do a
>
> pte = ptep_get(ptep);
> set_pte_at(vma->vm_mm, address, ptep, pte_mkold(pte));
>
> in the generic variant and rely on the per-arch variant to do the right thing.
>
> Which makes me wonder whether we should actually have per-architecture PMD
> helpers for that purpose?

Is it achieving the same thing? There's an invalidation here also.

pmdp_establish() is used to install a pmd_mkinvalid() entry and there's a
flush_pmd_tlb_range().

--
Cheers, Lorenzo

Re: [PATCH] mm/madvise: keep the hardware dirty bit in MADV_COLD's THP path

[Lorenzo Stoakes (ARM)]

+cc Zi

On Mon, Oct 12, 2026 at 11:33:48AM +0100, Lorenzo Stoakes (ARM) wrote:
> -cc slopper.
>
> On Mon, Oct 12, 2026 at 12:09:07PM +0200, David Hildenbrand (Arm) wrote:
> > On 10/11/26 15:22, samarth.colleges@gmail.com wrote:
>
> > > diff --git a/mm/madvise.c b/mm/madvise.c
> > > --- a/mm/madvise.c
> > > +++ b/mm/madvise.c
> > > @@ -426,8 +426,14 @@
> > >  		}
> > >
> > >  		if (!pageout && pmd_young(orig_pmd)) {
> > > -			pmdp_invalidate(vma, addr, pmd);
> > > -			orig_pmd = pmd_mkold(orig_pmd);
> > > +			/*
> > > +			 * Rebuild from the entry pmdp_invalidate() exchanged
> > > +			 * out, not from orig_pmd: hardware may have set the
> > > +			 * dirty bit since orig_pmd was read, and a MADV_FREE'd
> > > +			 * (lazyfree) THP whose dirty bit is lost is discarded
> > > +			 * by reclaim, silently dropping the store.
> > > +			 */
> > > +			orig_pmd = pmd_mkold(pmdp_invalidate(vma, addr, pmd));
> >
> > Okay, you are saying we can lose the dirty bit that could get set concurrently.
> > And if we did an MADV_FREE before, we might not detect that the content was changed.
>
> Yeah, lazyfree -> write means you don't actually free it.
>
> As per the manpage:
>
> 	For each of the pages that has been marked to be freed but has not yet
> 	been freed, the free operation will be canceled if the caller writes
> 	into the page.
>
> This is anon memory only:
>
> 	The MADV_FREE operation can be applied only to private anonymous
> 	pages (see mmap(2)).
>
> >
> >
> > I suspect this found through the help of an LLM and not an actual use case in
> > practice?
>
> Seems very likely.
>
> >
> > Using MADV_COLD following MADV_FREE sounds odd at thirst. In particular, when
> > having MADV_COLD race with actual access.
>
> It's odd to MADV_FREE (lazyfree) then MADV_COLD (deactivate) yes,
> especially with racing accesses.
>
> I wonder if there are other cases where you could get weird races like
> this...
>
> In any case it seems a really hard thing to hit in reality. But I think we
> probably need to fix this because of the stated contract of MADV_FREE.
>
> I think probably we shouldn't have added that contract...
>
> >
> > On the PTE path we use clear_young_dirty_ptes(CYDP_CLEAR_YOUNG) which achieves
> > the same through ptep_test_and_clear_young().
> >
> >
> > ... where we also just do a
> >
> > pte = ptep_get(ptep);
> > set_pte_at(vma->vm_mm, address, ptep, pte_mkold(pte));
> >
> > in the generic variant and rely on the per-arch variant to do the right thing.
> >
> > Which makes me wonder whether we should actually have per-architecture PMD
> > helpers for that purpose?
>
> Is it achieving the same thing? There's an invalidation here also.
>
> pmdp_establish() is used to install a pmd_mkinvalid() entry and there's a
> flush_pmd_tlb_range().
>
> --
> Cheers, Lorenzo

OK, isn't this all moot with Zi's patch? [0]

[0]: https://lore.kernel.org/linux-mm/20261009-lazyfree-stable-fix-v1-1-70997958ef8d@nvidia.com

--
Cheers, Lorenzo

Re: [PATCH] mm/madvise: keep the hardware dirty bit in MADV_COLD's THP path

[Zi Yan]

On 12 Oct 2026, at 6:38, Lorenzo Stoakes (ARM) wrote:

> +cc Zi
>
> On Mon, Oct 12, 2026 at 11:33:48AM +0100, Lorenzo Stoakes (ARM) wrote:
>> -cc slopper.
>>
>> On Mon, Oct 12, 2026 at 12:09:07PM +0200, David Hildenbrand (Arm) wrote:
>>> On 10/11/26 15:22, samarth.colleges@gmail.com wrote:
>>
>>>> diff --git a/mm/madvise.c b/mm/madvise.c
>>>> --- a/mm/madvise.c
>>>> +++ b/mm/madvise.c
>>>> @@ -426,8 +426,14 @@
>>>>  		}
>>>>
>>>>  		if (!pageout && pmd_young(orig_pmd)) {
>>>> -			pmdp_invalidate(vma, addr, pmd);
>>>> -			orig_pmd = pmd_mkold(orig_pmd);
>>>> +			/*
>>>> +			 * Rebuild from the entry pmdp_invalidate() exchanged
>>>> +			 * out, not from orig_pmd: hardware may have set the
>>>> +			 * dirty bit since orig_pmd was read, and a MADV_FREE'd
>>>> +			 * (lazyfree) THP whose dirty bit is lost is discarded
>>>> +			 * by reclaim, silently dropping the store.
>>>> +			 */
>>>> +			orig_pmd = pmd_mkold(pmdp_invalidate(vma, addr, pmd));
>>>
>>> Okay, you are saying we can lose the dirty bit that could get set concurrently.
>>> And if we did an MADV_FREE before, we might not detect that the content was changed.
>>
>> Yeah, lazyfree -> write means you don't actually free it.
>>
>> As per the manpage:
>>
>> 	For each of the pages that has been marked to be freed but has not yet
>> 	been freed, the free operation will be canceled if the caller writes
>> 	into the page.
>>
>> This is anon memory only:
>>
>> 	The MADV_FREE operation can be applied only to private anonymous
>> 	pages (see mmap(2)).
>>
>>>
>>>
>>> I suspect this found through the help of an LLM and not an actual use case in
>>> practice?
>>
>> Seems very likely.
>>
>>>
>>> Using MADV_COLD following MADV_FREE sounds odd at thirst. In particular, when
>>> having MADV_COLD race with actual access.
>>
>> It's odd to MADV_FREE (lazyfree) then MADV_COLD (deactivate) yes,
>> especially with racing accesses.
>>
>> I wonder if there are other cases where you could get weird races like
>> this...
>>
>> In any case it seems a really hard thing to hit in reality. But I think we
>> probably need to fix this because of the stated contract of MADV_FREE.
>>
>> I think probably we shouldn't have added that contract...
>>
>>>
>>> On the PTE path we use clear_young_dirty_ptes(CYDP_CLEAR_YOUNG) which achieves
>>> the same through ptep_test_and_clear_young().
>>>
>>>
>>> ... where we also just do a
>>>
>>> pte = ptep_get(ptep);
>>> set_pte_at(vma->vm_mm, address, ptep, pte_mkold(pte));
>>>
>>> in the generic variant and rely on the per-arch variant to do the right thing.
>>>
>>> Which makes me wonder whether we should actually have per-architecture PMD
>>> helpers for that purpose?
>>
>> Is it achieving the same thing? There's an invalidation here also.
>>
>> pmdp_establish() is used to install a pmd_mkinvalid() entry and there's a
>> flush_pmd_tlb_range().
>>
>> --
>> Cheers, Lorenzo
>
> OK, isn't this all moot with Zi's patch? [0]
>
> [0]: https://lore.kernel.org/linux-mm/20261009-lazyfree-stable-fix-v1-1-70997958ef8d@nvidia.com

No. Without lazyfree PMD level THP discard, the affect PMD will be split
into PTEs without dirty bit and the dirty folio is still discarded.

We will need Samarth’s fix for backport, then we can think carefully
about David’s per-arch solution for mainline.

Best Regards,
Yan, Zi

Re: [PATCH] mm/madvise: keep the hardware dirty bit in MADV_COLD's THP path

[Lorenzo Stoakes (ARM)]

+cc Pedro

On Mon, Oct 12, 2026 at 11:17:57AM -0400, Zi Yan wrote:
> On 12 Oct 2026, at 6:38, Lorenzo Stoakes (ARM) wrote:

> > [0]: https://lore.kernel.org/linux-mm/20261009-lazyfree-stable-fix-v1-1-70997958ef8d@nvidia.com
>
> No. Without lazyfree PMD level THP discard, the affect PMD will be split
> into PTEs without dirty bit and the dirty folio is still discarded.

OK thanks for clarifying!

>
> We will need Samarth’s fix for backport, then we can think carefully
> about David’s per-arch solution for mainline.

It's AI slop, and we don't accept that in mm (also -cc slopper).

Somebody from the core team will take of this fix separately.

Pedro - you mentioned you were interested in this, will you send a patch?

--
Cheers, Lorenzo

Re: [PATCH] mm/madvise: keep the hardware dirty bit in MADV_COLD's THP path

[Pedro Falcato]

On Mon, Oct 12, 2026 at 04:26:31PM +0100, Lorenzo Stoakes (ARM) wrote:
> +cc Pedro
> 
> On Mon, Oct 12, 2026 at 11:17:57AM -0400, Zi Yan wrote:
> > On 12 Oct 2026, at 6:38, Lorenzo Stoakes (ARM) wrote:
> 
> > > [0]: https://lore.kernel.org/linux-mm/20261009-lazyfree-stable-fix-v1-1-70997958ef8d@nvidia.com
> >
> > No. Without lazyfree PMD level THP discard, the affect PMD will be split
> > into PTEs without dirty bit and the dirty folio is still discarded.
> 
> OK thanks for clarifying!
> 
> >
> > We will need Samarth’s fix for backport, then we can think carefully
> > about David’s per-arch solution for mainline.
> 
> It's AI slop, and we don't accept that in mm (also -cc slopper).
> 
> Somebody from the core team will take of this fix separately.
> 
> Pedro - you mentioned you were interested in this, will you send a patch?

I'll send something, tomorrow :)

-- 
Pedro

Re: [PATCH] mm/madvise: keep the hardware dirty bit in MADV_COLD's THP path

[Lorenzo Stoakes (ARM)]

On Mon, Oct 12, 2026 at 02:25:38PM -0400, Samarth Narang wrote:
> Thanks both for your review into this; I was building a structural detector
> tool over page-table bit paths, and came across this.
> I got agents to reproduce it on x86 where we mark a 2MiB anon THP
> MADV_FREE, race a store from a second CPU against MADV_COLD, then force
> reclaim with MADV_PAGEOUT. The race pass loses stores while the control
> pass does not.
>
> Based on your suggestion, can we clear the accessed bit in place with
> pmdp_test_and_clear_young(), as the PTE path does via
> ptep_test_and_clear_young(), instead of rebuilding the entry from
> pmdb_invalidate()'s return value. I tried this patch on the bug, and the
> fix holds. Let me know what you think.

As I already told you, this is being handled by a member of the core team,
thanks.

--
Cheers, Lorenzo