Re: [PATCH 20/25] s390/fbatch: no lru_add_drain_all() in s390_wiggle_split_folio()

[Hugh Dickins]

On Wed, 26 Aug 2026, Claudio Imbrenda wrote:
> On Mon, 24 Aug 2026 07:39:12 -0700 (PDT)
> Hugh Dickins <hughd@xxxxxxxxxx> wrote:
> 
> > s390_wiggle_split_folio() has no good reason to lru_add_drain_all(),
> > now that the per-cpu fbatch references are gone.
> > 
> > Signed-off-by: Hugh Dickins <hughd@xxxxxxxxxx>
> > ---
> >  arch/s390/kernel/uv.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> > index dc14ebc0105b..120a467026a5 100644
> > --- a/arch/s390/kernel/uv.c
> > +++ b/arch/s390/kernel/uv.c
> > @@ -364,7 +364,6 @@ int s390_wiggle_split_folio(struct mm_struct *mm, struct folio *folio)
> >  
> >  	lockdep_assert_not_held(&mm->mmap_lock);
> >  	folio_wait_writeback(folio);
> > -	lru_add_drain_all();
> >  
> >  	if (!folio_test_large(folio))
> >  		return 0;
> 
> This is black magic for me, I am not sure I fully understand all the
> details, but what's the new purpose of lru_add_drain_all() ?
> 
> will we have a guarantee that no stray references to mapped folios will
> ever remain?
> 
> Any unexpected reference (i.e. not due to mappings, see
> expected_folio_refs()) will cause a protected guest to hang.

I most certanly don't know s390 or that code well enough to guarantee
you that no stray references to mapped folios can remain there. What I
can guarantee is that no references, of the kind which lru_add_drain_all()
used to be needed to remove, can exist there: so there will no longer
be any point in s390 (or others) calling it for that reason, to help
split_folio() to succeed.

You wonder then, what lru_add_drain_all()'s new purpose is, why it
still exists at all? I did hope to remove it completely, but found
two usages that I could not argue against: one is in user-forced page
reclaim (two memcg interfaces and a sysfs interface), where it's
still desirable to push folios on to the immediately reclaimable LRUs,
rather than leave any on the per-cpu fbatches preceding those LRUs;
the other is in memory hotremove, where it will be necessary to erase
stray addresses, through which a subsequent folio_try_get() might have
accessed a struct folio which (I imagine) might have been freed.

Yes, your split_folio() may still occasionally fail, because of
transient references and folio_try_get()s on that folio; but that's
so before and after the changes. And there is (in my mind anyway) an
open question of whether "folio_try_get() blips" will be visible a
little more than before.

Hmm, looking again at s390_wiggle_split_folio(), it seems rather
odd that it was doing an lru_add_drain_all() at all: because any
large (hence splittable) folios have themselves been immediately
flushed from the per-cpu fbatches, not left queued up there. Maybe
there was an earlier time when mm did not enforce that; and Barry
is currently looking to relax that, so the limitation intended for
pmd-sized folios is no longer forced on the smallest large folios.

If Barry's relaxation goes in before my drainage changes, then
there is value in that s390 lru_add_drain_all() in the interim.

Hugh