Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap

[David Hildenbrand (Arm)]

On 8/19/26 16:13, David Hildenbrand (Arm) wrote:
> On 8/19/26 12:12, Usama Arif wrote:
>> zap_huge_pmd_folio() propagates the pmd young bit to the folio for the
>> file case, but not the dirty bit.  The pte path does propagate it, in
>> zap_present_folio_ptes() and so does the pmd split path, in
>> __split_huge_pmd_locked().
>>
>> For most file mappings the omission is harmless, because writing to a
>> shared file mapping goes through page_mkwrite(), which dirties the
>> folio.  tmpfs is different: it has no page_mkwrite(), and
>> vma_wants_writenotify() is false for it, so a *read* fault on a
>> MAP_SHARED tmpfs mapping installs a writable pmd via do_read_fault().
>> do_read_fault() does not call fault_dirty_shared_page(), so subsequent
>> stores through that mapping set only the hardware dirty bit in the pmd
>> and never call folio_mark_dirty().  A shmem folio allocated by a fault
>> is marked uptodate but not dirty (see the clear: block in
>> shmem_get_folio_gfp()), so PG_dirty is never set at all.
>>
>> Unmapping such a folio - munmap(), or exit_mmap() when the process dies
>> - then loses the only record that it was written, because zap_huge_pmd()
>> drops the pmd without transferring the dirty bit.  Reclaim afterwards
>> sees a clean shmem folio: the whole swap-out block in
>> shrink_folio_list() is inside "if (folio_test_dirty(folio))", so
>> pageout() is skipped and the folio falls into __remove_mapping().
>> There, folio_is_file_lru() is false for a swapbacked folio, so no shadow
>> entry is created and __filemap_remove_folio(folio, NULL) simply empties
>> the i_pages slot.  The data is freed without ever being written to swap,
>> and the next fault on that index returns a freshly zeroed folio.
>>
>> This is silent data loss for any process that keeps state in a
>> MAP_SHARED tmpfs segment across an unmap - for example a cache handed
>> from one process generation to the next through /dev/shm.  It requires
>> the folio to be PMD-mapped, so it only shows up once shmem THP is
>> enabled (which is what we did in Meta fleet and started noticing crashes);
>> with THP off the pte path transfers the dirty bit correctly.
>> It also only becomes visible when swap is enabled, because with no swap
>> device shmem folios (which are on the anon LRU) are not scanned by
>> reclaim at all, so the clean folio is never dropped.
>>
>> Reproduced on x86_64 with a tmpfs mounted huge=within_size: read-fault a
>> 2MB-backed region, write a known pattern through the resulting mapping,
>> munmap, force reclaim of the cgroup, then re-map and read back.  Without
>> this patch the region reads back as zeros and vmstat shows zswpout 0 -
>> the data was discarded rather than swapped.  With this patch the region
>> reads back correctly and the pages are swapped out as expected.  With
>> huge=never, or when the first touch is a write, the test passes either
>> way.
>>
>> Fixes: 800d8c63b2e9 ("shmem: add huge pages support")
>> Cc: <stable@xxxxxxxxxxxxxxx>
>> Signed-off-by: Usama Arif <usama.arif@xxxxxxxxx>
>> ---
>>  mm/huge_memory.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index ced400f72d43a..afbb5974bd225 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -2449,6 +2449,8 @@ static void zap_huge_pmd_folio(struct mm_struct *mm, struct vm_area_struct *vma,
>>  		add_mm_counter(mm, mm_counter_file(folio),
>>  			       -HPAGE_PMD_NR);
>>  
>> +		if (is_present && pmd_dirty(pmdval))
>> +			folio_mark_dirty(folio);
> 
> We don't need that for anon folios, though. So best to just resemble
> zap_present_folio_ptes() logic and do it only for !anon folios?
> 

I'm stupid, that check is not visible in the diff above :)

Thanks!

Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

-- 
Cheers,

David