* [PATCH v5] mm: memory: fix truncation removing mapped folios
@ 2026-10-27 2:19 Baolin Wang
2026-10-27 9:00 ` David Hildenbrand (Arm)
0 siblings, 1 reply; 9+ messages in thread
From: Baolin Wang @ 2026-10-27 2:19 UTC (permalink / raw)
To: akpm, david
Cc: ljs, liam, vbabka, rppt, surenb, mhocko, qi.zheng, jack,
pfalcato, ayushr, baolin.wang, linux-mm, linux-kernel
Ayush reported an issue that shmem/tmpfs folios are still mapped when
truncating them on production:
"
BUG: Bad page cache in process ... pfn:...
page dumped because: still mapped when deleted
"
I reproduced the issue with Ayush's reproducer [1] with the latest mm-unstable
branch, and I got the following crash info. From the dump message, we can
see that truncate_inode_folio() is really trying to remove mapped folios,
which is incorrect.
"
[ 189.171977] page: refcount:3 mapcount:1 mapping:00000000fe168ee1
index:0x3880 pfn:0x191647
[ 189.171995] memcg:ffff0000cc073d40
[ 189.171997] aops:shmem_aops ino:3c01 dentry name(?):"memfd:runsc-memory"
[ 189.172006] flags: 0x17fffef0002022d(locked|referenced|uptodate|lru|
workingset|swapbacked|node=0|zone=2|lastcpupid=0x3ffff)
.......
[ 189.172017] page dumped because: VM_BUG_ON_FOLIO(folio_mapped(folio))
[ 189.172026] ------------[ cut here ]------------
[ 189.172027] kernel BUG at mm/filemap.c:155!
[ 189.172057] Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
.......
[ 189.177721] CPU: 3 UID: 0 PID: 5521 Comm: repro Kdump: loaded Tainted:
7.3.0-rc4+ #277 PREEMPT(lazy)
.......
[ 189.183986] Call trace:
[ 189.184124] filemap_unaccount_folio+0xf0/0x1e8 (P)
[ 189.184391] __filemap_remove_folio+0x34/0x160
[ 189.184633] filemap_remove_folio+0x4c/0xb0
[ 189.184859] truncate_inode_folio+0x34/0x58
[ 189.185087] shmem_undo_range+0x220/0x658
[ 189.185308] shmem_fallocate+0x2f0/0x470
[ 189.185527] vfs_fallocate+0x128/0x328
......
[ 189.187078] el0t_64_sync+0x184/0x188
"
After analysis, I believe the race exists between truncation and MADV_DONTNEED,
and shmem's ->map_pages() merely makes the issue easier to reproduce. Since
MADV_DONTNEED synchronously releases the pagetable page before calling
tlb_flush_rmaps(), this could cause another thread's truncation to skip
zap_pte_range() (due to pmd is none) but still observe the folio's mapcount
as non-zero. A possible race scenario is as follows:
CPU 0 CPU 1
shmem_fallocate madvise_dontneed_single_vma
unmap_mapping_range .......
......
(filemap_map_pages() remap folios)
zap_pte_range
truncate_inode_folio zap_empty_pte_table(pmd clear)
unmap_mapping_folio
......
zap_pmd_range (saw pmd none, skip)
filemap_remove_folio
BUG_ON(folio_mapped)
tlb_flush_rmaps(remove rmaps)
To fix this issue, we should move the pmd clear operation (via zap_empty_pte_table())
to after tlb_flush_rmaps() and use sync_with_folio_pmd_zap() to sync with
concurrent zapping, so that when we find a pmd_none() while unmapping a folio
without holding the PTL, the folio's mappings are guaranteed to have been
removed via tlb_flush_rmaps().
[1] https://lore.kernel.org/all/20260925065013.3682431-1-ayushr@modal.com/
Reported-by: Ayush Ranjan <ayushr@modal.com>
Closes: https://lore.kernel.org/all/20260924061708.1645968-1-ayushr@modal.com/
Fixes: 6375e95f381e ("mm: pgtable: reclaim empty PTE page in madvise(MADV_DONTNEED)")
Acked-by: Qi Zheng <qi.zheng@linux.dev>
Reviewed-by: Pedro Falcato <pfalcato@suse.de>
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
Changes from v4:
- Put the pmd_none() check in the 'else if' branch (per David).
Changes from v3:
- Collect review tags from Qi, Pedro and Lorenzo. Thanks.
- Update the related comments per Lorenzo.
Changes from v2:
- Use use sync_with_folio_pmd_zap() to sync with concurrent zapping (Per David).
Changes from v1:
- Add an smp_rmb() to pair with the smp_wmb() (Per Lorenzo and Pedro).
---
mm/internal.h | 10 ++++++----
mm/memory.c | 30 +++++++++++++++---------------
2 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/mm/internal.h b/mm/internal.h
index b1b0f29d2216..ffb8eaec90e3 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -584,14 +584,16 @@ void free_pgtables(struct mmu_gather *tlb, struct unmap_desc *desc);
void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte);
/**
- * sync_with_folio_pmd_zap - sync with concurrent zapping of a folio PMD
+ * sync_with_folio_pmd_zap - sync with concurrent zapping of folios
* @mm: The mm_struct.
* @pmdp: Pointer to the pmd that was found to be pmd_none().
*
* When we find a pmd_none() while unmapping a folio without holding the PTL,
- * zap_huge_pmd() may have cleared the PMD but not yet modified the folio to
- * indicate that it's unmapped. Skipping the PMD without synchronization could
- * make folio unmapping code assume that unmapping failed.
+ * zap_huge_pmd() may have cleared the PMD but not yet updated the folio
+ * mapcount, and zap_empty_pte_table() clears the PMD only after the rmaps
+ * for the table have been removed, but nothing orders the pmd_none() read
+ * against later mapcount reads. Skipping the PMD without synchronization
+ * could make folio unmapping code assume that unmapping failed.
*
* Wait for concurrent zapping to complete by grabbing the PTL.
*/
diff --git a/mm/memory.c b/mm/memory.c
index e911e63ba1ed..d9c54549b85b 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2036,6 +2036,15 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
}
} while (pte += nr, addr += PAGE_SIZE * nr, addr != end);
+ add_mm_rss_vec(mm, rss);
+ lazy_mmu_mode_disable();
+
+ /* Do the actual TLB flush before dropping ptl */
+ if (force_flush) {
+ tlb_flush_mmu_tlbonly(tlb);
+ tlb_flush_rmaps(tlb, vma);
+ }
+
/*
* Fast path: try to hold the pmd lock and unmap the PTE page.
*
@@ -2043,18 +2052,12 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
* to hold the pmd lock failed, then we need to recheck all pte entries
* to ensure they are still none, thereby preventing the pte entries
* from being repopulated by another thread.
+ *
+ * Clear the PMD after the rmaps for the table have been removed, in case
+ * an incorrect mapcount is observed after unmapping the folio.
*/
if (can_reclaim_pt && direct_reclaim && addr == end)
direct_reclaim = zap_empty_pte_table(mm, pmd, ptl, &pmdval);
-
- add_mm_rss_vec(mm, rss);
- lazy_mmu_mode_disable();
-
- /* Do the actual TLB flush before dropping ptl */
- if (force_flush) {
- tlb_flush_mmu_tlbonly(tlb);
- tlb_flush_rmaps(tlb, vma);
- }
pte_unmap_unlock(start_pte, ptl);
/*
@@ -2102,12 +2105,9 @@ static inline unsigned long zap_pmd_range(struct mmu_gather *tlb,
continue;
}
/* fall through */
- } else if (details && details->single_folio &&
- folio_test_pmd_mappable(details->single_folio) &&
- next - addr == HPAGE_PMD_SIZE && pmd_none(*pmd)) {
- sync_with_folio_pmd_zap(tlb->mm, pmd);
- }
- if (pmd_none(*pmd)) {
+ } else if (pmd_none(*pmd)) {
+ if (details && details->single_folio)
+ sync_with_folio_pmd_zap(tlb->mm, pmd);
addr = next;
continue;
}
--
2.47.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5] mm: memory: fix truncation removing mapped folios
2026-10-27 2:19 [PATCH v5] mm: memory: fix truncation removing mapped folios Baolin Wang
@ 2026-10-27 9:00 ` David Hildenbrand (Arm)
2026-10-27 9:03 ` Baolin Wang
0 siblings, 1 reply; 9+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-27 9:00 UTC (permalink / raw)
To: Baolin Wang, akpm
Cc: ljs, liam, vbabka, rppt, surenb, mhocko, qi.zheng, jack,
pfalcato, ayushr, linux-mm, linux-kernel
On 10/27/26 03:19, Baolin Wang wrote:
> Ayush reported an issue that shmem/tmpfs folios are still mapped when
> truncating them on production:
> "
> BUG: Bad page cache in process ... pfn:...
> page dumped because: still mapped when deleted
> "
>
> I reproduced the issue with Ayush's reproducer [1] with the latest mm-unstable
> branch, and I got the following crash info. From the dump message, we can
> see that truncate_inode_folio() is really trying to remove mapped folios,
> which is incorrect.
>
> "
> [ 189.171977] page: refcount:3 mapcount:1 mapping:00000000fe168ee1
> index:0x3880 pfn:0x191647
> [ 189.171995] memcg:ffff0000cc073d40
> [ 189.171997] aops:shmem_aops ino:3c01 dentry name(?):"memfd:runsc-memory"
> [ 189.172006] flags: 0x17fffef0002022d(locked|referenced|uptodate|lru|
> workingset|swapbacked|node=0|zone=2|lastcpupid=0x3ffff)
> .......
> [ 189.172017] page dumped because: VM_BUG_ON_FOLIO(folio_mapped(folio))
> [ 189.172026] ------------[ cut here ]------------
> [ 189.172027] kernel BUG at mm/filemap.c:155!
> [ 189.172057] Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> .......
> [ 189.177721] CPU: 3 UID: 0 PID: 5521 Comm: repro Kdump: loaded Tainted:
> 7.3.0-rc4+ #277 PREEMPT(lazy)
> .......
> [ 189.183986] Call trace:
> [ 189.184124] filemap_unaccount_folio+0xf0/0x1e8 (P)
> [ 189.184391] __filemap_remove_folio+0x34/0x160
> [ 189.184633] filemap_remove_folio+0x4c/0xb0
> [ 189.184859] truncate_inode_folio+0x34/0x58
> [ 189.185087] shmem_undo_range+0x220/0x658
> [ 189.185308] shmem_fallocate+0x2f0/0x470
> [ 189.185527] vfs_fallocate+0x128/0x328
> ......
> [ 189.187078] el0t_64_sync+0x184/0x188
> "
>
> After analysis, I believe the race exists between truncation and MADV_DONTNEED,
> and shmem's ->map_pages() merely makes the issue easier to reproduce. Since
> MADV_DONTNEED synchronously releases the pagetable page before calling
> tlb_flush_rmaps(), this could cause another thread's truncation to skip
> zap_pte_range() (due to pmd is none) but still observe the folio's mapcount
> as non-zero. A possible race scenario is as follows:
>
> CPU 0 CPU 1
> shmem_fallocate madvise_dontneed_single_vma
> unmap_mapping_range .......
> ......
> (filemap_map_pages() remap folios)
> zap_pte_range
> truncate_inode_folio zap_empty_pte_table(pmd clear)
> unmap_mapping_folio
> ......
> zap_pmd_range (saw pmd none, skip)
>
> filemap_remove_folio
> BUG_ON(folio_mapped)
> tlb_flush_rmaps(remove rmaps)
>
> To fix this issue, we should move the pmd clear operation (via zap_empty_pte_table())
> to after tlb_flush_rmaps() and use sync_with_folio_pmd_zap() to sync with
> concurrent zapping, so that when we find a pmd_none() while unmapping a folio
> without holding the PTL, the folio's mappings are guaranteed to have been
> removed via tlb_flush_rmaps().
>
> [1] https://lore.kernel.org/all/20260925065013.3682431-1-ayushr@modal.com/
> Reported-by: Ayush Ranjan <ayushr@modal.com>
> Closes: https://lore.kernel.org/all/20260924061708.1645968-1-ayushr@modal.com/
> Fixes: 6375e95f381e ("mm: pgtable: reclaim empty PTE page in madvise(MADV_DONTNEED)")
> Acked-by: Qi Zheng <qi.zheng@linux.dev>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
> Changes from v4:
> - Put the pmd_none() check in the 'else if' branch (per David).
> Changes from v3:
> - Collect review tags from Qi, Pedro and Lorenzo. Thanks.
> - Update the related comments per Lorenzo.
> Changes from v2:
> - Use use sync_with_folio_pmd_zap() to sync with concurrent zapping (Per David).
> Changes from v1:
> - Add an smp_rmb() to pair with the smp_wmb() (Per Lorenzo and Pedro).
> ---
> mm/internal.h | 10 ++++++----
> mm/memory.c | 30 +++++++++++++++---------------
> 2 files changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/mm/internal.h b/mm/internal.h
> index b1b0f29d2216..ffb8eaec90e3 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -584,14 +584,16 @@ void free_pgtables(struct mmu_gather *tlb, struct unmap_desc *desc);
> void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte);
>
> /**
> - * sync_with_folio_pmd_zap - sync with concurrent zapping of a folio PMD
> + * sync_with_folio_pmd_zap - sync with concurrent zapping of folios
> * @mm: The mm_struct.
> * @pmdp: Pointer to the pmd that was found to be pmd_none().
> *
> * When we find a pmd_none() while unmapping a folio without holding the PTL,
> - * zap_huge_pmd() may have cleared the PMD but not yet modified the folio to
> - * indicate that it's unmapped. Skipping the PMD without synchronization could
> - * make folio unmapping code assume that unmapping failed.
> + * zap_huge_pmd() may have cleared the PMD but not yet updated the folio
> + * mapcount, and zap_empty_pte_table() clears the PMD only after the rmaps
> + * for the table have been removed, but nothing orders the pmd_none() read
> + * against later mapcount reads. Skipping the PMD without synchronization
> + * could make folio unmapping code assume that unmapping failed.
> *
> * Wait for concurrent zapping to complete by grabbing the PTL.
> */
> diff --git a/mm/memory.c b/mm/memory.c
> index e911e63ba1ed..d9c54549b85b 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2036,6 +2036,15 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
> }
> } while (pte += nr, addr += PAGE_SIZE * nr, addr != end);
>
> + add_mm_rss_vec(mm, rss);
> + lazy_mmu_mode_disable();
> +
> + /* Do the actual TLB flush before dropping ptl */
> + if (force_flush) {
> + tlb_flush_mmu_tlbonly(tlb);
> + tlb_flush_rmaps(tlb, vma);
> + }
> +
> /*
> * Fast path: try to hold the pmd lock and unmap the PTE page.
> *
> @@ -2043,18 +2052,12 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
> * to hold the pmd lock failed, then we need to recheck all pte entries
> * to ensure they are still none, thereby preventing the pte entries
> * from being repopulated by another thread.
> + *
> + * Clear the PMD after the rmaps for the table have been removed, in case
> + * an incorrect mapcount is observed after unmapping the folio.
> */
> if (can_reclaim_pt && direct_reclaim && addr == end)
> direct_reclaim = zap_empty_pte_table(mm, pmd, ptl, &pmdval);
> -
> - add_mm_rss_vec(mm, rss);
> - lazy_mmu_mode_disable();
> -
> - /* Do the actual TLB flush before dropping ptl */
> - if (force_flush) {
> - tlb_flush_mmu_tlbonly(tlb);
> - tlb_flush_rmaps(tlb, vma);
> - }
> pte_unmap_unlock(start_pte, ptl);
>
> /*
> @@ -2102,12 +2105,9 @@ static inline unsigned long zap_pmd_range(struct mmu_gather *tlb,
> continue;
> }
> /* fall through */
> - } else if (details && details->single_folio &&
> - folio_test_pmd_mappable(details->single_folio) &&
> - next - addr == HPAGE_PMD_SIZE && pmd_none(*pmd)) {
> - sync_with_folio_pmd_zap(tlb->mm, pmd);
> - }
> - if (pmd_none(*pmd)) {
> + } else if (pmd_none(*pmd)) {
> + if (details && details->single_folio)
> + sync_with_folio_pmd_zap(tlb->mm, pmd);
> addr = next;
> continue;
> }
} else if (pmd_none(*pmd) && details && details->single_folio)) {
sync_with_folio_pmd_zap(tlb->mm, pmd);
}
if (pmd_none(*pmd) {
addr = next;
continue;
}
Minimal change required, no?
--
Cheers,
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5] mm: memory: fix truncation removing mapped folios
2026-10-27 9:00 ` David Hildenbrand (Arm)
@ 2026-10-27 9:03 ` Baolin Wang
2026-10-27 9:33 ` David Hildenbrand (Arm)
0 siblings, 1 reply; 9+ messages in thread
From: Baolin Wang @ 2026-10-27 9:03 UTC (permalink / raw)
To: David Hildenbrand (Arm), akpm
Cc: ljs, liam, vbabka, rppt, surenb, mhocko, qi.zheng, jack,
pfalcato, ayushr, linux-mm, linux-kernel
On 10/27/26 5:00 PM, David Hildenbrand (Arm) wrote:
> On 10/27/26 03:19, Baolin Wang wrote:
>> Ayush reported an issue that shmem/tmpfs folios are still mapped when
>> truncating them on production:
>> "
>> BUG: Bad page cache in process ... pfn:...
>> page dumped because: still mapped when deleted
>> "
>>
>> I reproduced the issue with Ayush's reproducer [1] with the latest mm-unstable
>> branch, and I got the following crash info. From the dump message, we can
>> see that truncate_inode_folio() is really trying to remove mapped folios,
>> which is incorrect.
>>
>> "
>> [ 189.171977] page: refcount:3 mapcount:1 mapping:00000000fe168ee1
>> index:0x3880 pfn:0x191647
>> [ 189.171995] memcg:ffff0000cc073d40
>> [ 189.171997] aops:shmem_aops ino:3c01 dentry name(?):"memfd:runsc-memory"
>> [ 189.172006] flags: 0x17fffef0002022d(locked|referenced|uptodate|lru|
>> workingset|swapbacked|node=0|zone=2|lastcpupid=0x3ffff)
>> .......
>> [ 189.172017] page dumped because: VM_BUG_ON_FOLIO(folio_mapped(folio))
>> [ 189.172026] ------------[ cut here ]------------
>> [ 189.172027] kernel BUG at mm/filemap.c:155!
>> [ 189.172057] Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
>> .......
>> [ 189.177721] CPU: 3 UID: 0 PID: 5521 Comm: repro Kdump: loaded Tainted:
>> 7.3.0-rc4+ #277 PREEMPT(lazy)
>> .......
>> [ 189.183986] Call trace:
>> [ 189.184124] filemap_unaccount_folio+0xf0/0x1e8 (P)
>> [ 189.184391] __filemap_remove_folio+0x34/0x160
>> [ 189.184633] filemap_remove_folio+0x4c/0xb0
>> [ 189.184859] truncate_inode_folio+0x34/0x58
>> [ 189.185087] shmem_undo_range+0x220/0x658
>> [ 189.185308] shmem_fallocate+0x2f0/0x470
>> [ 189.185527] vfs_fallocate+0x128/0x328
>> ......
>> [ 189.187078] el0t_64_sync+0x184/0x188
>> "
>>
>> After analysis, I believe the race exists between truncation and MADV_DONTNEED,
>> and shmem's ->map_pages() merely makes the issue easier to reproduce. Since
>> MADV_DONTNEED synchronously releases the pagetable page before calling
>> tlb_flush_rmaps(), this could cause another thread's truncation to skip
>> zap_pte_range() (due to pmd is none) but still observe the folio's mapcount
>> as non-zero. A possible race scenario is as follows:
>>
>> CPU 0 CPU 1
>> shmem_fallocate madvise_dontneed_single_vma
>> unmap_mapping_range .......
>> ......
>> (filemap_map_pages() remap folios)
>> zap_pte_range
>> truncate_inode_folio zap_empty_pte_table(pmd clear)
>> unmap_mapping_folio
>> ......
>> zap_pmd_range (saw pmd none, skip)
>>
>> filemap_remove_folio
>> BUG_ON(folio_mapped)
>> tlb_flush_rmaps(remove rmaps)
>>
>> To fix this issue, we should move the pmd clear operation (via zap_empty_pte_table())
>> to after tlb_flush_rmaps() and use sync_with_folio_pmd_zap() to sync with
>> concurrent zapping, so that when we find a pmd_none() while unmapping a folio
>> without holding the PTL, the folio's mappings are guaranteed to have been
>> removed via tlb_flush_rmaps().
>>
>> [1] https://lore.kernel.org/all/20260925065013.3682431-1-ayushr@modal.com/
>> Reported-by: Ayush Ranjan <ayushr@modal.com>
>> Closes: https://lore.kernel.org/all/20260924061708.1645968-1-ayushr@modal.com/
>> Fixes: 6375e95f381e ("mm: pgtable: reclaim empty PTE page in madvise(MADV_DONTNEED)")
>> Acked-by: Qi Zheng <qi.zheng@linux.dev>
>> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
>> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>> ---
>> Changes from v4:
>> - Put the pmd_none() check in the 'else if' branch (per David).
>> Changes from v3:
>> - Collect review tags from Qi, Pedro and Lorenzo. Thanks.
>> - Update the related comments per Lorenzo.
>> Changes from v2:
>> - Use use sync_with_folio_pmd_zap() to sync with concurrent zapping (Per David).
>> Changes from v1:
>> - Add an smp_rmb() to pair with the smp_wmb() (Per Lorenzo and Pedro).
>> ---
>> mm/internal.h | 10 ++++++----
>> mm/memory.c | 30 +++++++++++++++---------------
>> 2 files changed, 21 insertions(+), 19 deletions(-)
>>
>> diff --git a/mm/internal.h b/mm/internal.h
>> index b1b0f29d2216..ffb8eaec90e3 100644
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -584,14 +584,16 @@ void free_pgtables(struct mmu_gather *tlb, struct unmap_desc *desc);
>> void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte);
>>
>> /**
>> - * sync_with_folio_pmd_zap - sync with concurrent zapping of a folio PMD
>> + * sync_with_folio_pmd_zap - sync with concurrent zapping of folios
>> * @mm: The mm_struct.
>> * @pmdp: Pointer to the pmd that was found to be pmd_none().
>> *
>> * When we find a pmd_none() while unmapping a folio without holding the PTL,
>> - * zap_huge_pmd() may have cleared the PMD but not yet modified the folio to
>> - * indicate that it's unmapped. Skipping the PMD without synchronization could
>> - * make folio unmapping code assume that unmapping failed.
>> + * zap_huge_pmd() may have cleared the PMD but not yet updated the folio
>> + * mapcount, and zap_empty_pte_table() clears the PMD only after the rmaps
>> + * for the table have been removed, but nothing orders the pmd_none() read
>> + * against later mapcount reads. Skipping the PMD without synchronization
>> + * could make folio unmapping code assume that unmapping failed.
>> *
>> * Wait for concurrent zapping to complete by grabbing the PTL.
>> */
>> diff --git a/mm/memory.c b/mm/memory.c
>> index e911e63ba1ed..d9c54549b85b 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -2036,6 +2036,15 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
>> }
>> } while (pte += nr, addr += PAGE_SIZE * nr, addr != end);
>>
>> + add_mm_rss_vec(mm, rss);
>> + lazy_mmu_mode_disable();
>> +
>> + /* Do the actual TLB flush before dropping ptl */
>> + if (force_flush) {
>> + tlb_flush_mmu_tlbonly(tlb);
>> + tlb_flush_rmaps(tlb, vma);
>> + }
>> +
>> /*
>> * Fast path: try to hold the pmd lock and unmap the PTE page.
>> *
>> @@ -2043,18 +2052,12 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
>> * to hold the pmd lock failed, then we need to recheck all pte entries
>> * to ensure they are still none, thereby preventing the pte entries
>> * from being repopulated by another thread.
>> + *
>> + * Clear the PMD after the rmaps for the table have been removed, in case
>> + * an incorrect mapcount is observed after unmapping the folio.
>> */
>> if (can_reclaim_pt && direct_reclaim && addr == end)
>> direct_reclaim = zap_empty_pte_table(mm, pmd, ptl, &pmdval);
>> -
>> - add_mm_rss_vec(mm, rss);
>> - lazy_mmu_mode_disable();
>> -
>> - /* Do the actual TLB flush before dropping ptl */
>> - if (force_flush) {
>> - tlb_flush_mmu_tlbonly(tlb);
>> - tlb_flush_rmaps(tlb, vma);
>> - }
>> pte_unmap_unlock(start_pte, ptl);
>>
>> /*
>> @@ -2102,12 +2105,9 @@ static inline unsigned long zap_pmd_range(struct mmu_gather *tlb,
>> continue;
>> }
>> /* fall through */
>> - } else if (details && details->single_folio &&
>> - folio_test_pmd_mappable(details->single_folio) &&
>> - next - addr == HPAGE_PMD_SIZE && pmd_none(*pmd)) {
>> - sync_with_folio_pmd_zap(tlb->mm, pmd);
>> - }
>> - if (pmd_none(*pmd)) {
>> + } else if (pmd_none(*pmd)) {
>> + if (details && details->single_folio)
>> + sync_with_folio_pmd_zap(tlb->mm, pmd);
>> addr = next;
>> continue;
>> }
>
>
>
> } else if (pmd_none(*pmd) && details && details->single_folio)) {
> sync_with_folio_pmd_zap(tlb->mm, pmd);
> }
>
> if (pmd_none(*pmd) {
> addr = next;
> continue;
> }
>
>
> Minimal change required, no?
No, keeping two pmd_none() checks cannot fix the race, as I replied
before. [1]
[1]
https://lore.kernel.org/all/179c26ed-1764-4e5b-a7ee-047ddabbfea1@linux.alibaba.com/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5] mm: memory: fix truncation removing mapped folios
2026-10-27 9:03 ` Baolin Wang
@ 2026-10-27 9:33 ` David Hildenbrand (Arm)
2026-10-27 10:35 ` Baolin Wang
0 siblings, 1 reply; 9+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-27 9:33 UTC (permalink / raw)
To: Baolin Wang, akpm
Cc: ljs, liam, vbabka, rppt, surenb, mhocko, qi.zheng, jack,
pfalcato, ayushr, linux-mm, linux-kernel
On 10/27/26 10:03, Baolin Wang wrote:
>
>
> On 10/27/26 5:00 PM, David Hildenbrand (Arm) wrote:
>> On 10/27/26 03:19, Baolin Wang wrote:
>>> Ayush reported an issue that shmem/tmpfs folios are still mapped when
>>> truncating them on production:
>>> "
>>> BUG: Bad page cache in process ... pfn:...
>>> page dumped because: still mapped when deleted
>>> "
>>>
>>> I reproduced the issue with Ayush's reproducer [1] with the latest mm-unstable
>>> branch, and I got the following crash info. From the dump message, we can
>>> see that truncate_inode_folio() is really trying to remove mapped folios,
>>> which is incorrect.
>>>
>>> "
>>> [ 189.171977] page: refcount:3 mapcount:1 mapping:00000000fe168ee1
>>> index:0x3880 pfn:0x191647
>>> [ 189.171995] memcg:ffff0000cc073d40
>>> [ 189.171997] aops:shmem_aops ino:3c01 dentry name(?):"memfd:runsc-memory"
>>> [ 189.172006] flags: 0x17fffef0002022d(locked|referenced|uptodate|lru|
>>> workingset|swapbacked|node=0|zone=2|lastcpupid=0x3ffff)
>>> .......
>>> [ 189.172017] page dumped because: VM_BUG_ON_FOLIO(folio_mapped(folio))
>>> [ 189.172026] ------------[ cut here ]------------
>>> [ 189.172027] kernel BUG at mm/filemap.c:155!
>>> [ 189.172057] Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
>>> .......
>>> [ 189.177721] CPU: 3 UID: 0 PID: 5521 Comm: repro Kdump: loaded Tainted:
>>> 7.3.0-rc4+ #277 PREEMPT(lazy)
>>> .......
>>> [ 189.183986] Call trace:
>>> [ 189.184124] filemap_unaccount_folio+0xf0/0x1e8 (P)
>>> [ 189.184391] __filemap_remove_folio+0x34/0x160
>>> [ 189.184633] filemap_remove_folio+0x4c/0xb0
>>> [ 189.184859] truncate_inode_folio+0x34/0x58
>>> [ 189.185087] shmem_undo_range+0x220/0x658
>>> [ 189.185308] shmem_fallocate+0x2f0/0x470
>>> [ 189.185527] vfs_fallocate+0x128/0x328
>>> ......
>>> [ 189.187078] el0t_64_sync+0x184/0x188
>>> "
>>>
>>> After analysis, I believe the race exists between truncation and MADV_DONTNEED,
>>> and shmem's ->map_pages() merely makes the issue easier to reproduce. Since
>>> MADV_DONTNEED synchronously releases the pagetable page before calling
>>> tlb_flush_rmaps(), this could cause another thread's truncation to skip
>>> zap_pte_range() (due to pmd is none) but still observe the folio's mapcount
>>> as non-zero. A possible race scenario is as follows:
>>>
>>> CPU 0 CPU 1
>>> shmem_fallocate madvise_dontneed_single_vma
>>> unmap_mapping_range .......
>>> ......
>>> (filemap_map_pages() remap folios)
>>> zap_pte_range
>>> truncate_inode_folio zap_empty_pte_table(pmd clear)
>>> unmap_mapping_folio
>>> ......
>>> zap_pmd_range (saw pmd none, skip)
>>>
>>> filemap_remove_folio
>>> BUG_ON(folio_mapped)
>>> tlb_flush_rmaps(remove rmaps)
>>>
>>> To fix this issue, we should move the pmd clear operation (via
>>> zap_empty_pte_table())
>>> to after tlb_flush_rmaps() and use sync_with_folio_pmd_zap() to sync with
>>> concurrent zapping, so that when we find a pmd_none() while unmapping a folio
>>> without holding the PTL, the folio's mappings are guaranteed to have been
>>> removed via tlb_flush_rmaps().
>>>
>>> [1] https://lore.kernel.org/all/20260925065013.3682431-1-ayushr@modal.com/
>>> Reported-by: Ayush Ranjan <ayushr@modal.com>
>>> Closes: https://lore.kernel.org/all/20260924061708.1645968-1-ayushr@modal.com/
>>> Fixes: 6375e95f381e ("mm: pgtable: reclaim empty PTE page in
>>> madvise(MADV_DONTNEED)")
>>> Acked-by: Qi Zheng <qi.zheng@linux.dev>
>>> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
>>> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>>> ---
>>> Changes from v4:
>>> - Put the pmd_none() check in the 'else if' branch (per David).
>>> Changes from v3:
>>> - Collect review tags from Qi, Pedro and Lorenzo. Thanks.
>>> - Update the related comments per Lorenzo.
>>> Changes from v2:
>>> - Use use sync_with_folio_pmd_zap() to sync with concurrent zapping (Per
>>> David).
>>> Changes from v1:
>>> - Add an smp_rmb() to pair with the smp_wmb() (Per Lorenzo and Pedro).
>>> ---
>>> mm/internal.h | 10 ++++++----
>>> mm/memory.c | 30 +++++++++++++++---------------
>>> 2 files changed, 21 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/mm/internal.h b/mm/internal.h
>>> index b1b0f29d2216..ffb8eaec90e3 100644
>>> --- a/mm/internal.h
>>> +++ b/mm/internal.h
>>> @@ -584,14 +584,16 @@ void free_pgtables(struct mmu_gather *tlb, struct
>>> unmap_desc *desc);
>>> void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte);
>>> /**
>>> - * sync_with_folio_pmd_zap - sync with concurrent zapping of a folio PMD
>>> + * sync_with_folio_pmd_zap - sync with concurrent zapping of folios
>>> * @mm: The mm_struct.
>>> * @pmdp: Pointer to the pmd that was found to be pmd_none().
>>> *
>>> * When we find a pmd_none() while unmapping a folio without holding the PTL,
>>> - * zap_huge_pmd() may have cleared the PMD but not yet modified the folio to
>>> - * indicate that it's unmapped. Skipping the PMD without synchronization could
>>> - * make folio unmapping code assume that unmapping failed.
>>> + * zap_huge_pmd() may have cleared the PMD but not yet updated the folio
>>> + * mapcount, and zap_empty_pte_table() clears the PMD only after the rmaps
>>> + * for the table have been removed, but nothing orders the pmd_none() read
>>> + * against later mapcount reads. Skipping the PMD without synchronization
>>> + * could make folio unmapping code assume that unmapping failed.
>>> *
>>> * Wait for concurrent zapping to complete by grabbing the PTL.
>>> */
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index e911e63ba1ed..d9c54549b85b 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -2036,6 +2036,15 @@ static unsigned long zap_pte_range(struct mmu_gather
>>> *tlb,
>>> }
>>> } while (pte += nr, addr += PAGE_SIZE * nr, addr != end);
>>> + add_mm_rss_vec(mm, rss);
>>> + lazy_mmu_mode_disable();
>>> +
>>> + /* Do the actual TLB flush before dropping ptl */
>>> + if (force_flush) {
>>> + tlb_flush_mmu_tlbonly(tlb);
>>> + tlb_flush_rmaps(tlb, vma);
>>> + }
>>> +
>>> /*
>>> * Fast path: try to hold the pmd lock and unmap the PTE page.
>>> *
>>> @@ -2043,18 +2052,12 @@ static unsigned long zap_pte_range(struct mmu_gather
>>> *tlb,
>>> * to hold the pmd lock failed, then we need to recheck all pte entries
>>> * to ensure they are still none, thereby preventing the pte entries
>>> * from being repopulated by another thread.
>>> + *
>>> + * Clear the PMD after the rmaps for the table have been removed, in case
>>> + * an incorrect mapcount is observed after unmapping the folio.
>>> */
>>> if (can_reclaim_pt && direct_reclaim && addr == end)
>>> direct_reclaim = zap_empty_pte_table(mm, pmd, ptl, &pmdval);
>>> -
>>> - add_mm_rss_vec(mm, rss);
>>> - lazy_mmu_mode_disable();
>>> -
>>> - /* Do the actual TLB flush before dropping ptl */
>>> - if (force_flush) {
>>> - tlb_flush_mmu_tlbonly(tlb);
>>> - tlb_flush_rmaps(tlb, vma);
>>> - }
>>> pte_unmap_unlock(start_pte, ptl);
>>> /*
>>> @@ -2102,12 +2105,9 @@ static inline unsigned long zap_pmd_range(struct
>>> mmu_gather *tlb,
>>> continue;
>>> }
>>> /* fall through */
>>> - } else if (details && details->single_folio &&
>>> - folio_test_pmd_mappable(details->single_folio) &&
>>> - next - addr == HPAGE_PMD_SIZE && pmd_none(*pmd)) {
>>> - sync_with_folio_pmd_zap(tlb->mm, pmd);
>>> - }
>>> - if (pmd_none(*pmd)) {
>>> + } else if (pmd_none(*pmd)) {
>>> + if (details && details->single_folio)
>>> + sync_with_folio_pmd_zap(tlb->mm, pmd);
>>> addr = next;
>>> continue;
>>> }
>>
>>
>>
>> } else if (pmd_none(*pmd) && details && details->single_folio)) {
>> sync_with_folio_pmd_zap(tlb->mm, pmd);
>> }
>>
>> if (pmd_none(*pmd) {
>> addr = next;
>> continue;
>> }
>>
>>
>> Minimal change required, no?
>
> No, keeping two pmd_none() checks cannot fix the race, as I replied before. [1]
>
Best to wait to submit new versions if there are open discussions? Because now
we have the same discussion in two places.
I think you're right about the race, the issue is that we check for
pmd_is_huge(), not pmd_present() which confused me.
Clearest to me would be to not drop the second pmd_none() check but move it
where it belongs: if splitting zapped the PMD.
@@ -2101,13 +2104,15 @@ static inline unsigned long zap_pmd_range(struct
mmu_gather *tlb,
addr = next;
continue;
}
+ /* We already synchronized with concurrent zapping. */
+ if (pmd_none(*pmd)) {
+ addr = next;
+ continue;
+ }
/* fall through */
- } else if (details && details->single_folio &&
- folio_test_pmd_mappable(details->single_folio) &&
- next - addr == HPAGE_PMD_SIZE && pmd_none(*pmd)) {
- sync_with_folio_pmd_zap(tlb->mm, pmd);
- }
- if (pmd_none(*pmd)) {
+ } else if (pmd_none(*pmd)) {
+ if (details && details->single_folio)
+ sync_with_folio_pmd_zap(tlb->mm, pmd);
addr = next;
continue;
}
Removing the "if (pmd_none(*pmd)) {" in your variant is what confused me,
because it looks like we no longer happen the case where we zapped the PMD
ourselves.
--
Cheers,
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5] mm: memory: fix truncation removing mapped folios
2026-10-27 9:33 ` David Hildenbrand (Arm)
@ 2026-10-27 10:35 ` Baolin Wang
2026-10-27 10:55 ` David Hildenbrand (Arm)
0 siblings, 1 reply; 9+ messages in thread
From: Baolin Wang @ 2026-10-27 10:35 UTC (permalink / raw)
To: David Hildenbrand (Arm), akpm
Cc: ljs, liam, vbabka, rppt, surenb, mhocko, qi.zheng, jack,
pfalcato, ayushr, linux-mm, linux-kernel
On 10/27/26 5:33 PM, David Hildenbrand (Arm) wrote:
> On 10/27/26 10:03, Baolin Wang wrote:
>>
>>
>> On 10/27/26 5:00 PM, David Hildenbrand (Arm) wrote:
>>> On 10/27/26 03:19, Baolin Wang wrote:
>>>> Ayush reported an issue that shmem/tmpfs folios are still mapped when
>>>> truncating them on production:
>>>> "
>>>> BUG: Bad page cache in process ... pfn:...
>>>> page dumped because: still mapped when deleted
>>>> "
>>>>
>>>> I reproduced the issue with Ayush's reproducer [1] with the latest mm-unstable
>>>> branch, and I got the following crash info. From the dump message, we can
>>>> see that truncate_inode_folio() is really trying to remove mapped folios,
>>>> which is incorrect.
>>>>
>>>> "
>>>> [ 189.171977] page: refcount:3 mapcount:1 mapping:00000000fe168ee1
>>>> index:0x3880 pfn:0x191647
>>>> [ 189.171995] memcg:ffff0000cc073d40
>>>> [ 189.171997] aops:shmem_aops ino:3c01 dentry name(?):"memfd:runsc-memory"
>>>> [ 189.172006] flags: 0x17fffef0002022d(locked|referenced|uptodate|lru|
>>>> workingset|swapbacked|node=0|zone=2|lastcpupid=0x3ffff)
>>>> .......
>>>> [ 189.172017] page dumped because: VM_BUG_ON_FOLIO(folio_mapped(folio))
>>>> [ 189.172026] ------------[ cut here ]------------
>>>> [ 189.172027] kernel BUG at mm/filemap.c:155!
>>>> [ 189.172057] Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
>>>> .......
>>>> [ 189.177721] CPU: 3 UID: 0 PID: 5521 Comm: repro Kdump: loaded Tainted:
>>>> 7.3.0-rc4+ #277 PREEMPT(lazy)
>>>> .......
>>>> [ 189.183986] Call trace:
>>>> [ 189.184124] filemap_unaccount_folio+0xf0/0x1e8 (P)
>>>> [ 189.184391] __filemap_remove_folio+0x34/0x160
>>>> [ 189.184633] filemap_remove_folio+0x4c/0xb0
>>>> [ 189.184859] truncate_inode_folio+0x34/0x58
>>>> [ 189.185087] shmem_undo_range+0x220/0x658
>>>> [ 189.185308] shmem_fallocate+0x2f0/0x470
>>>> [ 189.185527] vfs_fallocate+0x128/0x328
>>>> ......
>>>> [ 189.187078] el0t_64_sync+0x184/0x188
>>>> "
>>>>
>>>> After analysis, I believe the race exists between truncation and MADV_DONTNEED,
>>>> and shmem's ->map_pages() merely makes the issue easier to reproduce. Since
>>>> MADV_DONTNEED synchronously releases the pagetable page before calling
>>>> tlb_flush_rmaps(), this could cause another thread's truncation to skip
>>>> zap_pte_range() (due to pmd is none) but still observe the folio's mapcount
>>>> as non-zero. A possible race scenario is as follows:
>>>>
>>>> CPU 0 CPU 1
>>>> shmem_fallocate madvise_dontneed_single_vma
>>>> unmap_mapping_range .......
>>>> ......
>>>> (filemap_map_pages() remap folios)
>>>> zap_pte_range
>>>> truncate_inode_folio zap_empty_pte_table(pmd clear)
>>>> unmap_mapping_folio
>>>> ......
>>>> zap_pmd_range (saw pmd none, skip)
>>>>
>>>> filemap_remove_folio
>>>> BUG_ON(folio_mapped)
>>>> tlb_flush_rmaps(remove rmaps)
>>>>
>>>> To fix this issue, we should move the pmd clear operation (via
>>>> zap_empty_pte_table())
>>>> to after tlb_flush_rmaps() and use sync_with_folio_pmd_zap() to sync with
>>>> concurrent zapping, so that when we find a pmd_none() while unmapping a folio
>>>> without holding the PTL, the folio's mappings are guaranteed to have been
>>>> removed via tlb_flush_rmaps().
>>>>
>>>> [1] https://lore.kernel.org/all/20260925065013.3682431-1-ayushr@modal.com/
>>>> Reported-by: Ayush Ranjan <ayushr@modal.com>
>>>> Closes: https://lore.kernel.org/all/20260924061708.1645968-1-ayushr@modal.com/
>>>> Fixes: 6375e95f381e ("mm: pgtable: reclaim empty PTE page in
>>>> madvise(MADV_DONTNEED)")
>>>> Acked-by: Qi Zheng <qi.zheng@linux.dev>
>>>> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
>>>> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>>>> ---
>>>> Changes from v4:
>>>> - Put the pmd_none() check in the 'else if' branch (per David).
>>>> Changes from v3:
>>>> - Collect review tags from Qi, Pedro and Lorenzo. Thanks.
>>>> - Update the related comments per Lorenzo.
>>>> Changes from v2:
>>>> - Use use sync_with_folio_pmd_zap() to sync with concurrent zapping (Per
>>>> David).
>>>> Changes from v1:
>>>> - Add an smp_rmb() to pair with the smp_wmb() (Per Lorenzo and Pedro).
>>>> ---
>>>> mm/internal.h | 10 ++++++----
>>>> mm/memory.c | 30 +++++++++++++++---------------
>>>> 2 files changed, 21 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/mm/internal.h b/mm/internal.h
>>>> index b1b0f29d2216..ffb8eaec90e3 100644
>>>> --- a/mm/internal.h
>>>> +++ b/mm/internal.h
>>>> @@ -584,14 +584,16 @@ void free_pgtables(struct mmu_gather *tlb, struct
>>>> unmap_desc *desc);
>>>> void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte);
>>>> /**
>>>> - * sync_with_folio_pmd_zap - sync with concurrent zapping of a folio PMD
>>>> + * sync_with_folio_pmd_zap - sync with concurrent zapping of folios
>>>> * @mm: The mm_struct.
>>>> * @pmdp: Pointer to the pmd that was found to be pmd_none().
>>>> *
>>>> * When we find a pmd_none() while unmapping a folio without holding the PTL,
>>>> - * zap_huge_pmd() may have cleared the PMD but not yet modified the folio to
>>>> - * indicate that it's unmapped. Skipping the PMD without synchronization could
>>>> - * make folio unmapping code assume that unmapping failed.
>>>> + * zap_huge_pmd() may have cleared the PMD but not yet updated the folio
>>>> + * mapcount, and zap_empty_pte_table() clears the PMD only after the rmaps
>>>> + * for the table have been removed, but nothing orders the pmd_none() read
>>>> + * against later mapcount reads. Skipping the PMD without synchronization
>>>> + * could make folio unmapping code assume that unmapping failed.
>>>> *
>>>> * Wait for concurrent zapping to complete by grabbing the PTL.
>>>> */
>>>> diff --git a/mm/memory.c b/mm/memory.c
>>>> index e911e63ba1ed..d9c54549b85b 100644
>>>> --- a/mm/memory.c
>>>> +++ b/mm/memory.c
>>>> @@ -2036,6 +2036,15 @@ static unsigned long zap_pte_range(struct mmu_gather
>>>> *tlb,
>>>> }
>>>> } while (pte += nr, addr += PAGE_SIZE * nr, addr != end);
>>>> + add_mm_rss_vec(mm, rss);
>>>> + lazy_mmu_mode_disable();
>>>> +
>>>> + /* Do the actual TLB flush before dropping ptl */
>>>> + if (force_flush) {
>>>> + tlb_flush_mmu_tlbonly(tlb);
>>>> + tlb_flush_rmaps(tlb, vma);
>>>> + }
>>>> +
>>>> /*
>>>> * Fast path: try to hold the pmd lock and unmap the PTE page.
>>>> *
>>>> @@ -2043,18 +2052,12 @@ static unsigned long zap_pte_range(struct mmu_gather
>>>> *tlb,
>>>> * to hold the pmd lock failed, then we need to recheck all pte entries
>>>> * to ensure they are still none, thereby preventing the pte entries
>>>> * from being repopulated by another thread.
>>>> + *
>>>> + * Clear the PMD after the rmaps for the table have been removed, in case
>>>> + * an incorrect mapcount is observed after unmapping the folio.
>>>> */
>>>> if (can_reclaim_pt && direct_reclaim && addr == end)
>>>> direct_reclaim = zap_empty_pte_table(mm, pmd, ptl, &pmdval);
>>>> -
>>>> - add_mm_rss_vec(mm, rss);
>>>> - lazy_mmu_mode_disable();
>>>> -
>>>> - /* Do the actual TLB flush before dropping ptl */
>>>> - if (force_flush) {
>>>> - tlb_flush_mmu_tlbonly(tlb);
>>>> - tlb_flush_rmaps(tlb, vma);
>>>> - }
>>>> pte_unmap_unlock(start_pte, ptl);
>>>> /*
>>>> @@ -2102,12 +2105,9 @@ static inline unsigned long zap_pmd_range(struct
>>>> mmu_gather *tlb,
>>>> continue;
>>>> }
>>>> /* fall through */
>>>> - } else if (details && details->single_folio &&
>>>> - folio_test_pmd_mappable(details->single_folio) &&
>>>> - next - addr == HPAGE_PMD_SIZE && pmd_none(*pmd)) {
>>>> - sync_with_folio_pmd_zap(tlb->mm, pmd);
>>>> - }
>>>> - if (pmd_none(*pmd)) {
>>>> + } else if (pmd_none(*pmd)) {
>>>> + if (details && details->single_folio)
>>>> + sync_with_folio_pmd_zap(tlb->mm, pmd);
>>>> addr = next;
>>>> continue;
>>>> }
>>>
>>>
>>>
>>> } else if (pmd_none(*pmd) && details && details->single_folio)) {
>>> sync_with_folio_pmd_zap(tlb->mm, pmd);
>>> }
>>>
>>> if (pmd_none(*pmd) {
>>> addr = next;
>>> continue;
>>> }
>>>
>>>
>>> Minimal change required, no?
>>
>> No, keeping two pmd_none() checks cannot fix the race, as I replied before. [1]
>>
>
> Best to wait to submit new versions if there are open discussions? Because now
> we have the same discussion in two places.
Sure.
>
> I think you're right about the race, the issue is that we check for
> pmd_is_huge(), not pmd_present() which confused me.
>
> Clearest to me would be to not drop the second pmd_none() check but move it
> where it belongs: if splitting zapped the PMD.
>
> @@ -2101,13 +2104,15 @@ static inline unsigned long zap_pmd_range(struct
> mmu_gather *tlb,
> addr = next;
> continue;
> }
> + /* We already synchronized with concurrent zapping. */
> + if (pmd_none(*pmd)) {
> + addr = next;
> + continue;
> + }
> /* fall through */
> - } else if (details && details->single_folio &&
> - folio_test_pmd_mappable(details->single_folio) &&
> - next - addr == HPAGE_PMD_SIZE && pmd_none(*pmd)) {
> - sync_with_folio_pmd_zap(tlb->mm, pmd);
> - }
> - if (pmd_none(*pmd)) {
> + } else if (pmd_none(*pmd)) {
> + if (details && details->single_folio)
> + sync_with_folio_pmd_zap(tlb->mm, pmd);
> addr = next;
> continue;
> }
IMHO, I'm not a big fan of adding two pmd_none() checks just to avoid an
uncommon concurrent sync. If you don't like the approach in v4 [1]
either, I'll follow your preferred coding style.
@@ -2102,12 +2105,11 @@ static inline unsigned long zap_pmd_range(struct
mmu_gather *tlb,
continue;
}
/* fall through */
- } else if (details && details->single_folio &&
- folio_test_pmd_mappable(details->single_folio) &&
- next - addr == HPAGE_PMD_SIZE && pmd_none(*pmd)) {
- sync_with_folio_pmd_zap(tlb->mm, pmd);
}
+
if (pmd_none(*pmd)) {
+ if (details && details->single_folio)
+ sync_with_folio_pmd_zap(tlb->mm, pmd);
addr = next;
continue;
}
[1]
https://lore.kernel.org/all/4cc521a118564b4145edf0324066ab532fb41181.1792979870.git.baolin.wang@linux.alibaba.com/#t
> Removing the "if (pmd_none(*pmd)) {" in your variant is what confused me,
> because it looks like we no longer happen the case where we zapped the PMD
> ourselves.
Because zap_pte_range() will return the same addr and check the none PMD
again in zap_pmd_range() in this case, I thought you wanted this simple
version, considering keeping two pmd_none() checks is incorrect.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5] mm: memory: fix truncation removing mapped folios
2026-10-27 10:35 ` Baolin Wang
@ 2026-10-27 10:55 ` David Hildenbrand (Arm)
2026-10-27 12:50 ` Baolin Wang
0 siblings, 1 reply; 9+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-27 10:55 UTC (permalink / raw)
To: Baolin Wang, akpm
Cc: ljs, liam, vbabka, rppt, surenb, mhocko, qi.zheng, jack,
pfalcato, ayushr, linux-mm, linux-kernel
On 10/27/26 11:35, Baolin Wang wrote:
>
>
> On 10/27/26 5:33 PM, David Hildenbrand (Arm) wrote:
>> On 10/27/26 10:03, Baolin Wang wrote:
>>>
>>>
>>>
>>> No, keeping two pmd_none() checks cannot fix the race, as I replied before. [1]
>>>
>>
>> Best to wait to submit new versions if there are open discussions? Because now
>> we have the same discussion in two places.
>
> Sure.
>
>>
>> I think you're right about the race, the issue is that we check for
>> pmd_is_huge(), not pmd_present() which confused me.
>>
>> Clearest to me would be to not drop the second pmd_none() check but move it
>> where it belongs: if splitting zapped the PMD.
>>
>> @@ -2101,13 +2104,15 @@ static inline unsigned long zap_pmd_range(struct
>> mmu_gather *tlb,
>> addr = next;
>> continue;
>> }
>> + /* We already synchronized with concurrent zapping. */
>> + if (pmd_none(*pmd)) {
>> + addr = next;
>> + continue;
>> + }
>> /* fall through */
>> - } else if (details && details->single_folio &&
>> - folio_test_pmd_mappable(details->single_folio) &&
>> - next - addr == HPAGE_PMD_SIZE && pmd_none(*pmd)) {
>> - sync_with_folio_pmd_zap(tlb->mm, pmd);
>> - }
>> - if (pmd_none(*pmd)) {
>> + } else if (pmd_none(*pmd)) {
>> + if (details && details->single_folio)
>> + sync_with_folio_pmd_zap(tlb->mm, pmd);
>> addr = next;
>> continue;
>> }
>
> IMHO, I'm not a big fan of adding two pmd_none() checks just to avoid an
> uncommon concurrent sync.
It's what we already had. We had to: one for handling pmd_none() directly and
one for handling zapping while splitting (file THPs).
We just managed to let the second also handle the final piece of the former.
Anyhow, let's get this fixed.
If you think the code is clearer your way (where I disagree), then keep it as is.
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
--
Cheers,
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5] mm: memory: fix truncation removing mapped folios
2026-10-27 10:55 ` David Hildenbrand (Arm)
@ 2026-10-27 12:50 ` Baolin Wang
2026-10-27 12:53 ` David Hildenbrand (Arm)
0 siblings, 1 reply; 9+ messages in thread
From: Baolin Wang @ 2026-10-27 12:50 UTC (permalink / raw)
To: David Hildenbrand (Arm), akpm
Cc: ljs, liam, vbabka, rppt, surenb, mhocko, qi.zheng, jack,
pfalcato, ayushr, linux-mm, linux-kernel
On 10/27/26 6:55 PM, David Hildenbrand (Arm) wrote:
> On 10/27/26 11:35, Baolin Wang wrote:
>>
>>
>> On 10/27/26 5:33 PM, David Hildenbrand (Arm) wrote:
>>> On 10/27/26 10:03, Baolin Wang wrote:
>>>>
>>>>
>>>>
>>>> No, keeping two pmd_none() checks cannot fix the race, as I replied before. [1]
>>>>
>>>
>>> Best to wait to submit new versions if there are open discussions? Because now
>>> we have the same discussion in two places.
>>
>> Sure.
>>
>>>
>>> I think you're right about the race, the issue is that we check for
>>> pmd_is_huge(), not pmd_present() which confused me.
>>>
>>> Clearest to me would be to not drop the second pmd_none() check but move it
>>> where it belongs: if splitting zapped the PMD.
>>>
>>> @@ -2101,13 +2104,15 @@ static inline unsigned long zap_pmd_range(struct
>>> mmu_gather *tlb,
>>> addr = next;
>>> continue;
>>> }
>>> + /* We already synchronized with concurrent zapping. */
>>> + if (pmd_none(*pmd)) {
>>> + addr = next;
>>> + continue;
>>> + }
>>> /* fall through */
>>> - } else if (details && details->single_folio &&
>>> - folio_test_pmd_mappable(details->single_folio) &&
>>> - next - addr == HPAGE_PMD_SIZE && pmd_none(*pmd)) {
>>> - sync_with_folio_pmd_zap(tlb->mm, pmd);
>>> - }
>>> - if (pmd_none(*pmd)) {
>>> + } else if (pmd_none(*pmd)) {
>>> + if (details && details->single_folio)
>>> + sync_with_folio_pmd_zap(tlb->mm, pmd);
>>> addr = next;
>>> continue;
>>> }
>>
>> IMHO, I'm not a big fan of adding two pmd_none() checks just to avoid an
>> uncommon concurrent sync.
>
> It's what we already had. We had to: one for handling pmd_none() directly and
> one for handling zapping while splitting (file THPs).
>
> We just managed to let the second also handle the final piece of the former.
>
> Anyhow, let's get this fixed.
>
> If you think the code is clearer your way (where I disagree)
Fair enough. Though I have a different taste, I'd still like to make the
maintainer happy. I'll send out v6 tomorrow following your suggestion.
But could you help check the following whole diff to avoid any
unnecessary misunderstandings? (I don't want to give Andrew more trouble
:)). Thanks.
diff --git a/mm/internal.h b/mm/internal.h
index b1b0f29d2216..ffb8eaec90e3 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -584,14 +584,16 @@ void free_pgtables(struct mmu_gather *tlb, struct
unmap_desc *desc);
void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte);
/**
- * sync_with_folio_pmd_zap - sync with concurrent zapping of a folio PMD
+ * sync_with_folio_pmd_zap - sync with concurrent zapping of folios
* @mm: The mm_struct.
* @pmdp: Pointer to the pmd that was found to be pmd_none().
*
* When we find a pmd_none() while unmapping a folio without holding
the PTL,
- * zap_huge_pmd() may have cleared the PMD but not yet modified the
folio to
- * indicate that it's unmapped. Skipping the PMD without
synchronization could
- * make folio unmapping code assume that unmapping failed.
+ * zap_huge_pmd() may have cleared the PMD but not yet updated the folio
+ * mapcount, and zap_empty_pte_table() clears the PMD only after the rmaps
+ * for the table have been removed, but nothing orders the pmd_none() read
+ * against later mapcount reads. Skipping the PMD without synchronization
+ * could make folio unmapping code assume that unmapping failed.
*
* Wait for concurrent zapping to complete by grabbing the PTL.
*/
diff --git a/mm/memory.c b/mm/memory.c
index e911e63ba1ed..e0fef107a022 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2036,6 +2036,15 @@ static unsigned long zap_pte_range(struct
mmu_gather *tlb,
}
} while (pte += nr, addr += PAGE_SIZE * nr, addr != end);
+ add_mm_rss_vec(mm, rss);
+ lazy_mmu_mode_disable();
+
+ /* Do the actual TLB flush before dropping ptl */
+ if (force_flush) {
+ tlb_flush_mmu_tlbonly(tlb);
+ tlb_flush_rmaps(tlb, vma);
+ }
+
/*
* Fast path: try to hold the pmd lock and unmap the PTE page.
*
@@ -2043,18 +2052,12 @@ static unsigned long zap_pte_range(struct
mmu_gather *tlb,
* to hold the pmd lock failed, then we need to recheck all pte
entries
* to ensure they are still none, thereby preventing the pte
entries
* from being repopulated by another thread.
+ *
+ * Clear the PMD after the rmaps for the table have been
removed, in case
+ * an incorrect mapcount is observed after unmapping the folio.
*/
if (can_reclaim_pt && direct_reclaim && addr == end)
direct_reclaim = zap_empty_pte_table(mm, pmd, ptl,
&pmdval);
-
- add_mm_rss_vec(mm, rss);
- lazy_mmu_mode_disable();
-
- /* Do the actual TLB flush before dropping ptl */
- if (force_flush) {
- tlb_flush_mmu_tlbonly(tlb);
- tlb_flush_rmaps(tlb, vma);
- }
pte_unmap_unlock(start_pte, ptl);
/*
@@ -2101,13 +2104,15 @@ static inline unsigned long zap_pmd_range(struct
mmu_gather *tlb,
addr = next;
continue;
}
+ /* We already synchronized with concurrent
zapping. */
+ if (pmd_none(*pmd)) {
+ addr = next;
+ continue;
+ }
/* fall through */
- } else if (details && details->single_folio &&
- folio_test_pmd_mappable(details->single_folio) &&
- next - addr == HPAGE_PMD_SIZE &&
pmd_none(*pmd)) {
- sync_with_folio_pmd_zap(tlb->mm, pmd);
- }
- if (pmd_none(*pmd)) {
+ } else if (pmd_none(*pmd)) {
+ if (details && details->single_folio)
+ sync_with_folio_pmd_zap(tlb->mm, pmd);
addr = next;
continue;
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5] mm: memory: fix truncation removing mapped folios
2026-10-27 12:50 ` Baolin Wang
@ 2026-10-27 12:53 ` David Hildenbrand (Arm)
2026-10-27 13:04 ` Baolin Wang
0 siblings, 1 reply; 9+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-27 12:53 UTC (permalink / raw)
To: Baolin Wang, akpm
Cc: ljs, liam, vbabka, rppt, surenb, mhocko, qi.zheng, jack,
pfalcato, ayushr, linux-mm, linux-kernel
On 10/27/26 13:50, Baolin Wang wrote:
>
>
> On 10/27/26 6:55 PM, David Hildenbrand (Arm) wrote:
>> On 10/27/26 11:35, Baolin Wang wrote:
>>>
>>>
>>>
>>> Sure.
>>>
>>>
>>> IMHO, I'm not a big fan of adding two pmd_none() checks just to avoid an
>>> uncommon concurrent sync.
>>
>> It's what we already had. We had to: one for handling pmd_none() directly and
>> one for handling zapping while splitting (file THPs).
>>
>> We just managed to let the second also handle the final piece of the former.
>>
>> Anyhow, let's get this fixed.
>>
>> If you think the code is clearer your way (where I disagree)
>
> Fair enough. Though I have a different taste, I'd still like to make the
> maintainer happy.
Don't worry, it's hard to make me unhappy :)
(just a bit overloaded right now)
> I'll send out v6 tomorrow following your suggestion.
>
That looks good. Are you planning on following up with the memory barrier
optimization for smaller folios?
I hope below doesn't end up in some performance degradation.
--
Cheers,
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5] mm: memory: fix truncation removing mapped folios
2026-10-27 12:53 ` David Hildenbrand (Arm)
@ 2026-10-27 13:04 ` Baolin Wang
0 siblings, 0 replies; 9+ messages in thread
From: Baolin Wang @ 2026-10-27 13:04 UTC (permalink / raw)
To: David Hildenbrand (Arm), akpm
Cc: ljs, liam, vbabka, rppt, surenb, mhocko, qi.zheng, jack,
pfalcato, ayushr, linux-mm, linux-kernel
On 10/27/26 8:53 PM, David Hildenbrand (Arm) wrote:
> On 10/27/26 13:50, Baolin Wang wrote:
>>
>>
>> On 10/27/26 6:55 PM, David Hildenbrand (Arm) wrote:
>>> On 10/27/26 11:35, Baolin Wang wrote:
>>>>
>>>>
>>>>
>>>> Sure.
>>>>
>>>>
>>>> IMHO, I'm not a big fan of adding two pmd_none() checks just to avoid an
>>>> uncommon concurrent sync.
>>>
>>> It's what we already had. We had to: one for handling pmd_none() directly and
>>> one for handling zapping while splitting (file THPs).
>>>
>>> We just managed to let the second also handle the final piece of the former.
>>>
>>> Anyhow, let's get this fixed.
>>>
>>> If you think the code is clearer your way (where I disagree)
>>
>> Fair enough. Though I have a different taste, I'd still like to make the
>> maintainer happy.
>
> Don't worry, it's hard to make me unhappy :)
>
> (just a bit overloaded right now)
>
>> I'll send out v6 tomorrow following your suggestion.
>>
>
> That looks good.
Thanks.
> Are you planning on following up with the memory barrier
> optimization for smaller folios?
If I can find some time, though I have quite a few pending TODOs now :).
I don't think this is urgent, so if no one else picks it up, I'll add it
to my TODO list.
> I hope below doesn't end up in some performance degradation.
This is not a common case, so I think it's fine.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-27 13:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-27 2:19 [PATCH v5] mm: memory: fix truncation removing mapped folios Baolin Wang
2026-10-27 9:00 ` David Hildenbrand (Arm)
2026-10-27 9:03 ` Baolin Wang
2026-10-27 9:33 ` David Hildenbrand (Arm)
2026-10-27 10:35 ` Baolin Wang
2026-10-27 10:55 ` David Hildenbrand (Arm)
2026-10-27 12:50 ` Baolin Wang
2026-10-27 12:53 ` David Hildenbrand (Arm)
2026-10-27 13:04 ` Baolin Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®