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

[Lance Yang]

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@xxxxxxxxxx>
>> 
>> > Cc: <stable@xxxxxxxxxxxxxxx>
>> > Signed-off-by: Usama Arif <usama.arif@xxxxxxxxx>
>> > ---
>> >  mm/huge_memory.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> > 
>> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> > index ced400f72d43a..afbb5974bd225 100644
>> > --- a/mm/huge_memory.c
>> > +++ b/mm/huge_memory.c
>> > @@ -2449,6 +2449,8 @@ static void zap_huge_pmd_folio(struct mm_struct *mm, struct vm_area_struct *vma,
>> >  		add_mm_counter(mm, mm_counter_file(folio),
>> >  			       -HPAGE_PMD_NR);
>> >  
>> > +		if (is_present && pmd_dirty(pmdval))
>> > +			folio_mark_dirty(folio);
>> 
>> 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
>