Re: [PATCH 33/46] mm/madvise: fix data loss from MADV_FREE on a KSM page

[xu . xin]

> MADV_FREE can clear the dirty flag of a singly mapped KSM folio. The
> folio remains in the stable tree and can gain new mappings without being
> dirtied again. Once marked lazyfree, reclaim can discard data that those
> mappings still need.
> 
> KSM always marks a folio dirty when converting it to a KSM folio in
> try_to_merge_one_page(). Marking a folio lazyfree clears PG_swapbacked
> but leaves PG_dirty unchanged. Preserving PG_dirty makes reclaim restore
> PG_swapbacked instead of discarding the folio. This also protects folios
> that were already lazyfree when KSM converted them.
> 
> Skip KSM folios before clearing their dirty flag, checking under the
> folio lock to serialize with KSM conversion.
> 
> Fixes: 854e9ed09ded ("mm: support madvise(MADV_FREE)")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> ---
>  mm/madvise.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 75b409e97dbf2..a16e3cb256b7b 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -756,6 +756,15 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
>  		if (folio_test_swapcache(folio) || folio_test_dirty(folio)) {
>  			if (!folio_trylock(folio))
>  				continue;
> +			/*
> +			 * KSM folios can gain new mappings at any time, so we must
> +			 * not clear their dirty flag.
> +			 */
> +			if (folio_test_ksm(folio)) {
> +				folio_unlock(folio);
> +				continue;
> +			}
> +
>  			/*
>  			 * If we have a large folio at this point, we know it is
>  			 * fully mapped so if its mapcount is the same as its
> 
> -- 
> 2.43.0

The fix looks correct to me: checking folio_test_ksm() under the
folio lock properly serializes with KSM conversion, and skipping
the folio avoids clearing PG_dirty on a KSM folio.

Reviewed-by: Xu Xin <xu.xin@xxxxxxxxx>