Linux cgroups development
 help / color / mirror / Atom feed
* [PATCH v3] block/blk-iocost: annotate ioc_pd_stat reads with data_race()
@ 2026-08-04  5:41 Tao Cui
  2026-08-04  6:42 ` Tejun Heo
  2026-08-04 13:23 ` Jens Axboe
  0 siblings, 2 replies; 3+ messages in thread
From: Tao Cui @ 2026-08-04  5:41 UTC (permalink / raw)
  To: tj, axboe; +Cc: josef, cgroups, linux-block, linux-kernel, cui.tao, Tao Cui

From: Tao Cui <cuitao@kylinos.cn>

ioc_pd_stat() reads ioc->enabled, ioc->vtime_base_rate, and
iocg->last_stat without holding ioc->lock, which trips KCSAN since
ioc_adjust_base_vrate() and iocg_flush_stat_upward() write those
fields under ioc->lock.

Commit 35198e323001 fixed the same issue in ioc_qos_prfill() and
ioc_cost_model_prfill() by adding spin_lock_irq(&ioc->lock).  However,
those functions read configuration parameters (qos/model) that need
synchronized reads.  In contrast, ioc_pd_stat() only reads stat
values (vrate, usage) where stale reads are harmless, so data_race()
is more appropriate — it silences the KCSAN warning without adding
lock contention during high-frequency stat reads.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>

---

Changes in v3:
- Use data_race() instead of spin_lock_irqsave/irqrestore, since
  these are stat values that don't require synchronized reads — the
  goal is just to silence KCSAN, not to guarantee consistency.
  (Tejun Heo)

Changes in v2:
- Use spin_lock_irqsave/irqrestore instead of spin_lock_irq/irq,
  since the caller (blkcg_print_stat) may already have IRQs disabled.

Link: https://lore.kernel.org/all/20260802150913.31977-1-cui.tao@linux.dev/
---
 block/blk-iocost.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 8b2aeba2e1e3..bc47ed034b25 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -3093,23 +3093,23 @@ static void ioc_pd_stat(struct blkg_policy_data *pd, struct seq_file *s)
 	struct ioc_gq *iocg = pd_to_iocg(pd);
 	struct ioc *ioc = iocg->ioc;
 
-	if (!ioc->enabled)
+	if (!data_race(ioc->enabled))
 		return;
 
 	if (iocg->level == 0) {
 		unsigned vp10k = DIV64_U64_ROUND_CLOSEST(
-			ioc->vtime_base_rate * 10000,
+			data_race(ioc->vtime_base_rate) * 10000,
 			VTIME_PER_USEC);
 		seq_printf(s, " cost.vrate=%u.%02u", vp10k / 100, vp10k % 100);
 	}
 
-	seq_printf(s, " cost.usage=%llu", iocg->last_stat.usage_us);
+	seq_printf(s, " cost.usage=%llu", data_race(iocg->last_stat.usage_us));
 
 	if (blkcg_debug_stats)
 		seq_printf(s, " cost.wait=%llu cost.indebt=%llu cost.indelay=%llu",
-			iocg->last_stat.wait_us,
-			iocg->last_stat.indebt_us,
-			iocg->last_stat.indelay_us);
+			data_race(iocg->last_stat.wait_us),
+			data_race(iocg->last_stat.indebt_us),
+			data_race(iocg->last_stat.indelay_us));
 }
 
 static u64 ioc_weight_prfill(struct seq_file *sf, struct blkg_policy_data *pd,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] block/blk-iocost: annotate ioc_pd_stat reads with data_race()
  2026-08-04  5:41 [PATCH v3] block/blk-iocost: annotate ioc_pd_stat reads with data_race() Tao Cui
@ 2026-08-04  6:42 ` Tejun Heo
  2026-08-04 13:23 ` Jens Axboe
  1 sibling, 0 replies; 3+ messages in thread
From: Tejun Heo @ 2026-08-04  6:42 UTC (permalink / raw)
  To: Tao Cui; +Cc: axboe, josef, cgroups, linux-block, linux-kernel, Tao Cui

On Tue, Aug 04, 2026 at 01:41:20PM +0800, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> ioc_pd_stat() reads ioc->enabled, ioc->vtime_base_rate, and
> iocg->last_stat without holding ioc->lock, which trips KCSAN since
> ioc_adjust_base_vrate() and iocg_flush_stat_upward() write those
> fields under ioc->lock.
> 
> Commit 35198e323001 fixed the same issue in ioc_qos_prfill() and
> ioc_cost_model_prfill() by adding spin_lock_irq(&ioc->lock).  However,
> those functions read configuration parameters (qos/model) that need
> synchronized reads.  In contrast, ioc_pd_stat() only reads stat
> values (vrate, usage) where stale reads are harmless, so data_race()
> is more appropriate — it silences the KCSAN warning without adding
> lock contention during high-frequency stat reads.
> 
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] block/blk-iocost: annotate ioc_pd_stat reads with data_race()
  2026-08-04  5:41 [PATCH v3] block/blk-iocost: annotate ioc_pd_stat reads with data_race() Tao Cui
  2026-08-04  6:42 ` Tejun Heo
@ 2026-08-04 13:23 ` Jens Axboe
  1 sibling, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2026-08-04 13:23 UTC (permalink / raw)
  To: tj, Tao Cui; +Cc: josef, cgroups, linux-block, linux-kernel, Tao Cui


On Tue, 04 Aug 2026 13:41:20 +0800, Tao Cui wrote:
> ioc_pd_stat() reads ioc->enabled, ioc->vtime_base_rate, and
> iocg->last_stat without holding ioc->lock, which trips KCSAN since
> ioc_adjust_base_vrate() and iocg_flush_stat_upward() write those
> fields under ioc->lock.
> 
> Commit 35198e323001 fixed the same issue in ioc_qos_prfill() and
> ioc_cost_model_prfill() by adding spin_lock_irq(&ioc->lock).  However,
> those functions read configuration parameters (qos/model) that need
> synchronized reads.  In contrast, ioc_pd_stat() only reads stat
> values (vrate, usage) where stale reads are harmless, so data_race()
> is more appropriate — it silences the KCSAN warning without adding
> lock contention during high-frequency stat reads.
> 
> [...]

Applied, thanks!

[1/1] block/blk-iocost: annotate ioc_pd_stat reads with data_race()
      commit: 4d73bf0ca4fbe7f252154ce98d6693c6c198164c

Best regards,
-- 
Jens Axboe




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-04 13:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-04  5:41 [PATCH v3] block/blk-iocost: annotate ioc_pd_stat reads with data_race() Tao Cui
2026-08-04  6:42 ` Tejun Heo
2026-08-04 13:23 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox