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

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

[Xuewen Wang]

KCSAN reports a data-race when cpu_needs_drain() reads another CPU's
per-cpu folio_batch->nr without locking, while the owning CPU writes
to it via folio_batch_add().

Reading a slightly stale value is harmless -- cpu_needs_drain() only
decides whether to schedule a drain, and the next iteration of
__lru_add_drain_all() will re-check. Use data_race() to annotate
the intentional race.

Signed-off-by: Xuewen Wang <wangxuewen@kylinos.cn>
---
Changes in v3:
- Wrap the entire || expression in a single data_race() instead of wrapping
  each folio_batch_count() call individually, as suggested by Pedro and Lorenzo.
  This is equally effective and more readable.
- Remove data_race() from need_mlock_drain(), as it is now covered by the data_race()
  in cpu_needs_drain().
v2:
  https://lore.kernel.org/all/20260625065153.1581419-1-wangxuewen@kylinos.cn/
v1:
  https://lore.kernel.org/all/20260624092606.1083449-1-wangxuewen@kylinos.cn/
---
 mm/swap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/swap.c b/mm/swap.c
index 588f50d8f1a8..46ea207e0624 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) ||
+	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) ||
+		need_mlock_drain(cpu)) ||
 		has_bh_in_lru(cpu, NULL);
 }
 
-- 
2.25.1

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

[David Hildenbrand (Arm)]

On 6/26/26 07:37, Xuewen Wang wrote:
> KCSAN reports a data-race when cpu_needs_drain() reads another CPU's
> per-cpu folio_batch->nr without locking, while the owning CPU writes
> to it via folio_batch_add().
> 
> Reading a slightly stale value is harmless -- cpu_needs_drain() only
> decides whether to schedule a drain, and the next iteration of
> __lru_add_drain_all() will re-check. Use data_race() to annotate
> the intentional race.
> 
> Signed-off-by: Xuewen Wang <wangxuewen@kylinos.cn>
> ---
> Changes in v3:
> - Wrap the entire || expression in a single data_race() instead of wrapping
>   each folio_batch_count() call individually, as suggested by Pedro and Lorenzo.
>   This is equally effective and more readable.
> - Remove data_race() from need_mlock_drain(), as it is now covered by the data_race()
>   in cpu_needs_drain().
> v2:
>   https://lore.kernel.org/all/20260625065153.1581419-1-wangxuewen@kylinos.cn/
> v1:
>   https://lore.kernel.org/all/20260624092606.1083449-1-wangxuewen@kylinos.cn/
> ---
>  mm/swap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/swap.c b/mm/swap.c
> index 588f50d8f1a8..46ea207e0624 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) ||
> +	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) ||
> +		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);
 }

But I'll let others decide :)

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

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

[Pedro Falcato]

On Fri, Jun 26, 2026 at 01:37:00PM +0800, Xuewen Wang wrote:
> KCSAN reports a data-race when cpu_needs_drain() reads another CPU's
> per-cpu folio_batch->nr without locking, while the owning CPU writes
> to it via folio_batch_add().
> 
> Reading a slightly stale value is harmless -- cpu_needs_drain() only
> decides whether to schedule a drain, and the next iteration of
> __lru_add_drain_all() will re-check. Use data_race() to annotate
> the intentional race.
> 
> Signed-off-by: Xuewen Wang <wangxuewen@kylinos.cn>

Reviewed-by: Pedro Falcato <pfalcato@suse.de>

> ---
> Changes in v3:
> - Wrap the entire || expression in a single data_race() instead of wrapping
>   each folio_batch_count() call individually, as suggested by Pedro and Lorenzo.
>   This is equally effective and more readable.
> - Remove data_race() from need_mlock_drain(), as it is now covered by the data_race()
>   in cpu_needs_drain().
> v2:
>   https://lore.kernel.org/all/20260625065153.1581419-1-wangxuewen@kylinos.cn/
> v1:
>   https://lore.kernel.org/all/20260624092606.1083449-1-wangxuewen@kylinos.cn/
> ---
>  mm/swap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/swap.c b/mm/swap.c
> index 588f50d8f1a8..46ea207e0624 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) ||
> +	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) ||
> +		need_mlock_drain(cpu)) ||
>  		has_bh_in_lru(cpu, NULL);
>  }
>  
> -- 
> 2.25.1
> 

-- 
Pedro

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

[Lorenzo Stoakes]

On Fri, Jun 26, 2026 at 01:37:00PM +0800, Xuewen Wang wrote:
> KCSAN reports a data-race when cpu_needs_drain() reads another CPU's
> per-cpu folio_batch->nr without locking, while the owning CPU writes
> to it via folio_batch_add().
>
> Reading a slightly stale value is harmless -- cpu_needs_drain() only
> decides whether to schedule a drain, and the next iteration of
> __lru_add_drain_all() will re-check. Use data_race() to annotate
> the intentional race.
>
> Signed-off-by: Xuewen Wang <wangxuewen@kylinos.cn>

LGTM, so:

Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>

> ---
> Changes in v3:
> - Wrap the entire || expression in a single data_race() instead of wrapping
>   each folio_batch_count() call individually, as suggested by Pedro and Lorenzo.
>   This is equally effective and more readable.
> - Remove data_race() from need_mlock_drain(), as it is now covered by the data_race()
>   in cpu_needs_drain().
> v2:
>   https://lore.kernel.org/all/20260625065153.1581419-1-wangxuewen@kylinos.cn/
> v1:
>   https://lore.kernel.org/all/20260624092606.1083449-1-wangxuewen@kylinos.cn/
> ---
>  mm/swap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/swap.c b/mm/swap.c
> index 588f50d8f1a8..46ea207e0624 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) ||
> +	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) ||
> +		need_mlock_drain(cpu)) ||
>  		has_bh_in_lru(cpu, NULL);
>  }
>
> --
> 2.25.1
>

Cheers, Lorenzo

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

[Lorenzo Stoakes]

On Fri, Jun 26, 2026 at 10:17:14AM +0200, David Hildenbrand (Arm) wrote:
> On 6/26/26 07:37, Xuewen Wang wrote:
> > KCSAN reports a data-race when cpu_needs_drain() reads another CPU's
> > per-cpu folio_batch->nr without locking, while the owning CPU writes
> > to it via folio_batch_add().
> >
> > Reading a slightly stale value is harmless -- cpu_needs_drain() only
> > decides whether to schedule a drain, and the next iteration of
> > __lru_add_drain_all() will re-check. Use data_race() to annotate
> > the intentional race.
> >
> > Signed-off-by: Xuewen Wang <wangxuewen@kylinos.cn>
> > ---
> > Changes in v3:
> > - Wrap the entire || expression in a single data_race() instead of wrapping
> >   each folio_batch_count() call individually, as suggested by Pedro and Lorenzo.
> >   This is equally effective and more readable.
> > - Remove data_race() from need_mlock_drain(), as it is now covered by the data_race()
> >   in cpu_needs_drain().
> > v2:
> >   https://lore.kernel.org/all/20260625065153.1581419-1-wangxuewen@kylinos.cn/
> > v1:
> >   https://lore.kernel.org/all/20260624092606.1083449-1-wangxuewen@kylinos.cn/
> > ---
> >  mm/swap.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/swap.c b/mm/swap.c
> > index 588f50d8f1a8..46ea207e0624 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) ||
> > +	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) ||
> > +		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? :)

>  }
>
> But I'll let others decide :)
>
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>
>
> --
> Cheers,
>
> David

Cheers, Lorenzo

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@kernel.org> 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()?

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

[Lorenzo Stoakes]

On Mon, Jun 29, 2026 at 09:34:02AM -0700, Andrew Morton wrote:
> On Mon, 29 Jun 2026 11:23:26 +0100 Lorenzo Stoakes <ljs@kernel.org> 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()?

That references a static per-CPU variable (mlock_fbatch) in mlock.c's
compilation unit so I think it has to stay as it us unfortunately.

And it's better I think to only use the data_race() here where we definitely
know we need it (and as the only instance of that).

Cheers, Lorenzo

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

[Pedro Falcato]

On Mon, Jun 29, 2026 at 05:45:22PM +0100, Lorenzo Stoakes wrote:
> On Mon, Jun 29, 2026 at 09:34:02AM -0700, Andrew Morton wrote:
> > On Mon, 29 Jun 2026 11:23:26 +0100 Lorenzo Stoakes <ljs@kernel.org> 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()?
> 
> That references a static per-CPU variable (mlock_fbatch) in mlock.c's
> compilation unit so I think it has to stay as it us unfortunately.
> 
> And it's better I think to only use the data_race() here where we definitely
> know we need it (and as the only instance of that).

I agree. FWIW something that would perhaps be nice to pull off would be:
lockdep_assert_held_or_data_race(), or something with a less excrutiating
name. Which could assert "either we hold a lock, or the caller is aware
that this can race". Which sounds nice. In that case, we could simply
slap that on need_mlock_drain() as requiring the mlock_fbatch's local lock,
or data_race().

It night be too special purpose though.

-- 
Pedro