[PATCH v3] block/blk-iocost: annotate ioc_pd_stat reads with data_race()

Tao Cui <[email protected]>
Newsgroups org.kernel.vger.linux-kernel,org.kernel.vger.cgroups,org.kernel.vger.linux-block
Message-ID <[email protected]>
From: Tao Cui <[email protected]>

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 <[email protected]>

---

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/[email protected]/
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.