mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mm: memory: fix truncation removing mapped folios
@ 2026-10-09 10:12 Baolin Wang
  2026-10-09 12:00 ` Lorenzo Stoakes (ARM)
  0 siblings, 1 reply; 5+ messages in thread
From: Baolin Wang @ 2026-10-09 10:12 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 a smp_wmb() memory barrier, 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>
---
 mm/memory.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 1f5d5f7d39cd..0272217ad7b0 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,10 @@ 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) {
+		/* rmap changes need to be observed before e.g PTEs get zapped. */
+		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);
 
-- 
2.47.3


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

* Re: [PATCH] mm: memory: fix truncation removing mapped folios
  2026-10-09 10:12 [PATCH] mm: memory: fix truncation removing mapped folios Baolin Wang
@ 2026-10-09 12:00 ` Lorenzo Stoakes (ARM)
  2026-10-09 13:36   ` Pedro Falcato
  0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-10-09 12:00 UTC (permalink / raw)
  To: Baolin Wang
  Cc: akpm, david, liam, vbabka, rppt, surenb, mhocko, qi.zheng, jack,
	pfalcato, ayushr, linux-mm, linux-kernel

On Fri, Oct 09, 2026 at 06:12:03PM +0800, Baolin Wang wrote:
> diff --git a/mm/memory.c b/mm/memory.c
> index 1f5d5f7d39cd..0272217ad7b0 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c

> @@ -2044,16 +2053,10 @@ 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) {
> +		/* rmap changes need to be observed before e.g PTEs get zapped. */
> +		smp_wmb();

What does this pair with? You should always say what in the comment.

rmb/wmb are generally not needed vs. acquire/release. Are you sure a wmb is
right here?

--
Cheers, Lorenzo

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

* Re: [PATCH] mm: memory: fix truncation removing mapped folios
  2026-10-09 12:00 ` Lorenzo Stoakes (ARM)
@ 2026-10-09 13:36   ` Pedro Falcato
  2026-10-09 13:53     ` Baolin Wang
  0 siblings, 1 reply; 5+ messages in thread
From: Pedro Falcato @ 2026-10-09 13:36 UTC (permalink / raw)
  To: Lorenzo Stoakes (ARM)
  Cc: Baolin Wang, akpm, david, liam, vbabka, rppt, surenb, mhocko,
	qi.zheng, jack, ayushr, linux-mm, linux-kernel

On Fri, Oct 09, 2026 at 01:00:52PM +0100, Lorenzo Stoakes (ARM) wrote:
> On Fri, Oct 09, 2026 at 06:12:03PM +0800, Baolin Wang wrote:
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 1f5d5f7d39cd..0272217ad7b0 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> 
> > @@ -2044,16 +2053,10 @@ 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) {
> > +		/* rmap changes need to be observed before e.g PTEs get zapped. */
> > +		smp_wmb();
> 
> What does this pair with? You should always say what in the comment.

Yep, this is missing a rmb to pair with. The diff I posted at
https://lore.kernel.org/linux-mm/asigKktcrSY80A_2@pedro-suse.tail5790ac.ts.net/
had one in zap_pmd_range(), which admittedly isn't the greatest. I'm not sure if
there's a better way to do this.

> 
> rmb/wmb are generally not needed vs. acquire/release. Are you sure a wmb is
> right here?

I don't think that works, because __folio_remove_rmap has a ton of stuff going
on, some implying full ordering, others not. Plus you'd need to switch the pmd
dereference to something like an smp_load_acquire().

-- 
Pedro

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

* Re: [PATCH] mm: memory: fix truncation removing mapped folios
  2026-10-09 13:36   ` Pedro Falcato
@ 2026-10-09 13:53     ` Baolin Wang
  2026-10-09 17:01       ` Pedro Falcato
  0 siblings, 1 reply; 5+ messages in thread
From: Baolin Wang @ 2026-10-09 13:53 UTC (permalink / raw)
  To: Pedro Falcato, Lorenzo Stoakes (ARM)
  Cc: akpm, david, liam, vbabka, rppt, surenb, mhocko, qi.zheng, jack,
	ayushr, linux-mm, linux-kernel



On 10/9/26 9:36 PM, Pedro Falcato wrote:
> On Fri, Oct 09, 2026 at 01:00:52PM +0100, Lorenzo Stoakes (ARM) wrote:
>> On Fri, Oct 09, 2026 at 06:12:03PM +0800, Baolin Wang wrote:
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index 1f5d5f7d39cd..0272217ad7b0 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>
>>> @@ -2044,16 +2053,10 @@ 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) {
>>> +		/* rmap changes need to be observed before e.g PTEs get zapped. */
>>> +		smp_wmb();
>>
>> What does this pair with? You should always say what in the comment.
> 
> Yep, this is missing a rmb to pair with. The diff I posted at
> https://lore.kernel.org/linux-mm/asigKktcrSY80A_2@pedro-suse.tail5790ac.ts.net/
> had one in zap_pmd_range(), which admittedly isn't the greatest. I'm not sure if
> there's a better way to do this.

I don't think we need an rmb to pair with. This is because the 
folio_mapped() check in __filemap_remove_folio() cannot be reordered 
(guarded by spinlock) with the pmd_none() check in zap_pmd_range().

We just need to guarantee that when we find a pmd_none(), the removal of 
the folio's mapping (via tlb_flush_rmaps()) is already observed.


>> rmb/wmb are generally not needed vs. acquire/release. Are you sure a wmb is
>> right here?
> 
> I don't think that works, because __folio_remove_rmap has a ton of stuff going
> on, some implying full ordering, others not. Plus you'd need to switch the pmd
> dereference to something like an smp_load_acquire().

Agree.

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

* Re: [PATCH] mm: memory: fix truncation removing mapped folios
  2026-10-09 13:53     ` Baolin Wang
@ 2026-10-09 17:01       ` Pedro Falcato
  0 siblings, 0 replies; 5+ messages in thread
From: Pedro Falcato @ 2026-10-09 17:01 UTC (permalink / raw)
  To: Baolin Wang
  Cc: Lorenzo Stoakes (ARM),
	akpm, david, liam, vbabka, rppt, surenb, mhocko, qi.zheng, jack,
	ayushr, linux-mm, linux-kernel

On Fri, Oct 09, 2026 at 09:53:55PM +0800, Baolin Wang wrote:
> 
> 
> On 10/9/26 9:36 PM, Pedro Falcato wrote:
> > On Fri, Oct 09, 2026 at 01:00:52PM +0100, Lorenzo Stoakes (ARM) wrote:
> > > On Fri, Oct 09, 2026 at 06:12:03PM +0800, Baolin Wang wrote:
> > > > diff --git a/mm/memory.c b/mm/memory.c
> > > > index 1f5d5f7d39cd..0272217ad7b0 100644
> > > > --- a/mm/memory.c
> > > > +++ b/mm/memory.c
> > > 
> > > > @@ -2044,16 +2053,10 @@ 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) {
> > > > +		/* rmap changes need to be observed before e.g PTEs get zapped. */
> > > > +		smp_wmb();
> > > 
> > > What does this pair with? You should always say what in the comment.
> > 
> > Yep, this is missing a rmb to pair with. The diff I posted at
> > https://lore.kernel.org/linux-mm/asigKktcrSY80A_2@pedro-suse.tail5790ac.ts.net/
> > had one in zap_pmd_range(), which admittedly isn't the greatest. I'm not sure if
> > there's a better way to do this.
> 
> I don't think we need an rmb to pair with. This is because the
> folio_mapped() check in __filemap_remove_folio() cannot be reordered
> (guarded by spinlock) with the pmd_none() check in zap_pmd_range().

Why? spin_lock has ACQUIRE semantics, which are one-way permeable. Earlier
loads and stores do not necessarily happen-before anything after the ACQUIRE.
Unless you mean something else.

> 
> We just need to guarantee that when we find a pmd_none(), the removal of the
> folio's mapping (via tlb_flush_rmaps()) is already observed.
> 
> 
> > > rmb/wmb are generally not needed vs. acquire/release. Are you sure a wmb is
> > > right here?
> > 
> > I don't think that works, because __folio_remove_rmap has a ton of stuff going
> > on, some implying full ordering, others not. Plus you'd need to switch the pmd
> > dereference to something like an smp_load_acquire().
> 
> Agree.

-- 
Pedro

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

end of thread, other threads:[~2026-10-09 17:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-09 10:12 [PATCH] mm: memory: fix truncation removing mapped folios Baolin Wang
2026-10-09 12:00 ` Lorenzo Stoakes (ARM)
2026-10-09 13:36   ` Pedro Falcato
2026-10-09 13:53     ` Baolin Wang
2026-10-09 17:01       ` Pedro Falcato

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®