mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] mm: memory: fix truncation removing mapped folios
@ 2026-10-10  7:22 Baolin Wang
  2026-10-12 10:23 ` David Hildenbrand (Arm)
  2026-10-13 10:24 ` Pedro Falcato
  0 siblings, 2 replies; 9+ messages in thread
From: Baolin Wang @ 2026-10-10  7:22 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 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


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

* Re: [PATCH v2] mm: memory: fix truncation removing mapped folios
  2026-10-10  7:22 [PATCH v2] mm: memory: fix truncation removing mapped folios Baolin Wang
@ 2026-10-12 10:23 ` David Hildenbrand (Arm)
  2026-10-13  9:36   ` Baolin Wang
  2026-10-13 10:24 ` Pedro Falcato
  1 sibling, 1 reply; 9+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-12 10:23 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/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

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

* Re: [PATCH v2] mm: memory: fix truncation removing mapped folios
  2026-10-12 10:23 ` David Hildenbrand (Arm)
@ 2026-10-13  9:36   ` Baolin Wang
  2026-10-14 13:25     ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 9+ messages in thread
From: Baolin Wang @ 2026-10-13  9:36 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/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/

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

* Re: [PATCH v2] mm: memory: fix truncation removing mapped folios
  2026-10-10  7:22 [PATCH v2] mm: memory: fix truncation removing mapped folios Baolin Wang
  2026-10-12 10:23 ` David Hildenbrand (Arm)
@ 2026-10-13 10:24 ` Pedro Falcato
  2026-10-14  9:51   ` Baolin Wang
  1 sibling, 1 reply; 9+ messages in thread
From: Pedro Falcato @ 2026-10-13 10:24 UTC (permalink / raw)
  To: Baolin Wang
  Cc: akpm, david, ljs, liam, vbabka, rppt, surenb, mhocko, qi.zheng,
	jack, ayushr, linux-mm, linux-kernel

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

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

* Re: [PATCH v2] mm: memory: fix truncation removing mapped folios
  2026-10-13 10:24 ` Pedro Falcato
@ 2026-10-14  9:51   ` Baolin Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Baolin Wang @ 2026-10-14  9:51 UTC (permalink / raw)
  To: Pedro Falcato
  Cc: akpm, david, ljs, liam, vbabka, rppt, surenb, mhocko, qi.zheng,
	jack, ayushr, linux-mm, linux-kernel



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...

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

* Re: [PATCH v2] mm: memory: fix truncation removing mapped folios
  2026-10-13  9:36   ` Baolin Wang
@ 2026-10-14 13:25     ` David Hildenbrand (Arm)
  2026-10-14 14:05       ` Pedro Falcato
  2026-10-15  6:41       ` Baolin Wang
  0 siblings, 2 replies; 9+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-14 13:25 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/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

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

* Re: [PATCH v2] mm: memory: fix truncation removing mapped folios
  2026-10-14 13:25     ` David Hildenbrand (Arm)
@ 2026-10-14 14:05       ` Pedro Falcato
  2026-10-14 18:44         ` David Hildenbrand (Arm)
  2026-10-15  6:41       ` Baolin Wang
  1 sibling, 1 reply; 9+ messages in thread
From: Pedro Falcato @ 2026-10-14 14:05 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Baolin Wang, akpm, ljs, liam, vbabka, rppt, surenb, mhocko,
	qi.zheng, jack, ayushr, linux-mm, linux-kernel

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

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

* Re: [PATCH v2] mm: memory: fix truncation removing mapped folios
  2026-10-14 14:05       ` Pedro Falcato
@ 2026-10-14 18:44         ` David Hildenbrand (Arm)
  0 siblings, 0 replies; 9+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-14 18:44 UTC (permalink / raw)
  To: Pedro Falcato
  Cc: Baolin Wang, akpm, ljs, liam, vbabka, rppt, surenb, mhocko,
	qi.zheng, jack, ayushr, linux-mm, linux-kernel

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

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

* Re: [PATCH v2] mm: memory: fix truncation removing mapped folios
  2026-10-14 13:25     ` David Hildenbrand (Arm)
  2026-10-14 14:05       ` Pedro Falcato
@ 2026-10-15  6:41       ` Baolin Wang
  1 sibling, 0 replies; 9+ messages in thread
From: Baolin Wang @ 2026-10-15  6:41 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/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;
                 }

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

end of thread, other threads:[~2026-10-15  6:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-10  7:22 [PATCH v2] mm: memory: fix truncation removing mapped folios Baolin Wang
2026-10-12 10:23 ` David Hildenbrand (Arm)
2026-10-13  9:36   ` Baolin Wang
2026-10-14 13:25     ` David Hildenbrand (Arm)
2026-10-14 14:05       ` Pedro Falcato
2026-10-14 18:44         ` David Hildenbrand (Arm)
2026-10-15  6:41       ` Baolin Wang
2026-10-13 10:24 ` Pedro Falcato
2026-10-14  9:51   ` 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®