[PATCH v2] mm: memory: fix truncation removing mapped folios

[PATCH v2] mm: memory: fix truncation removing mapped folios

[Baolin Wang]

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 add smp_wmb()/smp_rmb() memory barriers, 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)")
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
Changes from v1:
 - Add an smp_rmb() to pair with the smp_wmb() (Per Lorenzo and Pedro).
---
 mm/memory.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 6a8e7772b8d6..c3a1faf580c8 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.
 	 *
@@ -2044,16 +2053,13 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
 	 * to ensure they are still none, thereby preventing the pte entries
 	 * from being repopulated by another thread.
 	 */
-	if (can_reclaim_pt && direct_reclaim && addr == end)
+	if (can_reclaim_pt && direct_reclaim && addr == end) {
+		/*
+		 * The rmap changes need to be observed before PTEs get zapped.
+		 * Pairs with smp_rmb() in zap_pmd_range().
+		 */
+		smp_wmb();
 		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);
 
@@ -2108,6 +2114,11 @@ static inline unsigned long zap_pmd_range(struct mmu_gather *tlb,
 			sync_with_folio_pmd_zap(tlb->mm, pmd);
 		}
 		if (pmd_none(*pmd)) {
+			/*
+			 * Order this PMD read before the folio's mapcount read.
+			 * Pairs with smp_wmb() in zap_pte_range().
+			 */
+			smp_rmb();
 			addr = next;
 			continue;
 		}
-- 
2.47.3

Re: [PATCH v2] mm: memory: fix truncation removing mapped folios

[David Hildenbrand (Arm)]

On 10/10/26 09:22, 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)

That reminds me of sync_with_folio_pmd_zap() usage in page_vma_mapped_walk().

Isn't that the better approach?

-- 
Cheers,

David

Re: [PATCH v2] mm: memory: fix truncation removing mapped folios

[Baolin Wang]

On 10/12/26 6:23 PM, David Hildenbrand (Arm) wrote:
> On 10/10/26 09:22, 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)
> 
> That reminds me of sync_with_folio_pmd_zap() usage in page_vma_mapped_walk().
> 
> Isn't that the better approach?

I had considered this approach before, but it is not feasible. IIUC, the 
sync_with_folio_pmd_zap() is used to address the race with 
zap_huge_pmd(), and both the PMD clear and the folio mapcount decrement 
are performed under the PMD lock in zap_huge_pmd().

However, the race here is different: tlb_flush_rmaps() is under the PTE 
lock, while zap_empty_pte_table() is under the PMD lock, and reordering 
between the two functions is also possible. That is, the folio mapcount 
operation could execute after the acquire barrier (spin_lock()) of 
zap_empty_pte_table(), allowing a reader on CPU 0 to incorrectly observe 
that the PMD is none while mapcount > 0.

Also, we need to consider the possible reordering on the reader side on 
CPU 0, as discussed earlier [1].

[1] 
https://lore.kernel.org/all/1b77c2d7-acc6-476a-b5cd-547471a46409@linux.alibaba.com/

Re: [PATCH v2] mm: memory: fix truncation removing mapped folios

[Pedro Falcato]

On Sat, Oct 10, 2026 at 03:22:18PM +0800, 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 add smp_wmb()/smp_rmb() memory barriers, 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)")
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>

Reviewed-by: Pedro Falcato <pfalcato@suse.de>

Mostly LGTM, thanks!

> ---
> Changes from v1:
>  - Add an smp_rmb() to pair with the smp_wmb() (Per Lorenzo and Pedro).
> ---
>  mm/memory.c | 29 ++++++++++++++++++++---------
>  1 file changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 6a8e7772b8d6..c3a1faf580c8 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.
>  	 *
> @@ -2044,16 +2053,13 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
>  	 * to ensure they are still none, thereby preventing the pte entries
>  	 * from being repopulated by another thread.
>  	 */
> -	if (can_reclaim_pt && direct_reclaim && addr == end)
> +	if (can_reclaim_pt && direct_reclaim && addr == end) {
> +		/*
> +		 * The rmap changes need to be observed before PTEs get zapped.
> +		 * Pairs with smp_rmb() in zap_pmd_range().
> +		 */

I'd amend this comment to something like:
		/*
		 * rmap changes need to be observed before page tables get zapped.
		 * Pairs with smp_rmb() in zap_pmd_range().
		 */
> +		smp_wmb();
>  		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);
>  
> @@ -2108,6 +2114,11 @@ static inline unsigned long zap_pmd_range(struct mmu_gather *tlb,
>  			sync_with_folio_pmd_zap(tlb->mm, pmd);
>  		}
>  		if (pmd_none(*pmd)) {
> +			/*
> +			 * Order this PMD read before the folio's mapcount read.
> +			 * Pairs with smp_wmb() in zap_pte_range().
> +			 */

and this one to something like:
			/*
			 * Order this PMD read before any (possible) mapcount/rmap read.
			 * This isn't a problem in most cases, due to us synchronizing on
			 * the PTE lock, but such doesn't happen when PTE tables are zapped.
			 * Pairs with smp_wmb() in zap_pte_range().
			 */
> +			smp_rmb();
>  			addr = next;
>  			continue;
>  		}

Also, as a note, I would suggest we hold off on this fix until the merge window.
I don't think it's that urgent, and it seems like there is a possibility of a
nicer, cleaner fix. That and we need to audit pmd_none() sites for other
possible smp_rmb()'s...

-- 
Pedro

Re: [PATCH v2] mm: memory: fix truncation removing mapped folios

[Baolin Wang]

On 10/13/26 6:24 PM, Pedro Falcato wrote:
> On Sat, Oct 10, 2026 at 03:22:18PM +0800, 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 add smp_wmb()/smp_rmb() memory barriers, 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)")
>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> 
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>

Thanks.

> Mostly LGTM, thanks!
> 
>> ---
>> Changes from v1:
>>   - Add an smp_rmb() to pair with the smp_wmb() (Per Lorenzo and Pedro).
>> ---
>>   mm/memory.c | 29 ++++++++++++++++++++---------
>>   1 file changed, 20 insertions(+), 9 deletions(-)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 6a8e7772b8d6..c3a1faf580c8 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.
>>   	 *
>> @@ -2044,16 +2053,13 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
>>   	 * to ensure they are still none, thereby preventing the pte entries
>>   	 * from being repopulated by another thread.
>>   	 */
>> -	if (can_reclaim_pt && direct_reclaim && addr == end)
>> +	if (can_reclaim_pt && direct_reclaim && addr == end) {
>> +		/*
>> +		 * The rmap changes need to be observed before PTEs get zapped.
>> +		 * Pairs with smp_rmb() in zap_pmd_range().
>> +		 */
> 
> I'd amend this comment to something like:
> 		/*
> 		 * rmap changes need to be observed before page tables get zapped.
> 		 * Pairs with smp_rmb() in zap_pmd_range().
> 		 */

Ack.

>> +		smp_wmb();
>>   		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);
>>   
>> @@ -2108,6 +2114,11 @@ static inline unsigned long zap_pmd_range(struct mmu_gather *tlb,
>>   			sync_with_folio_pmd_zap(tlb->mm, pmd);
>>   		}
>>   		if (pmd_none(*pmd)) {
>> +			/*
>> +			 * Order this PMD read before the folio's mapcount read.
>> +			 * Pairs with smp_wmb() in zap_pte_range().
>> +			 */
> 
> and this one to something like:
> 			/*
> 			 * Order this PMD read before any (possible) mapcount/rmap read.
> 			 * This isn't a problem in most cases, due to us synchronizing on
> 			 * the PTE lock, but such doesn't happen when PTE tables are zapped.
> 			 * Pairs with smp_wmb() in zap_pte_range().
> 			 */

Ack.

>> +			smp_rmb();
>>   			addr = next;
>>   			continue;
>>   		}
> 
> Also, as a note, I would suggest we hold off on this fix until the merge window.

No strong opinion.

> I don't think it's that urgent, and it seems like there is a possibility of a
> nicer, cleaner fix. That and we need to audit pmd_none() sites for other
> possible smp_rmb()'s...

Re: [PATCH v2] mm: memory: fix truncation removing mapped folios

[David Hildenbrand (Arm)]

On 10/13/26 11:36, Baolin Wang wrote:
> 
> 
> On 10/12/26 6:23 PM, David Hildenbrand (Arm) wrote:
>> On 10/10/26 09:22, 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)
>>
>> That reminds me of sync_with_folio_pmd_zap() usage in page_vma_mapped_walk().
>>
>> Isn't that the better approach?
> 
> I had considered this approach before, but it is not feasible. IIUC, the
> sync_with_folio_pmd_zap() is used to address the race with zap_huge_pmd(), and
> both the PMD clear and the folio mapcount decrement are performed under the PMD
> lock in zap_huge_pmd().

Just to be clear, I am thinking about something like the following on the
zap_pmd_range() side:

diff --git a/mm/internal.h b/mm/internal.h
index 38b1165212c94..a8820ad7e2f21 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -584,14 +584,15 @@ 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() or zap_empty_pte_table() may have cleared the PMD but not
+ * yet modified the mapped folios to indicate they are unmapped. 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 8b0c2c735d3de..c585f5eb255c9 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2139,9 +2139,7 @@ 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)) {
+               } else if (details && details->single_folio && pmd_none(*pmd)) {
                        sync_with_folio_pmd_zap(tlb->mm, pmd);
                }
                if (pmd_none(*pmd)) {

> 
> However, the race here is different: tlb_flush_rmaps() is under the PTE lock,
> while zap_empty_pte_table() is under the PMD lock, and reordering between the
> two functions is also possible. That is, the folio mapcount operation could
> execute after the acquire barrier (spin_lock()) of zap_empty_pte_table(),
> allowing a reader on CPU 0 to incorrectly observe that the PMD is none while
> mapcount > 0.

I think we really just have to make sure to flush rmaps before zapping the PMD.
Then it's really just the same thing we had before with sync_with_folio_pmd_zap().

So I agree about doing the tlb_flush_rmaps() before zapping the PMD. But on the
zap_pmd_range() front that's really just what sync_with_folio_pmd_zap() wants to
handle.

-- 
Cheers,

David

Re: [PATCH v2] mm: memory: fix truncation removing mapped folios

[Pedro Falcato]

On Wed, Oct 14, 2026 at 03:25:33PM +0200, David Hildenbrand (Arm) wrote:
> On 10/13/26 11:36, Baolin Wang wrote:
> > 
> > 
> > On 10/12/26 6:23 PM, David Hildenbrand (Arm) wrote:
> >> On 10/10/26 09:22, 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)
> >>
> >> That reminds me of sync_with_folio_pmd_zap() usage in page_vma_mapped_walk().
> >>
> >> Isn't that the better approach?
> > 
> > I had considered this approach before, but it is not feasible. IIUC, the
> > sync_with_folio_pmd_zap() is used to address the race with zap_huge_pmd(), and
> > both the PMD clear and the folio mapcount decrement are performed under the PMD
> > lock in zap_huge_pmd().
> 
> Just to be clear, I am thinking about something like the following on the
> zap_pmd_range() side:
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 38b1165212c94..a8820ad7e2f21 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -584,14 +584,15 @@ 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() or zap_empty_pte_table() may have cleared the PMD but not
> + * yet modified the mapped folios to indicate they are unmapped. 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 8b0c2c735d3de..c585f5eb255c9 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2139,9 +2139,7 @@ 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)) {
> +               } else if (details && details->single_folio && pmd_none(*pmd)) {
>                         sync_with_folio_pmd_zap(tlb->mm, pmd);
>                 }
>                 if (pmd_none(*pmd)) {
> 
> > 
> > However, the race here is different: tlb_flush_rmaps() is under the PTE lock,
> > while zap_empty_pte_table() is under the PMD lock, and reordering between the
> > two functions is also possible. That is, the folio mapcount operation could
> > execute after the acquire barrier (spin_lock()) of zap_empty_pte_table(),
> > allowing a reader on CPU 0 to incorrectly observe that the PMD is none while
> > mapcount > 0.
> 
> I think we really just have to make sure to flush rmaps before zapping the PMD.
> Then it's really just the same thing we had before with sync_with_folio_pmd_zap().

Hmm, could we viably replace sync_with_folio_pmd_zap() with the memory barriers
instead? Seems lighter-weight and functionally equivalent?

-- 
Pedro

Re: [PATCH v2] mm: memory: fix truncation removing mapped folios

[David Hildenbrand (Arm)]

On 10/14/26 16:05, Pedro Falcato wrote:
> On Wed, Oct 14, 2026 at 03:25:33PM +0200, David Hildenbrand (Arm) wrote:
>> On 10/13/26 11:36, Baolin Wang wrote:
>>>
>>>
>>>
>>> I had considered this approach before, but it is not feasible. IIUC, the
>>> sync_with_folio_pmd_zap() is used to address the race with zap_huge_pmd(), and
>>> both the PMD clear and the folio mapcount decrement are performed under the PMD
>>> lock in zap_huge_pmd().
>>
>> Just to be clear, I am thinking about something like the following on the
>> zap_pmd_range() side:
>>
>> diff --git a/mm/internal.h b/mm/internal.h
>> index 38b1165212c94..a8820ad7e2f21 100644
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -584,14 +584,15 @@ 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() or zap_empty_pte_table() may have cleared the PMD but not
>> + * yet modified the mapped folios to indicate they are unmapped. 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 8b0c2c735d3de..c585f5eb255c9 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -2139,9 +2139,7 @@ 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)) {
>> +               } else if (details && details->single_folio && pmd_none(*pmd)) {
>>                         sync_with_folio_pmd_zap(tlb->mm, pmd);
>>                 }
>>                 if (pmd_none(*pmd)) {
>>
>>>
>>> However, the race here is different: tlb_flush_rmaps() is under the PTE lock,
>>> while zap_empty_pte_table() is under the PMD lock, and reordering between the
>>> two functions is also possible. That is, the folio mapcount operation could
>>> execute after the acquire barrier (spin_lock()) of zap_empty_pte_table(),
>>> allowing a reader on CPU 0 to incorrectly observe that the PMD is none while
>>> mapcount > 0.
>>
>> I think we really just have to make sure to flush rmaps before zapping the PMD.
>> Then it's really just the same thing we had before with sync_with_folio_pmd_zap().
> 
> Hmm, could we viably replace sync_with_folio_pmd_zap() with the memory barriers
> instead? Seems lighter-weight and functionally equivalent?

For the ordinary PMD zap case it's really:

1) Clear PMD

2) Drop refcount+mapcount

So I don't think it's a direct replacement. If we had a proper way to invalidate
+ adjust + clear it could work. Like

1) Invalidate

2) Drop refcount+mapcount

3) Clear


But in general I think:

a) Likely all places where we have existing sync_with_folio_pmd_zap() have this
   problem (2).

b) I don't want us to throw around memory barriers that way. This all belongs
   into a helper (like sync_with_folio_pmd_zap()) which we only call when
   required (e.g., details && details->single_folio) and that properly documents
   all the nasty details.

c) Maybe there is some smart way to decide whether we need a memory barrier or a
   proper lock for now inside sync_with_folio_pmd_zap(). Based on the folio size
   should do (PMD vs. < PMD). But I assume grabbing the PMD lock briefly is not
   really a concern: if there would be something mapped we would have to lock
   it, and if it would be a PTE table we would lock that instead.

[insert rant about our PMD table locking]

-- 
Cheers,

David

Re: [PATCH v2] mm: memory: fix truncation removing mapped folios

[Baolin Wang]

On 10/14/26 9:25 PM, David Hildenbrand (Arm) wrote:
> On 10/13/26 11:36, Baolin Wang wrote:
>>
>>
>> On 10/12/26 6:23 PM, David Hildenbrand (Arm) wrote:
>>> On 10/10/26 09:22, 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)
>>>
>>> That reminds me of sync_with_folio_pmd_zap() usage in page_vma_mapped_walk().
>>>
>>> Isn't that the better approach?
>>
>> I had considered this approach before, but it is not feasible. IIUC, the
>> sync_with_folio_pmd_zap() is used to address the race with zap_huge_pmd(), and
>> both the PMD clear and the folio mapcount decrement are performed under the PMD
>> lock in zap_huge_pmd().
> 
> Just to be clear, I am thinking about something like the following on the
> zap_pmd_range() side:
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 38b1165212c94..a8820ad7e2f21 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -584,14 +584,15 @@ 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() or zap_empty_pte_table() may have cleared the PMD but not
> + * yet modified the mapped folios to indicate they are unmapped. 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 8b0c2c735d3de..c585f5eb255c9 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2139,9 +2139,7 @@ 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)) {
> +               } else if (details && details->single_folio && pmd_none(*pmd)) {
>                          sync_with_folio_pmd_zap(tlb->mm, pmd);
>                  }
>                  if (pmd_none(*pmd)) {
> 

Yes, I've considered that. However, implicit memory barriers give me a 
headache, so I ended up going with explicit memory barriers (suggested 
by Pedro).

If you don't like memory barriers, let me re-check whether the implicit 
memory barriers here can ensure race safety with 
sync_with_folio_pmd_zap(). A simplified logic is as follows:

CPU 0 (truncation)                        CPU 1 (zapping)
down_read() ->Acquire A1.                pte spin_lock() ->Acquire A4
                                          clear mapcount

                                          pmd spin_lock() ->Acquire A5
                                          clear pmd
                                          pmd spin_unlock() ->Release R5

if (pmd_none()) { //read pmd
                                          pte spin_unlock() ->Release R4
     pmd spin_lock() ->Acquire A2
     pmd spin_unlock() ->Release R2
}
up_read() -> Release R1

spin_lock(y) ->Acquire A3
folio_mapped() // read mapcount
spin_unlock(y) ->Release R3

CPU 0 constraints:
1. R2 is a release, so "read pmd" completes before R2.
2. A2 completes before A3 (ACQUIRE vs ACQUIRE).
3. "read mapcount" completes after A3, hence after A2.

CPU 1 constraints:
1. R5 is a release. Both "clear mapcount" and "clear pmd" complete 
before R5.

According to above constraints, if CPU0 sees pmd_none() caused by CPU1's 
clear pmd, and CPU0 then acquires the same pmd lock released by CPU1 at 
R5, then CPU1's clear mapcount happens before CPU0's later folio_mapped().

Therefore folio_mapped() is guaranteed not to read the old mapcount in 
this case. So I think sync_with_folio_pmd_zap() can work, but needs a 
small change to avoid the double pmd_none() check.

David, does the following diff look good to you?

diff --git a/mm/memory.c b/mm/memory.c
index 6a8e7772b8d6..c0aadab1b59e 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.
          *
@@ -2044,16 +2053,8 @@ static unsigned long zap_pte_range(struct 
mmu_gather *tlb,
          * to ensure they are still none, thereby preventing the pte 
entries
          * from being repopulated by another thread.
          */
-       if (can_reclaim_pt && direct_reclaim && addr == end)
+       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 +2103,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;
                 }

Re: [PATCH v2] mm: memory: fix truncation removing mapped folios

[David Hildenbrand (Arm)]

On 10/15/26 08:41, Baolin Wang wrote:
> 
> 
> On 10/14/26 9:25 PM, David Hildenbrand (Arm) wrote:
>> On 10/13/26 11:36, Baolin Wang wrote:
>>>
>>>
>>>
>>> I had considered this approach before, but it is not feasible. IIUC, the
>>> sync_with_folio_pmd_zap() is used to address the race with zap_huge_pmd(), and
>>> both the PMD clear and the folio mapcount decrement are performed under the PMD
>>> lock in zap_huge_pmd().
>>
>> Just to be clear, I am thinking about something like the following on the
>> zap_pmd_range() side:
>>
>> diff --git a/mm/internal.h b/mm/internal.h
>> index 38b1165212c94..a8820ad7e2f21 100644
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -584,14 +584,15 @@ 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() or zap_empty_pte_table() may have cleared the PMD but not
>> + * yet modified the mapped folios to indicate they are unmapped. 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 8b0c2c735d3de..c585f5eb255c9 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -2139,9 +2139,7 @@ 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)) {
>> +               } else if (details && details->single_folio && pmd_none(*pmd)) {
>>                          sync_with_folio_pmd_zap(tlb->mm, pmd);
>>                  }
>>                  if (pmd_none(*pmd)) {
>>
> 
> Yes, I've considered that. However, implicit memory barriers give me a headache,
> so I ended up going with explicit memory barriers (suggested by Pedro).
> 
> If you don't like memory barriers, let me re-check whether the implicit memory
> barriers here can ensure race safety with sync_with_folio_pmd_zap(). A
> simplified logic is as follows:
> 
> CPU 0 (truncation)                        CPU 1 (zapping)
> down_read() ->Acquire A1.                pte spin_lock() ->Acquire A4
>                                          clear mapcount
> 
>                                          pmd spin_lock() ->Acquire A5
>                                          clear pmd
>                                          pmd spin_unlock() ->Release R5
> 
> if (pmd_none()) { //read pmd
>                                          pte spin_unlock() ->Release R4
>     pmd spin_lock() ->Acquire A2
>     pmd spin_unlock() ->Release R2
> }
> up_read() -> Release R1
> 
> spin_lock(y) ->Acquire A3
> folio_mapped() // read mapcount
> spin_unlock(y) ->Release R3
> 
> CPU 0 constraints:
> 1. R2 is a release, so "read pmd" completes before R2.
> 2. A2 completes before A3 (ACQUIRE vs ACQUIRE).
> 3. "read mapcount" completes after A3, hence after A2.
> 
> CPU 1 constraints:
> 1. R5 is a release. Both "clear mapcount" and "clear pmd" complete before R5.
> 
> According to above constraints, if CPU0 sees pmd_none() caused by CPU1's clear
> pmd, and CPU0 then acquires the same pmd lock released by CPU1 at R5, then
> CPU1's clear mapcount happens before CPU0's later folio_mapped().
> 
> Therefore folio_mapped() is guaranteed not to read the old mapcount in this
> case. So I think sync_with_folio_pmd_zap() can work, but needs a small change to
> avoid the double pmd_none() check.


I'd be okay with some (follow-up?) memory barrier handling inside
sync_with_folio_pmd_zap(), for example, based on the folio size and with proper
documentation why we need the PT lock in one case and why the memory barrier is
ok in the other case.

> 
> David, does the following diff look good to you?

I think we should extend the sync_with_folio_pmd_zap() doc as suggested.

> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 6a8e7772b8d6..c0aadab1b59e 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.
>          *
> @@ -2044,16 +2053,8 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
>          * to ensure they are still none, thereby preventing the pte entries
>          * from being repopulated by another thread.
>          */
> -       if (can_reclaim_pt && direct_reclaim && addr == end)
> +       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 +2103,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;
>                 }

We do have another sync_with_folio_pmd_zap() user in page_vma_mapped_walk(),
does that one need care as well?

Not sure if removing the thp_vma_suitable_order + pvmw->nr_pages negatively
affect performance, I suspect not but we better double check.

-- 
Cheers,

David