Re: [RESEND PATCH] mm/madvise: use folio_trylock() in the cold/pageout PMD split

["David Hildenbrand (Arm)" <david@kernel.org>]

On 9/12/26 13:05, Gregory Price wrote:
> MADV_COLD or MADV_PAGEOUT over part of a PMD splits the THP in
> madvise_cold_or_pageout_pte_range().  Two threads doing that to
> the same THP create spurious failures.
> 
>   CPU0                          CPU1
>   ----                          ----
>   folio_get()
>   spin_unlock(ptl)
>   folio_lock()
>                                 folio_get()
>                                 spin_unlock(ptl)
>                                 folio_lock()  <- blocks, keeps its ref
>   split_folio()
>     folio_expected_ref_count(folio) != folio_ref_count(folio) - 1
>     -EAGAIN
> 
> CPU1 cannot drop its reference until it gets the lock CPU0 holds, so CPU0's
> split always fails.  folio_trylock() makes CPU1 leave without ever taking a
> reference.  The PTE branch of this same function already does this, as do
> madvise_free_pte_range() and madvise_free_huge_pmd().
> 
> Reproducer: 400 rounds of eight threads calling MADV_COLD on half of each
> of eight THPs, re-formed with MADV_COLLAPSE between rounds.  From
> /proc/vmstat:
> 
>                      thp_split_page   thp_split_page_failed
>     before                     3186                     860
>     after                      3200                       0
> 
> The short before count is rounds where every thread failed and the
> advice was dropped for that THP entirely.

While the split now succeeds, one of both calls will just effectively skip
processing the page table. SO while CPU0 will succeed with the split, CPU1 would
just skip the page table.

And what happened before?

Split on CPU0 failed and it would skip the page table. Split on CPU1, however,
would likely have succeeded?

So this is all far from perfect.

... but this matches what we do further down in the PTE scenario.

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

-- 
Cheers,

David