[PATCH] mm/madvise: atomically clear the young bit on THP MADV_COLD

[PATCH] mm/madvise: atomically clear the young bit on THP MADV_COLD

[Pedro Falcato]

As-is, madvise_cold_or_pageout_pte_range() discards the hardware young
bit by invalidating the PMD, doing pmd_mkold() and subsequently writing
to the pmd again. However, it does so in a racy manner, looking at a
possibly-stale snapshot. It's entirely possible that a write elsewhere
set the hardware D bit, which this logic now loses.

Fix it by using pmdp_test_and_clear_young(). This is an atomic
modification of the PMD entry, and thus safe against any possible racing
writes. As a (nice) side-effect, this elides one non-required TLB flush
in pmdp_invalidate().

This bug can interact with the swapping system in complicated ways, leading
to data loss. For instance, if a folio is marked lazyfree, but was later
read from, and then MADV_COLD'd while racing a write to that same THP,
dropping the dirty bit may lead to data loss as reclaim/MADV_PAGEOUT drops
the "clean" anon folio:

T0                                T1
================================================================
MADV_FREE(thp)			|
  folio marked lazyfree		|
read(thp)			|
  PMD marked young by hardware	|
				| MADV_COLD(thp)
write(thp)			|   if (pmd_young(pmd)) {
  PMD marked dirty by hardware	|     set(pmdp, pmd_mkold(pmd));
MADV_PAGEOUT/reclaim		|
  dirty bit may or may not be   |
  set				|
    reclaim/pageout may drop    |
    the folio			|

Fixes: 9c276cc65a58 ("mm: introduce MADV_COLD")
Cc: stable@vger.kernel.org
Reported-by: Samarth Narang <samarth.colleges@gmail.com>
Closes: https://lore.kernel.org/linux-mm/20261011132227.413217-1-samarth.colleges@gmail.com/
Signed-off-by: Pedro Falcato <pfalcato@suse.de>
---
 mm/madvise.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 85ab9bc76e06..19cd3bb4d1b9 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -431,10 +431,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *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);
+			pmdp_test_and_clear_young(vma, addr, pmd);
 			tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
 		}
 
-- 
2.55.0

Re: [PATCH] mm/madvise: atomically clear the young bit on THP MADV_COLD

[Gregory Price]

On Tue, Oct 13, 2026 at 08:17:19PM +0100, Pedro Falcato wrote:
> As-is, madvise_cold_or_pageout_pte_range() discards the hardware young
> bit by invalidating the PMD, doing pmd_mkold() and subsequently writing
> to the pmd again. However, it does so in a racy manner, looking at a
> possibly-stale snapshot. It's entirely possible that a write elsewhere
> set the hardware D bit, which this logic now loses.
> 
> Fix it by using pmdp_test_and_clear_young(). This is an atomic
> modification of the PMD entry, and thus safe against any possible racing
> writes. As a (nice) side-effect, this elides one non-required TLB flush
> in pmdp_invalidate().
> 
> This bug can interact with the swapping system in complicated ways, leading
> to data loss. For instance, if a folio is marked lazyfree, but was later
> read from, and then MADV_COLD'd while racing a write to that same THP,
> dropping the dirty bit may lead to data loss as reclaim/MADV_PAGEOUT drops
> the "clean" anon folio:
> 
> T0                                T1
> ================================================================
> MADV_FREE(thp)			|
>   folio marked lazyfree		|
> read(thp)			|
>   PMD marked young by hardware	|
> 				| MADV_COLD(thp)
> write(thp)			|   if (pmd_young(pmd)) {
>   PMD marked dirty by hardware	|     set(pmdp, pmd_mkold(pmd));
> MADV_PAGEOUT/reclaim		|
>   dirty bit may or may not be   |
>   set				|
>     reclaim/pageout may drop    |
>     the folio			|
> 
> Fixes: 9c276cc65a58 ("mm: introduce MADV_COLD")
> Cc: stable@vger.kernel.org
> Reported-by: Samarth Narang <samarth.colleges@gmail.com>
> Closes: https://lore.kernel.org/linux-mm/20261011132227.413217-1-samarth.colleges@gmail.com/
> Signed-off-by: Pedro Falcato <pfalcato@suse.de>

The function that keeps on giving (bugs)

Reviewed-by: Gregory Price (Meta) <gourry@gourry.net>

Re: [PATCH] mm/madvise: atomically clear the young bit on THP MADV_COLD

[David Hildenbrand (Arm)]

On 10/13/26 21:17, Pedro Falcato wrote:
> As-is, madvise_cold_or_pageout_pte_range() discards the hardware young
> bit by invalidating the PMD, doing pmd_mkold() and subsequently writing
> to the pmd again. However, it does so in a racy manner, looking at a
> possibly-stale snapshot. It's entirely possible that a write elsewhere
> set the hardware D bit, which this logic now loses.
> 
> Fix it by using pmdp_test_and_clear_young(). This is an atomic
> modification of the PMD entry, and thus safe against any possible racing
> writes. As a (nice) side-effect, this elides one non-required TLB flush
> in pmdp_invalidate().
> 
> This bug can interact with the swapping system in complicated ways, leading
> to data loss. For instance, if a folio is marked lazyfree, but was later
> read from, and then MADV_COLD'd while racing a write to that same THP,
> dropping the dirty bit may lead to data loss as reclaim/MADV_PAGEOUT drops
> the "clean" anon folio:
> 
> T0                                T1
> ================================================================
> MADV_FREE(thp)			|
>   folio marked lazyfree		|
> read(thp)			|
>   PMD marked young by hardware	|
> 				| MADV_COLD(thp)
> write(thp)			|   if (pmd_young(pmd)) {
>   PMD marked dirty by hardware	|     set(pmdp, pmd_mkold(pmd));
> MADV_PAGEOUT/reclaim		|
>   dirty bit may or may not be   |
>   set				|
>     reclaim/pageout may drop    |
>     the folio			|
> 
> Fixes: 9c276cc65a58 ("mm: introduce MADV_COLD")
> Cc: stable@vger.kernel.org
> Reported-by: Samarth Narang <samarth.colleges@gmail.com>
> Closes: https://lore.kernel.org/linux-mm/20261011132227.413217-1-samarth.colleges@gmail.com/
> Signed-off-by: Pedro Falcato <pfalcato@suse.de>
> ---
>  mm/madvise.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 85ab9bc76e06..19cd3bb4d1b9 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -431,10 +431,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *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);
> +			pmdp_test_and_clear_young(vma, addr, pmd);
>  			tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
Ah, we even already have that arch helper, nice. Let's hope all relevant
architectures implement a proper variant :)

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

-- 
Cheers,

David

Re: [PATCH] mm/madvise: atomically clear the young bit on THP MADV_COLD

[Zi Yan]

On Tue Oct 13, 2026 at 3:17 PM EDT, Pedro Falcato wrote:
> As-is, madvise_cold_or_pageout_pte_range() discards the hardware young
> bit by invalidating the PMD, doing pmd_mkold() and subsequently writing
> to the pmd again. However, it does so in a racy manner, looking at a
> possibly-stale snapshot. It's entirely possible that a write elsewhere
> set the hardware D bit, which this logic now loses.
>
> Fix it by using pmdp_test_and_clear_young(). This is an atomic
> modification of the PMD entry, and thus safe against any possible racing
> writes. As a (nice) side-effect, this elides one non-required TLB flush
> in pmdp_invalidate().
>
> This bug can interact with the swapping system in complicated ways, leading
> to data loss. For instance, if a folio is marked lazyfree, but was later
> read from, and then MADV_COLD'd while racing a write to that same THP,
> dropping the dirty bit may lead to data loss as reclaim/MADV_PAGEOUT drops
> the "clean" anon folio:
>
> T0                                T1
> ================================================================
> MADV_FREE(thp)			|
>   folio marked lazyfree		|
> read(thp)			|
>   PMD marked young by hardware	|
> 				| MADV_COLD(thp)
> write(thp)			|   if (pmd_young(pmd)) {
>   PMD marked dirty by hardware	|     set(pmdp, pmd_mkold(pmd));
> MADV_PAGEOUT/reclaim		|
>   dirty bit may or may not be   |
>   set				|
>     reclaim/pageout may drop    |
>     the folio			|
>
> Fixes: 9c276cc65a58 ("mm: introduce MADV_COLD")
> Cc: stable@vger.kernel.org
> Reported-by: Samarth Narang <samarth.colleges@gmail.com>
> Closes: https://lore.kernel.org/linux-mm/20261011132227.413217-1-samarth.colleges@gmail.com/
> Signed-off-by: Pedro Falcato <pfalcato@suse.de>
> ---
>  mm/madvise.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>

LGTM.

Acked-by: Zi Yan <ziy@nvidia.com>



-- 
Best Regards,
Yan, Zi

Re: [PATCH] mm/madvise: atomically clear the young bit on THP MADV_COLD

[Lorenzo Stoakes (ARM)]

+cc Gregory

On Tue, Oct 13, 2026 at 08:17:19PM +0100, Pedro Falcato wrote:
> As-is, madvise_cold_or_pageout_pte_range() discards the hardware young

The function from hell... Gregory - PLEASE FIX THIS SOON :))

> bit by invalidating the PMD, doing pmd_mkold() and subsequently writing
> to the pmd again. However, it does so in a racy manner, looking at a
> possibly-stale snapshot. It's entirely possible that a write elsewhere
> set the hardware D bit, which this logic now loses.
>
> Fix it by using pmdp_test_and_clear_young(). This is an atomic
> modification of the PMD entry, and thus safe against any possible racing
> writes. As a (nice) side-effect, this elides one non-required TLB flush
> in pmdp_invalidate().

Nice, I'm guessing the tlb_remove_pmd_tlb_entry() does what we need already? (as
discussed off-list :)

>
> This bug can interact with the swapping system in complicated ways, leading
> to data loss. For instance, if a folio is marked lazyfree, but was later
> read from, and then MADV_COLD'd while racing a write to that same THP,
> dropping the dirty bit may lead to data loss as reclaim/MADV_PAGEOUT drops
> the "clean" anon folio:
>
> T0                                T1
> ================================================================
> MADV_FREE(thp)                  |
>   folio marked lazyfree         |
> read(thp)                       |
>   PMD marked young by hardware  |
>                                 | MADV_COLD(thp)
> write(thp)                      |   if (pmd_young(pmd)) {
>   PMD marked dirty by hardware  |     set(pmdp, pmd_mkold(pmd));
> MADV_PAGEOUT/reclaim            |
>   dirty bit may or may not be   |
>   set                           |
>     reclaim/pageout may drop    |
>     the folio                   |
>

Thanks, nice explanation (even if the >quoting ruins the indentation on my editor
:P) [untabbed for my benefit]

>
> Fixes: 9c276cc65a58 ("mm: introduce MADV_COLD")
> Cc: stable@vger.kernel.org
> Reported-by: Samarth Narang <samarth.colleges@gmail.com>
> Closes: https://lore.kernel.org/linux-mm/20261011132227.413217-1-samarth.colleges@gmail.com/
> Signed-off-by: Pedro Falcato <pfalcato@suse.de>

LGTM so:

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

> ---
>  mm/madvise.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 85ab9bc76e06..19cd3bb4d1b9 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -431,10 +431,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *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);
> +			pmdp_test_and_clear_young(vma, addr, pmd);

Lovely diffstat :)

>  			tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
>  		}
>
> --
> 2.55.0
>

--
Cheers, Lorenzo

Re: [PATCH] mm/madvise: atomically clear the young bit on THP MADV_COLD

[Pedro Falcato]

On Wed, Oct 14, 2026 at 04:37:59PM +0100, Lorenzo Stoakes (ARM) wrote:
> +cc Gregory
> 
> On Tue, Oct 13, 2026 at 08:17:19PM +0100, Pedro Falcato wrote:
> > As-is, madvise_cold_or_pageout_pte_range() discards the hardware young
> 
> The function from hell... Gregory - PLEASE FIX THIS SOON :))

:))

> 
> > bit by invalidating the PMD, doing pmd_mkold() and subsequently writing
> > to the pmd again. However, it does so in a racy manner, looking at a
> > possibly-stale snapshot. It's entirely possible that a write elsewhere
> > set the hardware D bit, which this logic now loses.
> >
> > Fix it by using pmdp_test_and_clear_young(). This is an atomic
> > modification of the PMD entry, and thus safe against any possible racing
> > writes. As a (nice) side-effect, this elides one non-required TLB flush
> > in pmdp_invalidate().
> 
> Nice, I'm guessing the tlb_remove_pmd_tlb_entry() does what we need already? (as
> discussed off-list :)

Right, so the whole gist of it is that indeed for 99% of cases you don't
need to flush TLBs when clearing young. Out of all the ptep_clear_flush_young()
implementations out there:
 - arm64 does a light tlbi to help make sure the A bit will be cleared whenever.
(I remember reading about this, and it was due to some really weird hardware
that kinda wanted this)
 - ppc64 just clears it
 - parisc does a non-atomic get+clear with a tlb flush in #if CONFIG_FLUSH_PAGE_ACCESSED.
CONFIG_FLUSH_PAGE_ACCESSED is hardcoded 0.
 - riscv is just a clear
 - s390 is just a clear
 - x86 is just a clear
 - pgtable-generic does a TLB flush. Unclear if most/all users using it actually
require it, or if it's just nonsense.

For now, the tlb_remove_pmd_tlb_entry() (and corresponding tlb flush in the PTE code)
does the job.

Anyway, the point being that I kinda want to avoid these flushes, but it's
not particularly trivial ATM, with the tlb_gather API. I'll try to remember
and send some (hopefully tasteful) patches next cycle, on top of Gregory's
make-madvise-cold-not-hurt-my-eyes patches that will have surely landed
by then.

> 
> >
> > This bug can interact with the swapping system in complicated ways, leading
> > to data loss. For instance, if a folio is marked lazyfree, but was later
> > read from, and then MADV_COLD'd while racing a write to that same THP,
> > dropping the dirty bit may lead to data loss as reclaim/MADV_PAGEOUT drops
> > the "clean" anon folio:
> >
> > T0                                T1
> > ================================================================
> > MADV_FREE(thp)                  |
> >   folio marked lazyfree         |
> > read(thp)                       |
> >   PMD marked young by hardware  |
> >                                 | MADV_COLD(thp)
> > write(thp)                      |   if (pmd_young(pmd)) {
> >   PMD marked dirty by hardware  |     set(pmdp, pmd_mkold(pmd));
> > MADV_PAGEOUT/reclaim            |
> >   dirty bit may or may not be   |
> >   set                           |
> >     reclaim/pageout may drop    |
> >     the folio                   |
> >
> 
> Thanks, nice explanation (even if the >quoting ruins the indentation on my editor
> :P) [untabbed for my benefit]

Thanks for the review :) indeed the tabbing looks ugly when quoted.

-- 
Pedro

Re: [PATCH] mm/madvise: atomically clear the young bit on THP MADV_COLD

[Gregory Price]

On Wed, Oct 14, 2026 at 04:52:59PM +0100, Pedro Falcato wrote:
> On Wed, Oct 14, 2026 at 04:37:59PM +0100, Lorenzo Stoakes (ARM) wrote:
> > +cc Gregory
> > 
> > On Tue, Oct 13, 2026 at 08:17:19PM +0100, Pedro Falcato wrote:
> > > As-is, madvise_cold_or_pageout_pte_range() discards the hardware young
> > 
> > The function from hell... Gregory - PLEASE FIX THIS SOON :))
> 
> :))
>

Sending up selftests and reworks shortly.