From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 28E83C5DF82 for ; Thu, 20 Aug 2026 06:13:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 214196B0098; Thu, 20 Aug 2026 02:13:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1EC056B009B; Thu, 20 Aug 2026 02:13:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0B4356B009D; Thu, 20 Aug 2026 02:13:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id CFD366B0098 for ; Thu, 20 Aug 2026 02:13:56 -0400 (EDT) Received: from smtpin03.hostedemail.com (lb01a-stub [10.200.18.249]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 53FCCC048E for ; Thu, 20 Aug 2026 06:13:56 +0000 (UTC) X-FDA: 85120632072.03.74336D9 Received: from mta0.migadu.com (out-246.mta0.migadu.com [91.218.175.246]) by imf18.hostedemail.com (Postfix) with ESMTP id 493561C0009 for ; Thu, 20 Aug 2026 06:13:54 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=gWsdERKG; spf=pass (imf18.hostedemail.com: domain of lance.yang@linux.dev designates 91.218.175.246 as permitted sender) smtp.mailfrom=lance.yang@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1787206434; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=VIgiuVnVrClbLqNUEm6VnA2rtONgvwj4lJEq6s2oIGM=; b=YcMm/XuoG7l+XrxqB5MWJStSnxbtgAW0eyR2p0a0cnVcrAF5XUj0bYcwUGAwFIdCsApG42 tdC21YGCJKqPvJtX0fnqale1Wr+xt2aycbf446sC/lgLH/dvnDRSj8FiPe7lDraEFqgiiZ IzNeLgq6IEk6vJBUlF0PLiM6c+7NLFY= ARC-Seal: i=1; a=rsa-sha256; d=hostedemail.com; s=arc-20220608; cv=none; t=1787206434; b=wINUcL57tXoCEvezXM3AacluVz7Dniu9VyCNT15gUrBCgLYMhd3QmxzQ7EXaPDCPIpj/7v n+FCQmvOCvMkDIwqtKqE0LFNvNZfDJeudu6XAsxtR+zrDf+iGZfg0rA7RSL+F8QCEgLufC Az7QQNSUBaLl3WBoqXvYl2FTwJmO0qM= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=gWsdERKG; spf=pass (imf18.hostedemail.com: domain of lance.yang@linux.dev designates 91.218.175.246 as permitted sender) smtp.mailfrom=lance.yang@linux.dev; dmarc=pass (policy=none) header.from=linux.dev X-Envelope-To: linux-mm@kvack.org DKIM-Signature: a=rsa-sha256; bh=OLL9Vg6Y98cx9hk3jOcUkhAPSNsd+cPxxJkp4/MTDQQ=; c=simple/simple; d=linux.dev; h=from:to:subject:date:message-id:mime-version:content-type; s=key1; t=1787206433; v=1; x=1787811233; b=gWsdERKGxhxJVoYfHa7AK5jWnL5WmZP7GsfSRU21S70/sVZkaZZFFLVQLQ25ItAHsEWHWSRs 2HbzNc+zxD8I8SnxQA9/+CIiYiNEPyAPxIryXnE2xsBxj/hDW2jUi/jd1/c7oGmV2l7Sumtix5B YNgT8QsbgCpGbCG5Cdm4g9OQ= X-Envelope-To: linux-mm@kvack.org Received: from localhost (2602:fce1:44f:115e::) by smtp.migadu.com with ESMTPS id 588eb7e31e708159; Thu, 20 Aug 2026 06:13:53 +0000 X-Mizu-Trace-ID: 588eb7e31e708159 X-Migadu-Flow: FLOW_OUT From: Lance Yang To: pfalcato@suse.de Cc: kas@kernel.org, usama.arif@linux.dev, hughd@google.com, akpm@linux-foundation.org, baohua@kernel.org, baolin.wang@linux.alibaba.com, david@kernel.org, dev.jain@arm.com, lance.yang@linux.dev, liam@infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, ljs@kernel.org, nico.pache@linux.dev, ryan.roberts@arm.com, ziy@nvidia.com, nphamcs@gmail.com, hannes@cmpxchg.org, riel@surriel.com, shakeel.butt@linux.dev, kernel-team@meta.com, stable@vger.kernel.org Subject: Re: [PATCH] mm/huge_memory: transfer the pmd dirty bit to the folio on zap Date: Thu, 20 Aug 2026 14:13:37 +0800 Message-Id: <20260820061337.24669-1-lance.yang@linux.dev> X-Mailer: git-send-email 2.39.3 (Apple Git-146) In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Stat-Signature: u4ez4ps67gu95gsq54d4i5moxkb4tx5a X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 493561C0009 X-HE-Tag: 1787206434-110877 X-HE-Meta: U2FsdGVkX1+80nDo9uRU+cK4Loc7weIj4hYc0CBQudlCAxvQgtrJ6CRAGxkxo/L/Q7K7ClVYEd+Er7aCbBCyb4yNxEbp9hUKMbvkT6VBwkXBbFQotZRQs7zqmHLUWuCjwgTepPDf4S9B1vAWuj/2m4Dyb2HSfY1b96zHJ8Mg0DTwDKNDGFz10kGz+hnPTXQPS6WfvCmL5GmuVQCWVVAw8vbcvV8r5yWxFZuFNXzJMyBjB1Hc8gFknO2HMbFjydVSoeUwfj3Po84SRzqkwFQy+LBjeAdjr0vLsyYoX6PiXgm7gQ1RDU7nt87moNIKQTD7ASSt6K4BeAZQQLQ+6jqCt6PQGqSRf4AEjbb8g5G+BdkThwpN29/GwiYXxrrj1Z6o/BgsqcK/h/m//Y4AopMFdGBcUKdfFufeqP/AzO3Z2g0bg9vS7K4DpFyRhVJEMz4YA1HopPvmKv8wv5rKq336wSTp1sYZhVgXzIfYIqWGFKd68FlXZ2OR686f9sqUcHCK2CcILLtFoceGd+akfGAuaKRS9zUJn36WkWIos/p9tQoP96hG8QG1Er4toUAv/26gCsqHGu4Yz8rE08pCCuYw4jR9j0ZEgne5h0vuem26YbdONPlc5mek8RpqkDlNbRxfbJ9jaZLETko8AaaQrP3GnxhN/FxW1FzOygyjHpEoPDyOLNcup5gmxba6H0V2XE3NQQd97K0jgihZy0yT2fIulBpn3vLnw4tpbMmOdDdowdgGTzhhJmwgfh0/Fpfawj3i3gPrbCS4keZJBDFE5cBSHhIt1M+bY1jFvUx90VHO1TfKmnxPJnsF3anX8PWxsPe84y7pp9BBC6LXUrDnswr/iquSfQhonPAPFXQLFnvkxh1UValsYpPIjMdgoWFeI0ttIoAgd/uYCa4CjBVRJVov5lcRLApLgmsax3jFPeNHjrPtWUdP0K4WWzwFM6QCVPWMenm+OXb9WZ78Vbu+3AU 8iCM69OU iKM3WKXUnowWmdoCCrXE9m6FnjmH5sFq7sSwdEXaoMZwmfQ6GaD5muFlN+U+stGu5NyJ3RMOGi4LSHus0A9Mf9MSi3YlINXsQZMouvvrhHON+Mr4wEfe+RXEt50HJWpjEcOKQevp5XlvXQiIYfzLRSRCBl4chaZoBtg06D8TO8QYnCLVXRCMFbGsCra2nx7E9RjOnTVBDTJMKX+SHB8p0p2OoTRVjKxwM/KVlaC8HGY2++Y2SUoluA2fk5+9zV9oNmGkA0d+yo3ghadDKk4cIjTfughLD93XSR1Abdd2do9KO3U6OVmh6BsEJOiuMS12IaFUBSW209+a3v/CaS1wtrMMPdhQp7TkbTUzM Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: 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 >> >> > Cc: >> > Signed-off-by: Usama Arif >> > --- >> > 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 >