Re: [PATCH v3] mm: annotate data-race in cpu_needs_drain()

[Andrew Morton]

On Mon, 29 Jun 2026 11:23:26 +0100 Lorenzo Stoakes <ljs@xxxxxxxxxx> wrote:

> > >  		folio_batch_count(&fbatches->lru_lazyfree) ||
> > >  		folio_batch_count(&fbatches->lru_activate) ||
> > > -		need_mlock_drain(cpu) ||
> > > +		need_mlock_drain(cpu)) ||
> >
> > The indentation is a bit suboptimal now.
> >
> > Would read nicer as
> >
> > diff --git a/mm/swap.c b/mm/swap.c
> > index 588f50d8f1a8c..5958e6fdd3593 100644
> > --- a/mm/swap.c
> > +++ b/mm/swap.c
> > @@ -828,13 +828,13 @@ static bool cpu_needs_drain(unsigned int cpu)
> >         struct cpu_fbatches *fbatches = &per_cpu(cpu_fbatches, cpu);
> >
> >         /* Check these in order of likelihood that they're not zero */
> > -       return folio_batch_count(&fbatches->lru_add) ||
> > -               folio_batch_count(&fbatches->lru_move_tail) ||
> > -               folio_batch_count(&fbatches->lru_deactivate_file) ||
> > -               folio_batch_count(&fbatches->lru_deactivate) ||
> > -               folio_batch_count(&fbatches->lru_lazyfree) ||
> > -               folio_batch_count(&fbatches->lru_activate) ||
> > -               need_mlock_drain(cpu) ||
> > +       return data_race(folio_batch_count(&fbatches->lru_add) ||
> > +                        folio_batch_count(&fbatches->lru_move_tail) ||
> > +                        folio_batch_count(&fbatches->lru_deactivate_file) ||
> > +                        folio_batch_count(&fbatches->lru_deactivate) ||
> > +                        folio_batch_count(&fbatches->lru_lazyfree) ||
> > +                        folio_batch_count(&fbatches->lru_activate) ||
> > +                        need_mlock_drain(cpu)) ||
> >                 has_bh_in_lru(cpu, NULL);
> 
> Yeah that works for me.
> 
> Andrew - maybe easier if you fix that up? :)
> 

Sure.

--- a/mm/swap.c~mm-annotate-data-race-in-cpu_needs_drain-fix
+++ a/mm/swap.c
@@ -832,12 +832,12 @@ static bool cpu_needs_drain(unsigned int
 
 	/* Check these in order of likelihood that they're not zero */
 	return data_race(folio_batch_count(&fbatches->lru_add) ||
-		folio_batch_count(&fbatches->lru_move_tail) ||
-		folio_batch_count(&fbatches->lru_deactivate_file) ||
-		folio_batch_count(&fbatches->lru_deactivate) ||
-		folio_batch_count(&fbatches->lru_lazyfree) ||
-		folio_batch_count(&fbatches->lru_activate) ||
-		need_mlock_drain(cpu)) ||
+			 folio_batch_count(&fbatches->lru_move_tail) ||
+			 folio_batch_count(&fbatches->lru_deactivate_file) ||
+			 folio_batch_count(&fbatches->lru_deactivate) ||
+			 folio_batch_count(&fbatches->lru_lazyfree) ||
+			 folio_batch_count(&fbatches->lru_activate) ||
+			 need_mlock_drain(cpu)) ||
 		has_bh_in_lru(cpu, NULL);
 }
 

The removal of data_race() in need_mlock_drain() is a little worrisome
- perhaps any future callers would have needed it?

need_mlock_drain() has only a single caller.  How about I remove it and
open-code it within cpu_needs_drain()?