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

[David Hildenbrand (Arm)]

On 10/29/26 05:53, xu.xin@xxxxxxxxx wrote:
>> 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.

Yes. And doing it under the lock is something the original AI fix missed.

Thanks!

-- 
Cheers,

David