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

[Pedro Falcato <pfalcato@suse.de>]

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