* Re: [PATCH v3] mm: annotate data-race in cpu_needs_drain()
2026-06-26 5:37 [PATCH v3] mm: annotate data-race in cpu_needs_drain() Xuewen Wang
@ 2026-06-26 8:17 ` David Hildenbrand (Arm)
2026-06-29 10:23 ` Lorenzo Stoakes
2026-06-26 8:32 ` Pedro Falcato
2026-06-29 10:22 ` Lorenzo Stoakes
2 siblings, 1 reply; 5+ messages in thread
From: David Hildenbrand (Arm) @ 2026-06-26 8:17 UTC (permalink / raw)
To: Xuewen Wang, akpm, liam, ljs, vbabka, jannh, pfalcato, chrisl,
kasong, shikemeng, nphamcs, baoquan.he, baohua, youngjun.park,
qi.zheng, shakeel.butt, axelrasmussen, yuanchu, weixugc
Cc: linux-mm, linux-kernel
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v3] mm: annotate data-race in cpu_needs_drain()
2026-06-26 8:17 ` David Hildenbrand (Arm)
@ 2026-06-29 10:23 ` Lorenzo Stoakes
0 siblings, 0 replies; 5+ messages in thread
From: Lorenzo Stoakes @ 2026-06-29 10:23 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Xuewen Wang, akpm, liam, vbabka, jannh, pfalcato, chrisl, kasong,
shikemeng, nphamcs, baoquan.he, baohua, youngjun.park, qi.zheng,
shakeel.butt, axelrasmussen, yuanchu, weixugc, linux-mm,
linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] mm: annotate data-race in cpu_needs_drain()
2026-06-26 5:37 [PATCH v3] mm: annotate data-race in cpu_needs_drain() Xuewen Wang
2026-06-26 8:17 ` David Hildenbrand (Arm)
@ 2026-06-26 8:32 ` Pedro Falcato
2026-06-29 10:22 ` Lorenzo Stoakes
2 siblings, 0 replies; 5+ messages in thread
From: Pedro Falcato @ 2026-06-26 8:32 UTC (permalink / raw)
To: Xuewen Wang
Cc: akpm, liam, ljs, vbabka, jannh, chrisl, kasong, shikemeng,
nphamcs, baoquan.he, baohua, youngjun.park, qi.zheng,
shakeel.butt, axelrasmussen, yuanchu, weixugc, david, linux-mm,
linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] mm: annotate data-race in cpu_needs_drain()
2026-06-26 5:37 [PATCH v3] mm: annotate data-race in cpu_needs_drain() Xuewen Wang
2026-06-26 8:17 ` David Hildenbrand (Arm)
2026-06-26 8:32 ` Pedro Falcato
@ 2026-06-29 10:22 ` Lorenzo Stoakes
2 siblings, 0 replies; 5+ messages in thread
From: Lorenzo Stoakes @ 2026-06-29 10:22 UTC (permalink / raw)
To: Xuewen Wang
Cc: akpm, liam, vbabka, jannh, pfalcato, chrisl, kasong, shikemeng,
nphamcs, baoquan.he, baohua, youngjun.park, qi.zheng,
shakeel.butt, axelrasmussen, yuanchu, weixugc, david, linux-mm,
linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread