Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
@ 2026-08-19 10:12 Usama Arif
  2026-08-19 14:13 ` David Hildenbrand (Arm)
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Usama Arif @ 2026-08-19 10:12 UTC (permalink / raw)
  To: Andrew Morton, baohua, baolin.wang, david, dev.jain, lance.yang,
	liam, linux-kernel, linux-mm, ljs, nico.pache, ryan.roberts,
	usama.arif, ziy, kas, nphamcs, hannes, riel, shakeel.butt,
	kernel-team
  Cc: stable

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@vger.kernel.org>
Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
 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);
 		if (is_present && pmd_young(pmdval) &&
 		    likely(vma_has_recency(vma)))
 			folio_mark_accessed(folio);
-- 
2.53.0-Meta



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 10:12 [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap Usama Arif
@ 2026-08-19 14:13 ` David Hildenbrand (Arm)
  2026-08-19 14:13   ` David Hildenbrand (Arm)
  2026-08-19 14:31 ` Kiryl Shutsemau
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: David Hildenbrand (Arm) @ 2026-08-19 14:13 UTC (permalink / raw)
  To: Usama Arif, Andrew Morton, baohua, baolin.wang, dev.jain,
	lance.yang, liam, linux-kernel, linux-mm, ljs, nico.pache,
	ryan.roberts, ziy, kas, nphamcs, hannes, riel, shakeel.butt,
	kernel-team
  Cc: stable

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@vger.kernel.org>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> ---
>  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?

-- 
Cheers,

David


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 14:13 ` David Hildenbrand (Arm)
@ 2026-08-19 14:13   ` David Hildenbrand (Arm)
  0 siblings, 0 replies; 17+ messages in thread
From: David Hildenbrand (Arm) @ 2026-08-19 14:13 UTC (permalink / raw)
  To: Usama Arif, Andrew Morton, baohua, baolin.wang, dev.jain,
	lance.yang, liam, linux-kernel, linux-mm, ljs, nico.pache,
	ryan.roberts, ziy, kas, nphamcs, hannes, riel, shakeel.butt,
	kernel-team
  Cc: stable

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@vger.kernel.org>
>> Signed-off-by: Usama Arif <usama.arif@linux.dev>
>> ---
>>  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@kernel.org>

-- 
Cheers,

David


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 10:12 [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap Usama Arif
  2026-08-19 14:13 ` David Hildenbrand (Arm)
@ 2026-08-19 14:31 ` Kiryl Shutsemau
  2026-08-19 16:17   ` Lance Yang
                     ` (2 more replies)
  2026-08-19 15:10 ` Lance Yang
                   ` (3 subsequent siblings)
  5 siblings, 3 replies; 17+ messages in thread
From: Kiryl Shutsemau @ 2026-08-19 14:31 UTC (permalink / raw)
  To: Usama Arif, Hugh Dickins
  Cc: Andrew Morton, baohua, baolin.wang, david, dev.jain, lance.yang,
	liam, linux-kernel, linux-mm, ljs, nico.pache, ryan.roberts, ziy,
	nphamcs, hannes, riel, shakeel.butt, kernel-team, stable

On Wed, Aug 19, 2026 at 03:12:22AM -0700, 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.

+Hugh.

Oopsie.

I'm confused why it took a decade to discover the bug...
Maybe read ahead of write for shmem is too rare, I donno.

> 
> Fixes: 800d8c63b2e9 ("shmem: add huge pages support")

This would be more precise: b5072380eb61 ("thp: support file pages in zap_huge_pmd()")

Reviewed-by: Kiryl Shutsemau <kas@kernel.org>

> Cc: <stable@vger.kernel.org>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> ---
>  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);

Unrelated to your patch, but noticed while looking at it: we drop the rmap
here under the pmd lock, while the TLB flush is deferred to
tlb_finish_mmu(). The pte path handles this with
tlb_delay_rmap()/force_flush (5df397dec7c4), but there's no pmd equivalent:
tlb_flush_rmap_batch() only knows folio_remove_rmap_ptes(), and
zap_huge_pmd() uses tlb_remove_page_size(), which takes no delay_rmap.

Doesn't matter for shmem, but xfs & friends do get PMD-order folios, and
do_set_pmd() makes the pmd dirty+writable once page_mkwrite() has run. So
folio_mkclean() can clean the folio while another CPU still stores through a
stale TLB entry -- silently lost write, no PG_dirty left behind.

I think we need to fix this too.

Wanna give it a try?

>  		if (is_present && pmd_young(pmdval) &&
>  		    likely(vma_has_recency(vma)))
>  			folio_mark_accessed(folio);
> -- 
> 2.53.0-Meta
> 

-- 
  Kiryl Shutsemau / Kirill A. Shutemov


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 10:12 [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap Usama Arif
  2026-08-19 14:13 ` David Hildenbrand (Arm)
  2026-08-19 14:31 ` Kiryl Shutsemau
@ 2026-08-19 15:10 ` Lance Yang
  2026-08-19 15:31 ` Zi Yan
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Lance Yang @ 2026-08-19 15:10 UTC (permalink / raw)
  To: usama.arif
  Cc: akpm, baohua, baolin.wang, david, dev.jain, liam, linux-kernel,
	linux-mm, ljs, nico.pache, ryan.roberts, ziy, kas, nphamcs,
	hannes, riel, shakeel.butt, kernel-team, stable, Lance Yang


On Wed, Aug 19, 2026 at 03:12:22AM -0700, 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@vger.kernel.org>
>Signed-off-by: Usama Arif <usama.arif@linux.dev>
>---

Good catch!

Tested-by: Lance Yang <lance.yang@linux.dev>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 10:12 [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap Usama Arif
                   ` (2 preceding siblings ...)
  2026-08-19 15:10 ` Lance Yang
@ 2026-08-19 15:31 ` Zi Yan
  2026-08-19 16:09 ` Lorenzo Stoakes (ARM)
  2026-08-20  2:06 ` Baolin Wang
  5 siblings, 0 replies; 17+ messages in thread
From: Zi Yan @ 2026-08-19 15:31 UTC (permalink / raw)
  To: Usama Arif, Andrew Morton, baohua, baolin.wang, david, dev.jain,
	lance.yang, liam, linux-kernel, linux-mm, ljs, nico.pache,
	ryan.roberts, kas, nphamcs, hannes, riel, shakeel.butt,
	kernel-team
  Cc: stable

On Wed Aug 19, 2026 at 6:12 AM EDT, 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@vger.kernel.org>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> ---
>  mm/huge_memory.c | 2 ++
>  1 file changed, 2 insertions(+)
>
Make sense. Thanks.

Reviewed-by: Zi Yan <ziy@nvidia.com>

-- 
Best Regards,
Yan, Zi



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 10:12 [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap Usama Arif
                   ` (3 preceding siblings ...)
  2026-08-19 15:31 ` Zi Yan
@ 2026-08-19 16:09 ` Lorenzo Stoakes (ARM)
  2026-08-20  2:06 ` Baolin Wang
  5 siblings, 0 replies; 17+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-19 16:09 UTC (permalink / raw)
  To: Usama Arif
  Cc: Andrew Morton, baohua, baolin.wang, david, dev.jain, lance.yang,
	liam, linux-kernel, linux-mm, nico.pache, ryan.roberts, ziy, kas,
	nphamcs, hannes, riel, shakeel.butt, kernel-team, stable

On Wed, Aug 19, 2026 at 03:12:22AM -0700, 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.

Good lord.

>
> 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@vger.kernel.org>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>

Thanks for this! LGTM so:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  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);
>  		if (is_present && pmd_young(pmdval) &&
>  		    likely(vma_has_recency(vma)))
>  			folio_mark_accessed(folio);
> --
> 2.53.0-Meta
>

--
Cheers, Lorenzo


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 14:31 ` Kiryl Shutsemau
@ 2026-08-19 16:17   ` Lance Yang
  2026-08-19 16:32     ` Usama Arif
  2026-08-19 16:35   ` Pedro Falcato
  2026-08-19 20:33   ` Hugh Dickins
  2 siblings, 1 reply; 17+ messages in thread
From: Lance Yang @ 2026-08-19 16:17 UTC (permalink / raw)
  To: kas
  Cc: usama.arif, hughd, akpm, baohua, baolin.wang, david, dev.jain,
	liam, linux-kernel, linux-mm, ljs, nico.pache, ryan.roberts, ziy,
	nphamcs, hannes, riel, shakeel.butt, kernel-team, stable,
	Lance Yang


On Wed, Aug 19, 2026 at 03:31:40PM +0100, Kiryl Shutsemau wrote:
>On Wed, Aug 19, 2026 at 03:12:22AM -0700, Usama Arif wrote:
[...]
>> ---
>>  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);
>
>Unrelated to your patch, but noticed while looking at it: we drop the rmap
>here under the pmd lock, while the TLB flush is deferred to
>tlb_finish_mmu(). The pte path handles this with
>tlb_delay_rmap()/force_flush (5df397dec7c4), but there's no pmd equivalent:
>tlb_flush_rmap_batch() only knows folio_remove_rmap_ptes(), and
>zap_huge_pmd() uses tlb_remove_page_size(), which takes no delay_rmap.

Well spotted!

>Doesn't matter for shmem, but xfs & friends do get PMD-order folios, and

Right. pageout() cannot pass its refcount check while PMD mapping still
holds an extra folio ref, and mmu_gather drops that ref only after TLB
flush.

>do_set_pmd() makes the pmd dirty+writable once page_mkwrite() has run. So
>folio_mkclean() can clean the folio while another CPU still stores through a
>stale TLB entry -- silently lost write, no PG_dirty left behind.

Yep. Writeback can run folio_mkclean() while that ref is still held,
though, and with rmap already gone it misses the PMD ...

>I think we need to fix this too.

+1

>Wanna give it a try?

zap_huge_pmd() only handles one PMD under PTL anyway ... how about just
flushing before folio_remove_rmap_pmd()?

---8<---
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index afbb5974bd22..6fb34924ef66 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2531,6 +2531,14 @@ bool zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
 	is_present = pmd_present(orig_pmd);
 	folio = normal_or_softleaf_folio_pmd(vma, addr, orig_pmd, is_present);
 	has_deposit = has_deposited_pgtable(vma, orig_pmd, folio);
+	/*
+	 * folio_mkclean() relies on the rmap to find writable mappings.
+	 * Flush stale TLB entries before removing it below.
+	 */
+	if (folio && is_present && !folio_test_anon(folio) &&
+	    pmd_dirty(orig_pmd))
+		tlb_flush_mmu_tlbonly(tlb);
+
 	if (folio)
 		zap_huge_pmd_folio(mm, vma, orig_pmd, folio, is_present);
 	if (has_deposit)
---

Cheers, Lance

>
>>  		if (is_present && pmd_young(pmdval) &&
>>  		    likely(vma_has_recency(vma)))
>>  			folio_mark_accessed(folio);
>> -- 
>> 2.53.0-Meta
>> 
>
>-- 
>  Kiryl Shutsemau / Kirill A. Shutemov
>


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 16:17   ` Lance Yang
@ 2026-08-19 16:32     ` Usama Arif
  2026-08-20 13:05       ` Usama Arif
  0 siblings, 1 reply; 17+ messages in thread
From: Usama Arif @ 2026-08-19 16:32 UTC (permalink / raw)
  To: Lance Yang, kas
  Cc: hughd, akpm, baohua, baolin.wang, david, dev.jain, liam,
	linux-kernel, linux-mm, ljs, nico.pache, ryan.roberts, ziy,
	nphamcs, hannes, riel, shakeel.butt, kernel-team, stable



On 19/08/2026 17:17, Lance Yang wrote:
> 
> On Wed, Aug 19, 2026 at 03:31:40PM +0100, Kiryl Shutsemau wrote:
>> On Wed, Aug 19, 2026 at 03:12:22AM -0700, Usama Arif wrote:
> [...]
>>> ---
>>>  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);
>>
>> Unrelated to your patch, but noticed while looking at it: we drop the rmap
>> here under the pmd lock, while the TLB flush is deferred to
>> tlb_finish_mmu(). The pte path handles this with
>> tlb_delay_rmap()/force_flush (5df397dec7c4), but there's no pmd equivalent:
>> tlb_flush_rmap_batch() only knows folio_remove_rmap_ptes(), and
>> zap_huge_pmd() uses tlb_remove_page_size(), which takes no delay_rmap.
> 
> Well spotted!
> 
>> Doesn't matter for shmem, but xfs & friends do get PMD-order folios, and
> 
> Right. pageout() cannot pass its refcount check while PMD mapping still
> holds an extra folio ref, and mmu_gather drops that ref only after TLB
> flush.
> 
>> do_set_pmd() makes the pmd dirty+writable once page_mkwrite() has run. So
>> folio_mkclean() can clean the folio while another CPU still stores through a
>> stale TLB entry -- silently lost write, no PG_dirty left behind.
> 
> Yep. Writeback can run folio_mkclean() while that ref is still held,
> though, and with rmap already gone it misses the PMD ...
> 
>> I think we need to fix this too.
> 
> +1
> 
>> Wanna give it a try?
> 
> zap_huge_pmd() only handles one PMD under PTL anyway ... how about just
> flushing before folio_remove_rmap_pmd()?
> 
> ---8<---
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index afbb5974bd22..6fb34924ef66 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2531,6 +2531,14 @@ bool zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>  	is_present = pmd_present(orig_pmd);
>  	folio = normal_or_softleaf_folio_pmd(vma, addr, orig_pmd, is_present);
>  	has_deposit = has_deposited_pgtable(vma, orig_pmd, folio);
> +	/*
> +	 * folio_mkclean() relies on the rmap to find writable mappings.
> +	 * Flush stale TLB entries before removing it below.
> +	 */
> +	if (folio && is_present && !folio_test_anon(folio) &&
> +	    pmd_dirty(orig_pmd))
> +		tlb_flush_mmu_tlbonly(tlb);
> +
>  	if (folio)
>  		zap_huge_pmd_folio(mm, vma, orig_pmd, folio, is_present);
>  	if (has_deposit)
> ---


I am currently at below to reduce tlb flushes, but still WIP


diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index bdcc2778ac64f..60bdd6287b5a9 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -301,6 +301,12 @@ bool __tlb_remove_folio_pages(struct mmu_gather *tlb, struct page *page,
  * function, except we define it before the 'struct mmu_gather'.
  */
 #define tlb_delay_rmap(tlb) (((tlb)->delayed_rmap = 1), true)
+/*
+ * Like tlb_delay_rmap() but without the side effect, for callers that must
+ * flush rather than delay: can another CPU still reach this mapping through a
+ * stale TLB entry once its rmap entry is gone? Not during fullmm teardown.
+ */
+#define tlb_rmap_needs_flush(tlb) (!(tlb)->fullmm)
 extern void tlb_flush_rmaps(struct mmu_gather *tlb, struct vm_area_struct *vma);
 #endif

@@ -315,6 +321,7 @@ extern void tlb_flush_rmaps(struct mmu_gather *tlb, struct vm_area_struct *vma);
  */
 #ifndef tlb_delay_rmap
 #define tlb_delay_rmap(tlb) (false)
+#define tlb_rmap_needs_flush(tlb) (false)
 static inline void tlb_flush_rmaps(struct mmu_gather *tlb, struct vm_area_struct *vma) { }
 #endif

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index afbb5974bd225..76d8d5cf92ee0 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2493,6 +2493,33 @@ static bool has_deposited_pgtable(struct vm_area_struct *vma, pmd_t pmdval,
        return folio && folio_test_anon(folio);
 }


+static bool pmd_zap_needs_tlb_flush(struct mmu_gather *tlb, pmd_t pmdval,
+                                   struct folio *folio, bool is_present)
+{
+       struct address_space *mapping;
+
+       if (!is_present || !pmd_dirty(pmdval) || folio_test_anon(folio))
+               return false;
+       if (!tlb_rmap_needs_flush(tlb))
+               return false;
+
+       mapping = folio_mapping(folio);
+       return mapping && mapping_can_writeback(mapping);
+}
+
 /**
  * zap_huge_pmd - Zap a huge THP which is of PMD size.
  * @tlb: The MMU gather TLB state associated with the operation.
@@ -2531,8 +2558,15 @@ bool zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
        is_present = pmd_present(orig_pmd);
        folio = normal_or_softleaf_folio_pmd(vma, addr, orig_pmd, is_present);
        has_deposit = has_deposited_pgtable(vma, orig_pmd, folio);
-       if (folio)
+       if (folio) {
+               /* Flush before zap_huge_pmd_folio() drops the rmap entry. */
+               if (pmd_zap_needs_tlb_flush(tlb, orig_pmd, folio, is_present)) {
+                       tlb_flush_mmu_tlbonly(tlb);
+                       /* Re-arm: tlb_remove_page_size() needs tlb->end set. */
+                       tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
+               }
                zap_huge_pmd_folio(mm, vma, orig_pmd, folio, is_present);
+       }
        if (has_deposit)
                zap_deposited_table(mm, pmd);

> 
> Cheers, Lance
> 
>>
>>>  		if (is_present && pmd_young(pmdval) &&
>>>  		    likely(vma_has_recency(vma)))
>>>  			folio_mark_accessed(folio);
>>> -- 
>>> 2.53.0-Meta
>>>
>>
>> -- 
>>  Kiryl Shutsemau / Kirill A. Shutemov
>>



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 14:31 ` Kiryl Shutsemau
  2026-08-19 16:17   ` Lance Yang
@ 2026-08-19 16:35   ` Pedro Falcato
  2026-08-20  6:13     ` Lance Yang
  2026-08-19 20:33   ` Hugh Dickins
  2 siblings, 1 reply; 17+ messages in thread
From: Pedro Falcato @ 2026-08-19 16:35 UTC (permalink / raw)
  To: Kiryl Shutsemau
  Cc: Usama Arif, Hugh Dickins, Andrew Morton, baohua, baolin.wang,
	david, dev.jain, lance.yang, liam, linux-kernel, linux-mm, ljs,
	nico.pache, ryan.roberts, ziy, nphamcs, hannes, riel,
	shakeel.butt, kernel-team, stable

On Wed, Aug 19, 2026 at 03:31:40PM +0100, Kiryl Shutsemau wrote:
> On Wed, Aug 19, 2026 at 03:12:22AM -0700, 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.
> 
> +Hugh.
> 
> Oopsie.
> 
> I'm confused why it took a decade to discover the bug...
> Maybe read ahead of write for shmem is too rare, I donno.
> 
> > 
> > Fixes: 800d8c63b2e9 ("shmem: add huge pages support")
> 
> This would be more precise: b5072380eb61 ("thp: support file pages in zap_huge_pmd()")
> 
> Reviewed-by: Kiryl Shutsemau <kas@kernel.org>
> 
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Usama Arif <usama.arif@linux.dev>
> > ---
> >  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);
> 
> Unrelated to your patch, but noticed while looking at it: we drop the rmap
> here under the pmd lock, while the TLB flush is deferred to
> tlb_finish_mmu(). The pte path handles this with
> tlb_delay_rmap()/force_flush (5df397dec7c4), but there's no pmd equivalent:
> tlb_flush_rmap_batch() only knows folio_remove_rmap_ptes(), and
> zap_huge_pmd() uses tlb_remove_page_size(), which takes no delay_rmap.
> 
> Doesn't matter for shmem, but xfs & friends do get PMD-order folios, and
> do_set_pmd() makes the pmd dirty+writable once page_mkwrite() has run. So
> folio_mkclean() can clean the folio while another CPU still stores through a
> stale TLB entry -- silently lost write, no PG_dirty left behind.

Where do you see page_mkwrite being called in the same path as do_set_pmd()?
Per my understanding of the code, this Should Not Happen, and it really Should
Not Happen for many, many reasons (write amplification being the main one).

Namely, see the comment in wp_huge_pmd():
	/* COW or write-notify handled on pte level: split pmd. */

if file huge pages get mapped writable, that's a bug.
 

-- 
Pedro


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 14:31 ` Kiryl Shutsemau
  2026-08-19 16:17   ` Lance Yang
  2026-08-19 16:35   ` Pedro Falcato
@ 2026-08-19 20:33   ` Hugh Dickins
  2 siblings, 0 replies; 17+ messages in thread
From: Hugh Dickins @ 2026-08-19 20:33 UTC (permalink / raw)
  To: Kiryl Shutsemau
  Cc: Usama Arif, Hugh Dickins, Andrew Morton, baohua, baolin.wang,
	david, dev.jain, lance.yang, liam, linux-kernel, linux-mm, ljs,
	nico.pache, ryan.roberts, ziy, nphamcs, hannes, riel,
	shakeel.butt, kernel-team, stable

On Wed, 19 Aug 2026, Kiryl Shutsemau wrote:
> On Wed, Aug 19, 2026 at 03:12:22AM -0700, 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.
> 
> +Hugh.
> 
> Oopsie.

How ghastly! Thanks for finding and fixing, Usama.

> 
> I'm confused why it took a decade to discover the bug...
> Maybe read ahead of write for shmem is too rare, I donno.

It isn't entirely clear from the report, but this is all about modifying
a 2MiB+ *hole* in a shmem file through an mmap thereof, with first fault
a read fault not a write fault.  I suppose only a few proceed in that way
(though truncating an empty file to some size and then mmap'ing that size
is very normal).

Or everybody who tried to report this bug, wrote their report into a
2MiB hole in a shmem file through an mmap thereof.

Other than holes, all shmem folios are dirty throughout (and re-marked
dirty as soon as brought back from swap): so for most, it doesn't matter
what the pmd says.

This raised a dim memory, took a while to locate what I was remembering:
e1f1b1572e8d ("mm/huge_memory.c: fix data loss when splitting a file pmd")
from 2018.

Not quite the same; but what a pity that one didn't prompt any of us
to look further and find what Usama now has.

> 
> > 
> > Fixes: 800d8c63b2e9 ("shmem: add huge pages support")
> 
> This would be more precise: b5072380eb61 ("thp: support file pages in zap_huge_pmd()")
> 
> Reviewed-by: Kiryl Shutsemau <kas@kernel.org>
> 
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Usama Arif <usama.arif@linux.dev>

Acked-by: Hugh Dickins <hughd@google.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 10:12 [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap Usama Arif
                   ` (4 preceding siblings ...)
  2026-08-19 16:09 ` Lorenzo Stoakes (ARM)
@ 2026-08-20  2:06 ` Baolin Wang
  5 siblings, 0 replies; 17+ messages in thread
From: Baolin Wang @ 2026-08-20  2:06 UTC (permalink / raw)
  To: Usama Arif, Andrew Morton, baohua, david, dev.jain, lance.yang,
	liam, linux-kernel, linux-mm, ljs, nico.pache, ryan.roberts, ziy,
	kas, nphamcs, hannes, riel, shakeel.butt, kernel-team
  Cc: stable



On 8/19/26 6:12 PM, 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@vger.kernel.org>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> ---

Thanks for the fix. Feel free to add:
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 16:35   ` Pedro Falcato
@ 2026-08-20  6:13     ` Lance Yang
  2026-08-20 12:12       ` Pedro Falcato
  0 siblings, 1 reply; 17+ messages in thread
From: Lance Yang @ 2026-08-20  6:13 UTC (permalink / raw)
  To: pfalcato
  Cc: kas, usama.arif, hughd, akpm, baohua, baolin.wang, david,
	dev.jain, lance.yang, liam, linux-kernel, linux-mm, ljs,
	nico.pache, ryan.roberts, ziy, nphamcs, hannes, riel,
	shakeel.butt, kernel-team, stable


On Wed, Aug 19, 2026 at 05:35:41PM +0100, Pedro Falcato wrote:
>On Wed, Aug 19, 2026 at 03:31:40PM +0100, Kiryl Shutsemau wrote:
>> On Wed, Aug 19, 2026 at 03:12:22AM -0700, 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.
>> 
>> +Hugh.
>> 
>> Oopsie.
>> 
>> I'm confused why it took a decade to discover the bug...
>> Maybe read ahead of write for shmem is too rare, I donno.
>> 
>> > 
>> > Fixes: 800d8c63b2e9 ("shmem: add huge pages support")
>> 
>> This would be more precise: b5072380eb61 ("thp: support file pages in zap_huge_pmd()")
>> 
>> Reviewed-by: Kiryl Shutsemau <kas@kernel.org>
>> 
>> > Cc: <stable@vger.kernel.org>
>> > Signed-off-by: Usama Arif <usama.arif@linux.dev>
>> > ---
>> >  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);
>> 
>> Unrelated to your patch, but noticed while looking at it: we drop the rmap
>> here under the pmd lock, while the TLB flush is deferred to
>> tlb_finish_mmu(). The pte path handles this with
>> tlb_delay_rmap()/force_flush (5df397dec7c4), but there's no pmd equivalent:
>> tlb_flush_rmap_batch() only knows folio_remove_rmap_ptes(), and
>> zap_huge_pmd() uses tlb_remove_page_size(), which takes no delay_rmap.
>> 
>> Doesn't matter for shmem, but xfs & friends do get PMD-order folios, and
>> do_set_pmd() makes the pmd dirty+writable once page_mkwrite() has run. So
>> folio_mkclean() can clean the folio while another CPU still stores through a
>> stale TLB entry -- silently lost write, no PG_dirty left behind.
>
>Where do you see page_mkwrite being called in the same path as do_set_pmd()?
>Per my understanding of the code, this Should Not Happen, and it really Should
>Not Happen for many, many reasons (write amplification being the main one).

Hmm.. that happens on an initial shared write fault. For non-DAX XFS, the
path starts with an empty PMD.

TL;DR

With an empty PMD and PMD-order THP allowed, __handle_mm_fault() first
tries create_huge_pmd(). VM_FAULT_FALLBACK sends the fault to
handle_pte_fault():

static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma,
		unsigned long address, unsigned int flags)
{
	...
	if (pmd_none(*vmf.pmd) &&
	    thp_vma_allowable_order(vma, vm_flags, TVA_PAGEFAULT, PMD_ORDER)) {
		ret = create_huge_pmd(&vmf);
		if (ret & VM_FAULT_FALLBACK)
			goto fallback;
		else
			return ret;
	}
	...
fallback:
	return handle_pte_fault(&vmf);
}

create_huge_pmd() dispatches to the filesystem's huge_fault callback:

static inline vm_fault_t create_huge_pmd(struct vm_fault *vmf)
{
	struct vm_area_struct *vma = vmf->vma;
	...
	if (vma->vm_ops->huge_fault)
		return vma->vm_ops->huge_fault(vmf, PMD_ORDER);
	return VM_FAULT_FALLBACK;
}

For non-DAX XFS, that callback returns VM_FAULT_FALLBACK:

static vm_fault_t
xfs_filemap_huge_fault(
	struct vm_fault		*vmf,
	unsigned int		order)
{
	if (!IS_DAX(file_inode(vmf->vma->vm_file)))
		return VM_FAULT_FALLBACK;
	...
}

XFS installs the huge-fault, regular-fault, and page_mkwrite callbacks
in the same vm_ops:

static const struct vm_operations_struct xfs_file_vm_ops = {
	.fault		= xfs_filemap_fault,
	.huge_fault	= xfs_filemap_huge_fault,
	...
	.page_mkwrite	= xfs_filemap_page_mkwrite,
	...
};

On the fallback path, handle_pte_fault() leaves an empty PMD without a
PTE and calls do_pte_missing():

static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
{
	...
	if (unlikely(pmd_none(*vmf->pmd))) {
		/*
		 * Leave __pte_alloc() until later: because vm_ops->fault may
		 * want to allocate huge page, and if we expose page table
		 * for an instant, it will be difficult to retract from
		 * concurrent faults and from rmap lookups.
		 */
		vmf->pte = NULL;
		vmf->flags &= ~FAULT_FLAG_ORIG_PTE_VALID;
	...
	}

	if (!vmf->pte)
		return do_pte_missing(vmf);
	...
}

For a file VMA, do_pte_missing() calls do_fault():

static vm_fault_t do_pte_missing(struct vm_fault *vmf)
{
	if (vma_is_anonymous(vmf->vma))
		return do_anonymous_page(vmf);
	else
		return do_fault(vmf);
}

do_fault() sends FAULT_FLAG_WRITE + VM_SHARED to do_shared_fault():

static vm_fault_t do_fault(struct vm_fault *vmf)
{
	struct vm_area_struct *vma = vmf->vma;
	...
	if (!vma->vm_ops->fault) {
		...
	} else if (!(vmf->flags & FAULT_FLAG_WRITE))
		ret = do_read_fault(vmf);
	else if (!(vma->vm_flags & VM_SHARED))
		ret = do_cow_fault(vmf);
	else
		ret = do_shared_fault(vmf);
	...
}

do_shared_fault() first calls __do_fault():

static vm_fault_t do_shared_fault(struct vm_fault *vmf)
{
	struct vm_area_struct *vma = vmf->vma;
	vm_fault_t ret, tmp;
	struct folio *folio;
	...
	ret = __do_fault(vmf);
	...
}

__do_fault() invokes the regular fault callback:

static vm_fault_t __do_fault(struct vm_fault *vmf)
{
	struct vm_area_struct *vma = vmf->vma;
	struct folio *folio;
	vm_fault_t ret;
	...
	ret = vma->vm_ops->fault(vmf);
	...
	return ret;
}

For non-DAX XFS, xfs_filemap_fault() reaches filemap_fault():

static vm_fault_t
xfs_filemap_fault(
	struct vm_fault		*vmf)
{
	struct inode		*inode = file_inode(vmf->vma->vm_file);
	...
	return filemap_fault(vmf);
}

Once that returns the folio, do_shared_fault() calls do_page_mkwrite()
and then finish_fault():

static vm_fault_t do_shared_fault(struct vm_fault *vmf)
{
	struct vm_area_struct *vma = vmf->vma;
	vm_fault_t ret, tmp;
	struct folio *folio;
	...
	folio = page_folio(vmf->page);
	...
	if (vma->vm_ops->page_mkwrite) {
		folio_unlock(folio);
		tmp = do_page_mkwrite(vmf, folio);
		...
	}

	ret |= finish_fault(vmf);
	...
}

do_page_mkwrite() calls the XFS callback installed above and restores
the original fault flags:

static vm_fault_t do_page_mkwrite(struct vm_fault *vmf, struct folio *folio)
{
	vm_fault_t ret;
	unsigned int old_flags = vmf->flags;

	vmf->flags = FAULT_FLAG_WRITE|FAULT_FLAG_MKWRITE;
	...
	ret = vmf->vma->vm_ops->page_mkwrite(vmf);
	/* Restore original flags so that caller is not surprised */
	vmf->flags = old_flags;
	...
}

So finish_fault() still sees FAULT_FLAG_WRITE. With an empty PMD, no
fallback requirement, and a PMD-mappable folio, it tries do_set_pmd():

vm_fault_t finish_fault(struct vm_fault *vmf)
{
	...
	if (pmd_none(*vmf->pmd)) {
		if (!needs_fallback && folio_test_pmd_mappable(folio)) {
			ret = do_set_pmd(vmf, folio, page);
			if (ret != VM_FAULT_FALLBACK)
				return ret;
		}
		...
	}
	...
}

After its checks pass, do_set_pmd() takes FAULT_FLAG_WRITE from vmf and
installs a dirty+writable PMD:

vm_fault_t do_set_pmd(struct vm_fault *vmf, struct folio *folio, struct page *page)
{
	struct vm_area_struct *vma = vmf->vma;
	bool write = vmf->flags & FAULT_FLAG_WRITE;
	unsigned long haddr = vmf->address & HPAGE_PMD_MASK;
	pmd_t entry;
	...
	entry = folio_mk_pmd(folio, vma->vm_page_prot);
	if (write)
		entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
	...
	set_pmd_at(vma->vm_mm, haddr, vmf->pmd, entry);
	...
}

maybe_pmd_mkwrite() sets write permission for VM_WRITE:

pmd_t maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma)
{
	if (likely(vma->vm_flags & VM_WRITE))
		pmd = pmd_mkwrite(pmd, vma);
	return pmd;
}

>Namely, see the comment in wp_huge_pmd():
>	/* COW or write-notify handled on pte level: split pmd. */
>
>if file huge pages get mapped writable, that's a bug.

That comment is about a different path. __handle_mm_fault() calls
wp_huge_pmd() only when a write/unshare fault hits an existing PMD THP
which is not writable:

static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma,
		unsigned long address, unsigned int flags)
{
	...
	if (pmd_trans_huge(vmf.orig_pmd)) {
		...
		if ((flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) &&
		    !pmd_write(vmf.orig_pmd)) {
			ret = wp_huge_pmd(&vmf);
			if (!(ret & VM_FAULT_FALLBACK))
				return ret;
		...
	}
	...
}

If the filesystem huge_fault callback falls back there, wp_huge_pmd()
splits that existing read-only PMD:

static inline vm_fault_t wp_huge_pmd(struct vm_fault *vmf)
{
	struct vm_area_struct *vma = vmf->vma;
	const bool unshare = vmf->flags & FAULT_FLAG_UNSHARE;
	vm_fault_t ret;
	...
	if (vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) {
		if (vma->vm_ops->huge_fault) {
			ret = vma->vm_ops->huge_fault(vmf, PMD_ORDER);
			if (!(ret & VM_FAULT_FALLBACK))
				return ret;
		}
	}

split:
	/* COW or write-notify handled on pte level: split pmd. */
	__split_huge_pmd(vma, vmf->pmd, vmf->address, false);

	return VM_FAULT_FALLBACK;
}

So I don't think that comment says file PMDs must never be writable. It
covers a later write fault against an existing read-only PMD. On the
initial shared write fault above, page_mkwrite runs before do_set_pmd()
installs the writable file PMD.

Cheers, Lance

> 
>
>-- 
>Pedro
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-20  6:13     ` Lance Yang
@ 2026-08-20 12:12       ` Pedro Falcato
  2026-08-20 13:20         ` Kiryl Shutsemau
  0 siblings, 1 reply; 17+ messages in thread
From: Pedro Falcato @ 2026-08-20 12:12 UTC (permalink / raw)
  To: Lance Yang
  Cc: kas, usama.arif, hughd, akpm, baohua, baolin.wang, david,
	dev.jain, liam, linux-kernel, linux-mm, ljs, nico.pache,
	ryan.roberts, ziy, nphamcs, hannes, riel, shakeel.butt,
	kernel-team, stable, willy

[-- Attachment #1: Type: text/plain, Size: 12160 bytes --]

+CC willy

On Thu, Aug 20, 2026 at 02:13:37PM +0800, Lance Yang wrote:
> 
> On Wed, Aug 19, 2026 at 05:35:41PM +0100, Pedro Falcato wrote:
> >On Wed, Aug 19, 2026 at 03:31:40PM +0100, Kiryl Shutsemau wrote:
> >> On Wed, Aug 19, 2026 at 03:12:22AM -0700, 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.
> >> 
> >> +Hugh.
> >> 
> >> Oopsie.
> >> 
> >> I'm confused why it took a decade to discover the bug...
> >> Maybe read ahead of write for shmem is too rare, I donno.
> >> 
> >> > 
> >> > Fixes: 800d8c63b2e9 ("shmem: add huge pages support")
> >> 
> >> This would be more precise: b5072380eb61 ("thp: support file pages in zap_huge_pmd()")
> >> 
> >> Reviewed-by: Kiryl Shutsemau <kas@kernel.org>
> >> 
> >> > Cc: <stable@vger.kernel.org>
> >> > Signed-off-by: Usama Arif <usama.arif@linux.dev>
> >> > ---
> >> >  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);
> >> 
> >> Unrelated to your patch, but noticed while looking at it: we drop the rmap
> >> here under the pmd lock, while the TLB flush is deferred to
> >> tlb_finish_mmu(). The pte path handles this with
> >> tlb_delay_rmap()/force_flush (5df397dec7c4), but there's no pmd equivalent:
> >> tlb_flush_rmap_batch() only knows folio_remove_rmap_ptes(), and
> >> zap_huge_pmd() uses tlb_remove_page_size(), which takes no delay_rmap.
> >> 
> >> Doesn't matter for shmem, but xfs & friends do get PMD-order folios, and
> >> do_set_pmd() makes the pmd dirty+writable once page_mkwrite() has run. So
> >> folio_mkclean() can clean the folio while another CPU still stores through a
> >> stale TLB entry -- silently lost write, no PG_dirty left behind.
> >
> >Where do you see page_mkwrite being called in the same path as do_set_pmd()?
> >Per my understanding of the code, this Should Not Happen, and it really Should
> >Not Happen for many, many reasons (write amplification being the main one).
> 
> Hmm.. that happens on an initial shared write fault. For non-DAX XFS, the
> path starts with an empty PMD.
> 
> TL;DR
> 
> With an empty PMD and PMD-order THP allowed, __handle_mm_fault() first
> tries create_huge_pmd(). VM_FAULT_FALLBACK sends the fault to
> handle_pte_fault():
> 
> static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma,
> 		unsigned long address, unsigned int flags)
> {
> 	...
> 	if (pmd_none(*vmf.pmd) &&
> 	    thp_vma_allowable_order(vma, vm_flags, TVA_PAGEFAULT, PMD_ORDER)) {
> 		ret = create_huge_pmd(&vmf);
> 		if (ret & VM_FAULT_FALLBACK)
> 			goto fallback;
> 		else
> 			return ret;
> 	}
> 	...
> fallback:
> 	return handle_pte_fault(&vmf);
> }
> 
> create_huge_pmd() dispatches to the filesystem's huge_fault callback:
> 
> static inline vm_fault_t create_huge_pmd(struct vm_fault *vmf)
> {
> 	struct vm_area_struct *vma = vmf->vma;
> 	...
> 	if (vma->vm_ops->huge_fault)
> 		return vma->vm_ops->huge_fault(vmf, PMD_ORDER);
> 	return VM_FAULT_FALLBACK;
> }
> 
> For non-DAX XFS, that callback returns VM_FAULT_FALLBACK:
> 
> static vm_fault_t
> xfs_filemap_huge_fault(
> 	struct vm_fault		*vmf,
> 	unsigned int		order)
> {
> 	if (!IS_DAX(file_inode(vmf->vma->vm_file)))
> 		return VM_FAULT_FALLBACK;
> 	...
> }
> 
> XFS installs the huge-fault, regular-fault, and page_mkwrite callbacks
> in the same vm_ops:
> 
> static const struct vm_operations_struct xfs_file_vm_ops = {
> 	.fault		= xfs_filemap_fault,
> 	.huge_fault	= xfs_filemap_huge_fault,
> 	...
> 	.page_mkwrite	= xfs_filemap_page_mkwrite,
> 	...
> };
> 
> On the fallback path, handle_pte_fault() leaves an empty PMD without a
> PTE and calls do_pte_missing():
> 
> static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
> {
> 	...
> 	if (unlikely(pmd_none(*vmf->pmd))) {
> 		/*
> 		 * Leave __pte_alloc() until later: because vm_ops->fault may
> 		 * want to allocate huge page, and if we expose page table
> 		 * for an instant, it will be difficult to retract from
> 		 * concurrent faults and from rmap lookups.
> 		 */
> 		vmf->pte = NULL;
> 		vmf->flags &= ~FAULT_FLAG_ORIG_PTE_VALID;
> 	...
> 	}
> 
> 	if (!vmf->pte)
> 		return do_pte_missing(vmf);
> 	...
> }
> 
> For a file VMA, do_pte_missing() calls do_fault():
> 
> static vm_fault_t do_pte_missing(struct vm_fault *vmf)
> {
> 	if (vma_is_anonymous(vmf->vma))
> 		return do_anonymous_page(vmf);
> 	else
> 		return do_fault(vmf);
> }
> 
> do_fault() sends FAULT_FLAG_WRITE + VM_SHARED to do_shared_fault():
> 
> static vm_fault_t do_fault(struct vm_fault *vmf)
> {
> 	struct vm_area_struct *vma = vmf->vma;
> 	...
> 	if (!vma->vm_ops->fault) {
> 		...
> 	} else if (!(vmf->flags & FAULT_FLAG_WRITE))
> 		ret = do_read_fault(vmf);
> 	else if (!(vma->vm_flags & VM_SHARED))
> 		ret = do_cow_fault(vmf);
> 	else
> 		ret = do_shared_fault(vmf);
> 	...
> }
> 
> do_shared_fault() first calls __do_fault():
> 
> static vm_fault_t do_shared_fault(struct vm_fault *vmf)
> {
> 	struct vm_area_struct *vma = vmf->vma;
> 	vm_fault_t ret, tmp;
> 	struct folio *folio;
> 	...
> 	ret = __do_fault(vmf);
> 	...
> }
> 
> __do_fault() invokes the regular fault callback:
> 
> static vm_fault_t __do_fault(struct vm_fault *vmf)
> {
> 	struct vm_area_struct *vma = vmf->vma;
> 	struct folio *folio;
> 	vm_fault_t ret;
> 	...
> 	ret = vma->vm_ops->fault(vmf);
> 	...
> 	return ret;
> }
> 
> For non-DAX XFS, xfs_filemap_fault() reaches filemap_fault():
> 
> static vm_fault_t
> xfs_filemap_fault(
> 	struct vm_fault		*vmf)
> {
> 	struct inode		*inode = file_inode(vmf->vma->vm_file);
> 	...
> 	return filemap_fault(vmf);
> }
> 
> Once that returns the folio, do_shared_fault() calls do_page_mkwrite()
> and then finish_fault():
> 
> static vm_fault_t do_shared_fault(struct vm_fault *vmf)
> {
> 	struct vm_area_struct *vma = vmf->vma;
> 	vm_fault_t ret, tmp;
> 	struct folio *folio;
> 	...
> 	folio = page_folio(vmf->page);
> 	...
> 	if (vma->vm_ops->page_mkwrite) {
> 		folio_unlock(folio);
> 		tmp = do_page_mkwrite(vmf, folio);
> 		...
> 	}
> 
> 	ret |= finish_fault(vmf);
> 	...
> }
> 
> do_page_mkwrite() calls the XFS callback installed above and restores
> the original fault flags:
> 
> static vm_fault_t do_page_mkwrite(struct vm_fault *vmf, struct folio *folio)
> {
> 	vm_fault_t ret;
> 	unsigned int old_flags = vmf->flags;
> 
> 	vmf->flags = FAULT_FLAG_WRITE|FAULT_FLAG_MKWRITE;
> 	...
> 	ret = vmf->vma->vm_ops->page_mkwrite(vmf);
> 	/* Restore original flags so that caller is not surprised */
> 	vmf->flags = old_flags;
> 	...
> }
> 
> So finish_fault() still sees FAULT_FLAG_WRITE. With an empty PMD, no
> fallback requirement, and a PMD-mappable folio, it tries do_set_pmd():
> 
> vm_fault_t finish_fault(struct vm_fault *vmf)
> {
> 	...
> 	if (pmd_none(*vmf->pmd)) {
> 		if (!needs_fallback && folio_test_pmd_mappable(folio)) {
> 			ret = do_set_pmd(vmf, folio, page);
> 			if (ret != VM_FAULT_FALLBACK)
> 				return ret;
> 		}
> 		...
> 	}
> 	...
> }
> 
> After its checks pass, do_set_pmd() takes FAULT_FLAG_WRITE from vmf and
> installs a dirty+writable PMD:
> 
> vm_fault_t do_set_pmd(struct vm_fault *vmf, struct folio *folio, struct page *page)
> {
> 	struct vm_area_struct *vma = vmf->vma;
> 	bool write = vmf->flags & FAULT_FLAG_WRITE;
> 	unsigned long haddr = vmf->address & HPAGE_PMD_MASK;
> 	pmd_t entry;
> 	...
> 	entry = folio_mk_pmd(folio, vma->vm_page_prot);
> 	if (write)
> 		entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
> 	...
> 	set_pmd_at(vma->vm_mm, haddr, vmf->pmd, entry);
> 	...
> }
> 
> maybe_pmd_mkwrite() sets write permission for VM_WRITE:
> 
> pmd_t maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma)
> {
> 	if (likely(vma->vm_flags & VM_WRITE))
> 		pmd = pmd_mkwrite(pmd, vma);
> 	return pmd;
> }

Thanks, this makes sense!

> 
> >Namely, see the comment in wp_huge_pmd():
> >	/* COW or write-notify handled on pte level: split pmd. */
> >
> >if file huge pages get mapped writable, that's a bug.
> 
> That comment is about a different path. __handle_mm_fault() calls
> wp_huge_pmd() only when a write/unshare fault hits an existing PMD THP
> which is not writable:

No. That is simply a bug. There's little reason you wouldn't try do un-WP
a huge PMD if the idea would be to do PMD granularity for write notifications.
It isn't, naturally, because that results in horrible write amplification.

The fix IMO is to make it so write faults on shared mappings with page_mkwrite
never create a PMD. I don't think it makes sense to add rmap flushing hacks
for PMDs, when the common case (read + write) instantly and purposefully
breaks down to the PTE level (such that you really aren't supposed to get
the above; you'll notice that as soon as the folio gets cleaned, it will
get broken by the next write fault, period).

See the attached patch. I know willy has been working on related stuff, so
perhaps he might want to pick it up.


-- 
Pedro

[-- Attachment #2: 0001-mm-always-fallback-to-PTE-mappings-for-shared-write-.patch --]
[-- Type: text/x-patch, Size: 970 bytes --]

From b2f3fee7f1cc285e6a1fa79e77c58a88a51197b0 Mon Sep 17 00:00:00 2001
From: Pedro Falcato <pfalcato@suse.de>
Date: Thu, 20 Aug 2026 13:07:20 +0100
Subject: [PATCH] mm: always fallback to PTE mappings for shared write faults

Signed-off-by: Pedro Falcato <pfalcato@suse.de>
---
 mm/memory.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index b4be57b590ce..d37f8a0f8355 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5778,6 +5778,16 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
 		 */
 		needs_fallback = !shmem_mapping(mapping) &&
 			file_end < folio_next_index(folio);
+
+		if (vma->vm_flags & VM_SHARED && vma->vm_ops->page_mkwrite &&
+		    vmf->flags & FAULT_FLAG_WRITE) {
+			/*
+			 * Filesystems that want write notification want as
+			 * much granular of a mapping as possible. Don't
+			 * install writable THPs for those.
+			 */
+			needs_fallback = true;
+		}
 	}
 
 	if (pmd_none(*vmf->pmd)) {
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-19 16:32     ` Usama Arif
@ 2026-08-20 13:05       ` Usama Arif
  0 siblings, 0 replies; 17+ messages in thread
From: Usama Arif @ 2026-08-20 13:05 UTC (permalink / raw)
  To: Lance Yang, kas
  Cc: hughd, akpm, baohua, baolin.wang, david, dev.jain, liam,
	linux-kernel, linux-mm, ljs, nico.pache, ryan.roberts, ziy,
	nphamcs, hannes, riel, shakeel.butt, kernel-team, stable



On 19/08/2026 17:32, Usama Arif wrote:
> 
> 
> On 19/08/2026 17:17, Lance Yang wrote:
>>
>> On Wed, Aug 19, 2026 at 03:31:40PM +0100, Kiryl Shutsemau wrote:
>>> On Wed, Aug 19, 2026 at 03:12:22AM -0700, Usama Arif wrote:
>> [...]
>>>> ---
>>>>  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);
>>>
>>> Unrelated to your patch, but noticed while looking at it: we drop the rmap
>>> here under the pmd lock, while the TLB flush is deferred to
>>> tlb_finish_mmu(). The pte path handles this with
>>> tlb_delay_rmap()/force_flush (5df397dec7c4), but there's no pmd equivalent:
>>> tlb_flush_rmap_batch() only knows folio_remove_rmap_ptes(), and
>>> zap_huge_pmd() uses tlb_remove_page_size(), which takes no delay_rmap.
>>
>> Well spotted!
>>
>>> Doesn't matter for shmem, but xfs & friends do get PMD-order folios, and
>>
>> Right. pageout() cannot pass its refcount check while PMD mapping still
>> holds an extra folio ref, and mmu_gather drops that ref only after TLB
>> flush.
>>
>>> do_set_pmd() makes the pmd dirty+writable once page_mkwrite() has run. So
>>> folio_mkclean() can clean the folio while another CPU still stores through a
>>> stale TLB entry -- silently lost write, no PG_dirty left behind.
>>
>> Yep. Writeback can run folio_mkclean() while that ref is still held,
>> though, and with rmap already gone it misses the PMD ...
>>
>>> I think we need to fix this too.
>>
>> +1
>>
>>> Wanna give it a try?
>>
>> zap_huge_pmd() only handles one PMD under PTL anyway ... how about just
>> flushing before folio_remove_rmap_pmd()?
>>
>> ---8<---
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index afbb5974bd22..6fb34924ef66 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -2531,6 +2531,14 @@ bool zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>>  	is_present = pmd_present(orig_pmd);
>>  	folio = normal_or_softleaf_folio_pmd(vma, addr, orig_pmd, is_present);
>>  	has_deposit = has_deposited_pgtable(vma, orig_pmd, folio);
>> +	/*
>> +	 * folio_mkclean() relies on the rmap to find writable mappings.
>> +	 * Flush stale TLB entries before removing it below.
>> +	 */
>> +	if (folio && is_present && !folio_test_anon(folio) &&
>> +	    pmd_dirty(orig_pmd))
>> +		tlb_flush_mmu_tlbonly(tlb);
>> +
>>  	if (folio)
>>  		zap_huge_pmd_folio(mm, vma, orig_pmd, folio, is_present);
>>  	if (has_deposit)
>> ---
> 

Hey Lance,

Just wanted to check, do you want to takeover fixing this? I got involved in
a couple of others things and might not be able to dedicate the time it deserves
in the next few days.

I think the main concern above is that tlb flush was being done in cases that was not
needed. The other thing is we dont batch flushing with either your or my change,
which is an important optimization to keep.

Thanks!
Usama

> 
> I am currently at below to reduce tlb flushes, but still WIP
> 
> 
> diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
> index bdcc2778ac64f..60bdd6287b5a9 100644
> --- a/include/asm-generic/tlb.h
> +++ b/include/asm-generic/tlb.h
> @@ -301,6 +301,12 @@ bool __tlb_remove_folio_pages(struct mmu_gather *tlb, struct page *page,
>   * function, except we define it before the 'struct mmu_gather'.
>   */
>  #define tlb_delay_rmap(tlb) (((tlb)->delayed_rmap = 1), true)
> +/*
> + * Like tlb_delay_rmap() but without the side effect, for callers that must
> + * flush rather than delay: can another CPU still reach this mapping through a
> + * stale TLB entry once its rmap entry is gone? Not during fullmm teardown.
> + */
> +#define tlb_rmap_needs_flush(tlb) (!(tlb)->fullmm)
>  extern void tlb_flush_rmaps(struct mmu_gather *tlb, struct vm_area_struct *vma);
>  #endif
> 
> @@ -315,6 +321,7 @@ extern void tlb_flush_rmaps(struct mmu_gather *tlb, struct vm_area_struct *vma);
>   */
>  #ifndef tlb_delay_rmap
>  #define tlb_delay_rmap(tlb) (false)
> +#define tlb_rmap_needs_flush(tlb) (false)
>  static inline void tlb_flush_rmaps(struct mmu_gather *tlb, struct vm_area_struct *vma) { }
>  #endif
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index afbb5974bd225..76d8d5cf92ee0 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2493,6 +2493,33 @@ static bool has_deposited_pgtable(struct vm_area_struct *vma, pmd_t pmdval,
>         return folio && folio_test_anon(folio);
>  }
> 
> 
> +static bool pmd_zap_needs_tlb_flush(struct mmu_gather *tlb, pmd_t pmdval,
> +                                   struct folio *folio, bool is_present)
> +{
> +       struct address_space *mapping;
> +
> +       if (!is_present || !pmd_dirty(pmdval) || folio_test_anon(folio))
> +               return false;
> +       if (!tlb_rmap_needs_flush(tlb))
> +               return false;
> +
> +       mapping = folio_mapping(folio);
> +       return mapping && mapping_can_writeback(mapping);
> +}
> +
>  /**
>   * zap_huge_pmd - Zap a huge THP which is of PMD size.
>   * @tlb: The MMU gather TLB state associated with the operation.
> @@ -2531,8 +2558,15 @@ bool zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>         is_present = pmd_present(orig_pmd);
>         folio = normal_or_softleaf_folio_pmd(vma, addr, orig_pmd, is_present);
>         has_deposit = has_deposited_pgtable(vma, orig_pmd, folio);
> -       if (folio)
> +       if (folio) {
> +               /* Flush before zap_huge_pmd_folio() drops the rmap entry. */
> +               if (pmd_zap_needs_tlb_flush(tlb, orig_pmd, folio, is_present)) {
> +                       tlb_flush_mmu_tlbonly(tlb);
> +                       /* Re-arm: tlb_remove_page_size() needs tlb->end set. */
> +                       tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> +               }
>                 zap_huge_pmd_folio(mm, vma, orig_pmd, folio, is_present);
> +       }
>         if (has_deposit)
>                 zap_deposited_table(mm, pmd);
> 
>>
>> Cheers, Lance
>>
>>>
>>>>  		if (is_present && pmd_young(pmdval) &&
>>>>  		    likely(vma_has_recency(vma)))
>>>>  			folio_mark_accessed(folio);
>>>> -- 
>>>> 2.53.0-Meta
>>>>
>>>
>>> -- 
>>>  Kiryl Shutsemau / Kirill A. Shutemov
>>>
> 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-20 12:12       ` Pedro Falcato
@ 2026-08-20 13:20         ` Kiryl Shutsemau
  2026-08-20 14:10           ` Pedro Falcato
  0 siblings, 1 reply; 17+ messages in thread
From: Kiryl Shutsemau @ 2026-08-20 13:20 UTC (permalink / raw)
  To: Pedro Falcato
  Cc: Lance Yang, usama.arif, hughd, akpm, baohua, baolin.wang, david,
	dev.jain, liam, linux-kernel, linux-mm, ljs, nico.pache,
	ryan.roberts, ziy, nphamcs, hannes, riel, shakeel.butt,
	kernel-team, stable, willy

On Thu, Aug 20, 2026 at 01:12:18PM +0100, Pedro Falcato wrote:
> +CC willy
> 
> On Thu, Aug 20, 2026 at 02:13:37PM +0800, Lance Yang wrote:
> > 
> > On Wed, Aug 19, 2026 at 05:35:41PM +0100, Pedro Falcato wrote:
> > >On Wed, Aug 19, 2026 at 03:31:40PM +0100, Kiryl Shutsemau wrote:
> > >> On Wed, Aug 19, 2026 at 03:12:22AM -0700, 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.
> > >> 
> > >> +Hugh.
> > >> 
> > >> Oopsie.
> > >> 
> > >> I'm confused why it took a decade to discover the bug...
> > >> Maybe read ahead of write for shmem is too rare, I donno.
> > >> 
> > >> > 
> > >> > Fixes: 800d8c63b2e9 ("shmem: add huge pages support")
> > >> 
> > >> This would be more precise: b5072380eb61 ("thp: support file pages in zap_huge_pmd()")
> > >> 
> > >> Reviewed-by: Kiryl Shutsemau <kas@kernel.org>
> > >> 
> > >> > Cc: <stable@vger.kernel.org>
> > >> > Signed-off-by: Usama Arif <usama.arif@linux.dev>
> > >> > ---
> > >> >  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);
> > >> 
> > >> Unrelated to your patch, but noticed while looking at it: we drop the rmap
> > >> here under the pmd lock, while the TLB flush is deferred to
> > >> tlb_finish_mmu(). The pte path handles this with
> > >> tlb_delay_rmap()/force_flush (5df397dec7c4), but there's no pmd equivalent:
> > >> tlb_flush_rmap_batch() only knows folio_remove_rmap_ptes(), and
> > >> zap_huge_pmd() uses tlb_remove_page_size(), which takes no delay_rmap.
> > >> 
> > >> Doesn't matter for shmem, but xfs & friends do get PMD-order folios, and
> > >> do_set_pmd() makes the pmd dirty+writable once page_mkwrite() has run. So
> > >> folio_mkclean() can clean the folio while another CPU still stores through a
> > >> stale TLB entry -- silently lost write, no PG_dirty left behind.
> > >
> > >Where do you see page_mkwrite being called in the same path as do_set_pmd()?
> > >Per my understanding of the code, this Should Not Happen, and it really Should
> > >Not Happen for many, many reasons (write amplification being the main one).
> > 
> > Hmm.. that happens on an initial shared write fault. For non-DAX XFS, the
> > path starts with an empty PMD.
> > 
> > TL;DR
> > 
> > With an empty PMD and PMD-order THP allowed, __handle_mm_fault() first
> > tries create_huge_pmd(). VM_FAULT_FALLBACK sends the fault to
> > handle_pte_fault():
> > 
> > static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma,
> > 		unsigned long address, unsigned int flags)
> > {
> > 	...
> > 	if (pmd_none(*vmf.pmd) &&
> > 	    thp_vma_allowable_order(vma, vm_flags, TVA_PAGEFAULT, PMD_ORDER)) {
> > 		ret = create_huge_pmd(&vmf);
> > 		if (ret & VM_FAULT_FALLBACK)
> > 			goto fallback;
> > 		else
> > 			return ret;
> > 	}
> > 	...
> > fallback:
> > 	return handle_pte_fault(&vmf);
> > }
> > 
> > create_huge_pmd() dispatches to the filesystem's huge_fault callback:
> > 
> > static inline vm_fault_t create_huge_pmd(struct vm_fault *vmf)
> > {
> > 	struct vm_area_struct *vma = vmf->vma;
> > 	...
> > 	if (vma->vm_ops->huge_fault)
> > 		return vma->vm_ops->huge_fault(vmf, PMD_ORDER);
> > 	return VM_FAULT_FALLBACK;
> > }
> > 
> > For non-DAX XFS, that callback returns VM_FAULT_FALLBACK:
> > 
> > static vm_fault_t
> > xfs_filemap_huge_fault(
> > 	struct vm_fault		*vmf,
> > 	unsigned int		order)
> > {
> > 	if (!IS_DAX(file_inode(vmf->vma->vm_file)))
> > 		return VM_FAULT_FALLBACK;
> > 	...
> > }
> > 
> > XFS installs the huge-fault, regular-fault, and page_mkwrite callbacks
> > in the same vm_ops:
> > 
> > static const struct vm_operations_struct xfs_file_vm_ops = {
> > 	.fault		= xfs_filemap_fault,
> > 	.huge_fault	= xfs_filemap_huge_fault,
> > 	...
> > 	.page_mkwrite	= xfs_filemap_page_mkwrite,
> > 	...
> > };
> > 
> > On the fallback path, handle_pte_fault() leaves an empty PMD without a
> > PTE and calls do_pte_missing():
> > 
> > static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
> > {
> > 	...
> > 	if (unlikely(pmd_none(*vmf->pmd))) {
> > 		/*
> > 		 * Leave __pte_alloc() until later: because vm_ops->fault may
> > 		 * want to allocate huge page, and if we expose page table
> > 		 * for an instant, it will be difficult to retract from
> > 		 * concurrent faults and from rmap lookups.
> > 		 */
> > 		vmf->pte = NULL;
> > 		vmf->flags &= ~FAULT_FLAG_ORIG_PTE_VALID;
> > 	...
> > 	}
> > 
> > 	if (!vmf->pte)
> > 		return do_pte_missing(vmf);
> > 	...
> > }
> > 
> > For a file VMA, do_pte_missing() calls do_fault():
> > 
> > static vm_fault_t do_pte_missing(struct vm_fault *vmf)
> > {
> > 	if (vma_is_anonymous(vmf->vma))
> > 		return do_anonymous_page(vmf);
> > 	else
> > 		return do_fault(vmf);
> > }
> > 
> > do_fault() sends FAULT_FLAG_WRITE + VM_SHARED to do_shared_fault():
> > 
> > static vm_fault_t do_fault(struct vm_fault *vmf)
> > {
> > 	struct vm_area_struct *vma = vmf->vma;
> > 	...
> > 	if (!vma->vm_ops->fault) {
> > 		...
> > 	} else if (!(vmf->flags & FAULT_FLAG_WRITE))
> > 		ret = do_read_fault(vmf);
> > 	else if (!(vma->vm_flags & VM_SHARED))
> > 		ret = do_cow_fault(vmf);
> > 	else
> > 		ret = do_shared_fault(vmf);
> > 	...
> > }
> > 
> > do_shared_fault() first calls __do_fault():
> > 
> > static vm_fault_t do_shared_fault(struct vm_fault *vmf)
> > {
> > 	struct vm_area_struct *vma = vmf->vma;
> > 	vm_fault_t ret, tmp;
> > 	struct folio *folio;
> > 	...
> > 	ret = __do_fault(vmf);
> > 	...
> > }
> > 
> > __do_fault() invokes the regular fault callback:
> > 
> > static vm_fault_t __do_fault(struct vm_fault *vmf)
> > {
> > 	struct vm_area_struct *vma = vmf->vma;
> > 	struct folio *folio;
> > 	vm_fault_t ret;
> > 	...
> > 	ret = vma->vm_ops->fault(vmf);
> > 	...
> > 	return ret;
> > }
> > 
> > For non-DAX XFS, xfs_filemap_fault() reaches filemap_fault():
> > 
> > static vm_fault_t
> > xfs_filemap_fault(
> > 	struct vm_fault		*vmf)
> > {
> > 	struct inode		*inode = file_inode(vmf->vma->vm_file);
> > 	...
> > 	return filemap_fault(vmf);
> > }
> > 
> > Once that returns the folio, do_shared_fault() calls do_page_mkwrite()
> > and then finish_fault():
> > 
> > static vm_fault_t do_shared_fault(struct vm_fault *vmf)
> > {
> > 	struct vm_area_struct *vma = vmf->vma;
> > 	vm_fault_t ret, tmp;
> > 	struct folio *folio;
> > 	...
> > 	folio = page_folio(vmf->page);
> > 	...
> > 	if (vma->vm_ops->page_mkwrite) {
> > 		folio_unlock(folio);
> > 		tmp = do_page_mkwrite(vmf, folio);
> > 		...
> > 	}
> > 
> > 	ret |= finish_fault(vmf);
> > 	...
> > }
> > 
> > do_page_mkwrite() calls the XFS callback installed above and restores
> > the original fault flags:
> > 
> > static vm_fault_t do_page_mkwrite(struct vm_fault *vmf, struct folio *folio)
> > {
> > 	vm_fault_t ret;
> > 	unsigned int old_flags = vmf->flags;
> > 
> > 	vmf->flags = FAULT_FLAG_WRITE|FAULT_FLAG_MKWRITE;
> > 	...
> > 	ret = vmf->vma->vm_ops->page_mkwrite(vmf);
> > 	/* Restore original flags so that caller is not surprised */
> > 	vmf->flags = old_flags;
> > 	...
> > }
> > 
> > So finish_fault() still sees FAULT_FLAG_WRITE. With an empty PMD, no
> > fallback requirement, and a PMD-mappable folio, it tries do_set_pmd():
> > 
> > vm_fault_t finish_fault(struct vm_fault *vmf)
> > {
> > 	...
> > 	if (pmd_none(*vmf->pmd)) {
> > 		if (!needs_fallback && folio_test_pmd_mappable(folio)) {
> > 			ret = do_set_pmd(vmf, folio, page);
> > 			if (ret != VM_FAULT_FALLBACK)
> > 				return ret;
> > 		}
> > 		...
> > 	}
> > 	...
> > }
> > 
> > After its checks pass, do_set_pmd() takes FAULT_FLAG_WRITE from vmf and
> > installs a dirty+writable PMD:
> > 
> > vm_fault_t do_set_pmd(struct vm_fault *vmf, struct folio *folio, struct page *page)
> > {
> > 	struct vm_area_struct *vma = vmf->vma;
> > 	bool write = vmf->flags & FAULT_FLAG_WRITE;
> > 	unsigned long haddr = vmf->address & HPAGE_PMD_MASK;
> > 	pmd_t entry;
> > 	...
> > 	entry = folio_mk_pmd(folio, vma->vm_page_prot);
> > 	if (write)
> > 		entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
> > 	...
> > 	set_pmd_at(vma->vm_mm, haddr, vmf->pmd, entry);
> > 	...
> > }
> > 
> > maybe_pmd_mkwrite() sets write permission for VM_WRITE:
> > 
> > pmd_t maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma)
> > {
> > 	if (likely(vma->vm_flags & VM_WRITE))
> > 		pmd = pmd_mkwrite(pmd, vma);
> > 	return pmd;
> > }
> 
> Thanks, this makes sense!
> 
> > 
> > >Namely, see the comment in wp_huge_pmd():
> > >	/* COW or write-notify handled on pte level: split pmd. */
> > >
> > >if file huge pages get mapped writable, that's a bug.
> > 
> > That comment is about a different path. __handle_mm_fault() calls
> > wp_huge_pmd() only when a write/unshare fault hits an existing PMD THP
> > which is not writable:
> 
> No. That is simply a bug. There's little reason you wouldn't try do un-WP
> a huge PMD if the idea would be to do PMD granularity for write notifications.
> It isn't, naturally, because that results in horrible write amplification.

Write notification is already folio-granular. Installing PTE instead of
PMD changes nothing.

> The fix IMO is to make it so write faults on shared mappings with page_mkwrite
> never create a PMD.

Why? Write batching from large folios is a win.

> I don't think it makes sense to add rmap flushing hacks
> for PMDs, when the common case (read + write) instantly and purposefully
> breaks down to the PTE level (such that you really aren't supposed to get
> the above; you'll notice that as soon as the folio gets cleaned, it will
> get broken by the next write fault, period).

If you consider delayed rmap a hack (I don't), it has to fixed on PTE
level too.

> See the attached patch. I know willy has been working on related stuff, so
> perhaps he might want to pick it up.

As I said the patch doesn't do what you expect it to do. PG_dirty is on
folio and we writeback folios, not PTEs.

Also, needs_fallback is not just "no PMD": finish_fault() then forces
nr_pages = 1, so it is 512 faults and 512 ->page_mkwrite calls per 2M folio
instead of one.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap
  2026-08-20 13:20         ` Kiryl Shutsemau
@ 2026-08-20 14:10           ` Pedro Falcato
  0 siblings, 0 replies; 17+ messages in thread
From: Pedro Falcato @ 2026-08-20 14:10 UTC (permalink / raw)
  To: Kiryl Shutsemau
  Cc: Lance Yang, usama.arif, hughd, akpm, baohua, baolin.wang, david,
	dev.jain, liam, linux-kernel, linux-mm, ljs, nico.pache,
	ryan.roberts, ziy, nphamcs, hannes, riel, shakeel.butt,
	kernel-team, stable, willy

On Thu, Aug 20, 2026 at 02:20:26PM +0100, Kiryl Shutsemau wrote:
> On Thu, Aug 20, 2026 at 01:12:18PM +0100, Pedro Falcato wrote:
> > +CC willy
> > 
> > On Thu, Aug 20, 2026 at 02:13:37PM +0800, Lance Yang wrote:
> > > 
> > > On Wed, Aug 19, 2026 at 05:35:41PM +0100, Pedro Falcato wrote:
> > > >On Wed, Aug 19, 2026 at 03:31:40PM +0100, Kiryl Shutsemau wrote:
> > > >> On Wed, Aug 19, 2026 at 03:12:22AM -0700, 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.
> > > >> 
> > > >> +Hugh.
> > > >> 
> > > >> Oopsie.
> > > >> 
> > > >> I'm confused why it took a decade to discover the bug...
> > > >> Maybe read ahead of write for shmem is too rare, I donno.
> > > >> 
> > > >> > 
> > > >> > Fixes: 800d8c63b2e9 ("shmem: add huge pages support")
> > > >> 
> > > >> This would be more precise: b5072380eb61 ("thp: support file pages in zap_huge_pmd()")
> > > >> 
> > > >> Reviewed-by: Kiryl Shutsemau <kas@kernel.org>
> > > >> 
> > > >> > Cc: <stable@vger.kernel.org>
> > > >> > Signed-off-by: Usama Arif <usama.arif@linux.dev>
> > > >> > ---
> > > >> >  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);
> > > >> 
> > > >> Unrelated to your patch, but noticed while looking at it: we drop the rmap
> > > >> here under the pmd lock, while the TLB flush is deferred to
> > > >> tlb_finish_mmu(). The pte path handles this with
> > > >> tlb_delay_rmap()/force_flush (5df397dec7c4), but there's no pmd equivalent:
> > > >> tlb_flush_rmap_batch() only knows folio_remove_rmap_ptes(), and
> > > >> zap_huge_pmd() uses tlb_remove_page_size(), which takes no delay_rmap.
> > > >> 
> > > >> Doesn't matter for shmem, but xfs & friends do get PMD-order folios, and
> > > >> do_set_pmd() makes the pmd dirty+writable once page_mkwrite() has run. So
> > > >> folio_mkclean() can clean the folio while another CPU still stores through a
> > > >> stale TLB entry -- silently lost write, no PG_dirty left behind.
> > > >
> > > >Where do you see page_mkwrite being called in the same path as do_set_pmd()?
> > > >Per my understanding of the code, this Should Not Happen, and it really Should
> > > >Not Happen for many, many reasons (write amplification being the main one).
> > > 
> > > Hmm.. that happens on an initial shared write fault. For non-DAX XFS, the
> > > path starts with an empty PMD.
> > > 
> > > TL;DR
> > > 
> > > With an empty PMD and PMD-order THP allowed, __handle_mm_fault() first
> > > tries create_huge_pmd(). VM_FAULT_FALLBACK sends the fault to
> > > handle_pte_fault():
> > > 
> > > static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma,
> > > 		unsigned long address, unsigned int flags)
> > > {
> > > 	...
> > > 	if (pmd_none(*vmf.pmd) &&
> > > 	    thp_vma_allowable_order(vma, vm_flags, TVA_PAGEFAULT, PMD_ORDER)) {
> > > 		ret = create_huge_pmd(&vmf);
> > > 		if (ret & VM_FAULT_FALLBACK)
> > > 			goto fallback;
> > > 		else
> > > 			return ret;
> > > 	}
> > > 	...
> > > fallback:
> > > 	return handle_pte_fault(&vmf);
> > > }
> > > 
> > > create_huge_pmd() dispatches to the filesystem's huge_fault callback:
> > > 
> > > static inline vm_fault_t create_huge_pmd(struct vm_fault *vmf)
> > > {
> > > 	struct vm_area_struct *vma = vmf->vma;
> > > 	...
> > > 	if (vma->vm_ops->huge_fault)
> > > 		return vma->vm_ops->huge_fault(vmf, PMD_ORDER);
> > > 	return VM_FAULT_FALLBACK;
> > > }
> > > 
> > > For non-DAX XFS, that callback returns VM_FAULT_FALLBACK:
> > > 
> > > static vm_fault_t
> > > xfs_filemap_huge_fault(
> > > 	struct vm_fault		*vmf,
> > > 	unsigned int		order)
> > > {
> > > 	if (!IS_DAX(file_inode(vmf->vma->vm_file)))
> > > 		return VM_FAULT_FALLBACK;
> > > 	...
> > > }
> > > 
> > > XFS installs the huge-fault, regular-fault, and page_mkwrite callbacks
> > > in the same vm_ops:
> > > 
> > > static const struct vm_operations_struct xfs_file_vm_ops = {
> > > 	.fault		= xfs_filemap_fault,
> > > 	.huge_fault	= xfs_filemap_huge_fault,
> > > 	...
> > > 	.page_mkwrite	= xfs_filemap_page_mkwrite,
> > > 	...
> > > };
> > > 
> > > On the fallback path, handle_pte_fault() leaves an empty PMD without a
> > > PTE and calls do_pte_missing():
> > > 
> > > static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
> > > {
> > > 	...
> > > 	if (unlikely(pmd_none(*vmf->pmd))) {
> > > 		/*
> > > 		 * Leave __pte_alloc() until later: because vm_ops->fault may
> > > 		 * want to allocate huge page, and if we expose page table
> > > 		 * for an instant, it will be difficult to retract from
> > > 		 * concurrent faults and from rmap lookups.
> > > 		 */
> > > 		vmf->pte = NULL;
> > > 		vmf->flags &= ~FAULT_FLAG_ORIG_PTE_VALID;
> > > 	...
> > > 	}
> > > 
> > > 	if (!vmf->pte)
> > > 		return do_pte_missing(vmf);
> > > 	...
> > > }
> > > 
> > > For a file VMA, do_pte_missing() calls do_fault():
> > > 
> > > static vm_fault_t do_pte_missing(struct vm_fault *vmf)
> > > {
> > > 	if (vma_is_anonymous(vmf->vma))
> > > 		return do_anonymous_page(vmf);
> > > 	else
> > > 		return do_fault(vmf);
> > > }
> > > 
> > > do_fault() sends FAULT_FLAG_WRITE + VM_SHARED to do_shared_fault():
> > > 
> > > static vm_fault_t do_fault(struct vm_fault *vmf)
> > > {
> > > 	struct vm_area_struct *vma = vmf->vma;
> > > 	...
> > > 	if (!vma->vm_ops->fault) {
> > > 		...
> > > 	} else if (!(vmf->flags & FAULT_FLAG_WRITE))
> > > 		ret = do_read_fault(vmf);
> > > 	else if (!(vma->vm_flags & VM_SHARED))
> > > 		ret = do_cow_fault(vmf);
> > > 	else
> > > 		ret = do_shared_fault(vmf);
> > > 	...
> > > }
> > > 
> > > do_shared_fault() first calls __do_fault():
> > > 
> > > static vm_fault_t do_shared_fault(struct vm_fault *vmf)
> > > {
> > > 	struct vm_area_struct *vma = vmf->vma;
> > > 	vm_fault_t ret, tmp;
> > > 	struct folio *folio;
> > > 	...
> > > 	ret = __do_fault(vmf);
> > > 	...
> > > }
> > > 
> > > __do_fault() invokes the regular fault callback:
> > > 
> > > static vm_fault_t __do_fault(struct vm_fault *vmf)
> > > {
> > > 	struct vm_area_struct *vma = vmf->vma;
> > > 	struct folio *folio;
> > > 	vm_fault_t ret;
> > > 	...
> > > 	ret = vma->vm_ops->fault(vmf);
> > > 	...
> > > 	return ret;
> > > }
> > > 
> > > For non-DAX XFS, xfs_filemap_fault() reaches filemap_fault():
> > > 
> > > static vm_fault_t
> > > xfs_filemap_fault(
> > > 	struct vm_fault		*vmf)
> > > {
> > > 	struct inode		*inode = file_inode(vmf->vma->vm_file);
> > > 	...
> > > 	return filemap_fault(vmf);
> > > }
> > > 
> > > Once that returns the folio, do_shared_fault() calls do_page_mkwrite()
> > > and then finish_fault():
> > > 
> > > static vm_fault_t do_shared_fault(struct vm_fault *vmf)
> > > {
> > > 	struct vm_area_struct *vma = vmf->vma;
> > > 	vm_fault_t ret, tmp;
> > > 	struct folio *folio;
> > > 	...
> > > 	folio = page_folio(vmf->page);
> > > 	...
> > > 	if (vma->vm_ops->page_mkwrite) {
> > > 		folio_unlock(folio);
> > > 		tmp = do_page_mkwrite(vmf, folio);
> > > 		...
> > > 	}
> > > 
> > > 	ret |= finish_fault(vmf);
> > > 	...
> > > }
> > > 
> > > do_page_mkwrite() calls the XFS callback installed above and restores
> > > the original fault flags:
> > > 
> > > static vm_fault_t do_page_mkwrite(struct vm_fault *vmf, struct folio *folio)
> > > {
> > > 	vm_fault_t ret;
> > > 	unsigned int old_flags = vmf->flags;
> > > 
> > > 	vmf->flags = FAULT_FLAG_WRITE|FAULT_FLAG_MKWRITE;
> > > 	...
> > > 	ret = vmf->vma->vm_ops->page_mkwrite(vmf);
> > > 	/* Restore original flags so that caller is not surprised */
> > > 	vmf->flags = old_flags;
> > > 	...
> > > }
> > > 
> > > So finish_fault() still sees FAULT_FLAG_WRITE. With an empty PMD, no
> > > fallback requirement, and a PMD-mappable folio, it tries do_set_pmd():
> > > 
> > > vm_fault_t finish_fault(struct vm_fault *vmf)
> > > {
> > > 	...
> > > 	if (pmd_none(*vmf->pmd)) {
> > > 		if (!needs_fallback && folio_test_pmd_mappable(folio)) {
> > > 			ret = do_set_pmd(vmf, folio, page);
> > > 			if (ret != VM_FAULT_FALLBACK)
> > > 				return ret;
> > > 		}
> > > 		...
> > > 	}
> > > 	...
> > > }
> > > 
> > > After its checks pass, do_set_pmd() takes FAULT_FLAG_WRITE from vmf and
> > > installs a dirty+writable PMD:
> > > 
> > > vm_fault_t do_set_pmd(struct vm_fault *vmf, struct folio *folio, struct page *page)
> > > {
> > > 	struct vm_area_struct *vma = vmf->vma;
> > > 	bool write = vmf->flags & FAULT_FLAG_WRITE;
> > > 	unsigned long haddr = vmf->address & HPAGE_PMD_MASK;
> > > 	pmd_t entry;
> > > 	...
> > > 	entry = folio_mk_pmd(folio, vma->vm_page_prot);
> > > 	if (write)
> > > 		entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
> > > 	...
> > > 	set_pmd_at(vma->vm_mm, haddr, vmf->pmd, entry);
> > > 	...
> > > }
> > > 
> > > maybe_pmd_mkwrite() sets write permission for VM_WRITE:
> > > 
> > > pmd_t maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma)
> > > {
> > > 	if (likely(vma->vm_flags & VM_WRITE))
> > > 		pmd = pmd_mkwrite(pmd, vma);
> > > 	return pmd;
> > > }
> > 
> > Thanks, this makes sense!
> > 
> > > 
> > > >Namely, see the comment in wp_huge_pmd():
> > > >	/* COW or write-notify handled on pte level: split pmd. */
> > > >
> > > >if file huge pages get mapped writable, that's a bug.
> > > 
> > > That comment is about a different path. __handle_mm_fault() calls
> > > wp_huge_pmd() only when a write/unshare fault hits an existing PMD THP
> > > which is not writable:
> > 
> > No. That is simply a bug. There's little reason you wouldn't try do un-WP
> > a huge PMD if the idea would be to do PMD granularity for write notifications.
> > It isn't, naturally, because that results in horrible write amplification.
> 
> Write notification is already folio-granular. Installing PTE instead of
> PMD changes nothing.

It does! See below.

> 
> > The fix IMO is to make it so write faults on shared mappings with page_mkwrite
> > never create a PMD.
> 
> Why? Write batching from large folios is a win.

For some context: we were discussing (off-list) the recent report that
systemd-journald had horrible write amplification, worsed quite a bit
by large folios. At the moment, there is quite a lot of write amplification,
but _only_ on mmap writes (if you look at the write(2) paths, you'll see
write_begin and write_end which tactically only dirty what you actually wrote
to, block-wise in the BHs or iomap IFS). This doesn't need to be true.

What we conjured up was the following: since WP faults always happen on PTEs,
you can simply dirty the block(s) corresponding to that particular PTE (which
naturally points to a page inside the folio, whether it is large or not, doesn't
matter). That way, you can avoid fully dirtying the folio's blocks. This is
something that ATM isn't done by any filesystem, but it really should.

Obviously this whole idea is thwarted if we keep writable shared file folios
PMD-mapped - you can't get meaningful write notifications apart from "someone
wrote to this folio", which isn't particularly useful once folios get 2M+ large.
So PMD-mapping on a shared-write fault is antithetical to getting useful,
granular write notifications to filesystems.

(I admit, I'm not sure if anyone cares about hugepages _and_ writing; I think
not, and if they do we could perhaps consider merging the PTEs back into a PMD
once the folio gets cleared, or if the whole folio is dirty, but I suspect the
details would be horribly hairy. Maybe mongodb cares.)

> 
> > I don't think it makes sense to add rmap flushing hacks
> > for PMDs, when the common case (read + write) instantly and purposefully
> > breaks down to the PTE level (such that you really aren't supposed to get
> > the above; you'll notice that as soon as the folio gets cleaned, it will
> > get broken by the next write fault, period).
> 
> If you consider delayed rmap a hack (I don't), it has to fixed on PTE
> level too.
> 
> > See the attached patch. I know willy has been working on related stuff, so
> > perhaps he might want to pick it up.
> 
> As I said the patch doesn't do what you expect it to do. PG_dirty is on
> folio and we writeback folios, not PTEs.
> 
> Also, needs_fallback is not just "no PMD": finish_fault() then forces
> nr_pages = 1, so it is 512 faults and 512 ->page_mkwrite calls per 2M folio
> instead of one.

You're 100% right :) That teaches me not to write patches on PTO :) So the
patch would need to be modified, but I think the principle still stands.

> 
> -- 
>   Kiryl Shutsemau / Kirill A. Shutemov

-- 
Pedro


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-08-20 14:10 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-19 10:12 [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap Usama Arif
2026-08-19 14:13 ` David Hildenbrand (Arm)
2026-08-19 14:13   ` David Hildenbrand (Arm)
2026-08-19 14:31 ` Kiryl Shutsemau
2026-08-19 16:17   ` Lance Yang
2026-08-19 16:32     ` Usama Arif
2026-08-20 13:05       ` Usama Arif
2026-08-19 16:35   ` Pedro Falcato
2026-08-20  6:13     ` Lance Yang
2026-08-20 12:12       ` Pedro Falcato
2026-08-20 13:20         ` Kiryl Shutsemau
2026-08-20 14:10           ` Pedro Falcato
2026-08-19 20:33   ` Hugh Dickins
2026-08-19 15:10 ` Lance Yang
2026-08-19 15:31 ` Zi Yan
2026-08-19 16:09 ` Lorenzo Stoakes (ARM)
2026-08-20  2:06 ` Baolin Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox