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

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

+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