[PATCH] blk-mq-debugfs: mark concurrent stats counters as data races

From: Marco Elver
Date: Fri Feb 05 2021 - 12:15:41 EST


KCSAN reports that several of the blk-mq debugfs stats counters are
updated concurrently. Because blk-mq-debugfs does not demand precise
stats counters, potential lossy updates due to data races can be
tolerated. Therefore, mark and comment the accesses accordingly.

Reported-by: syzbot+2c308b859c8c103aae53@xxxxxxxxxxxxxxxxxxxxxxxxx
Reported-by: syzbot+44f9b37d2de57637dbfd@xxxxxxxxxxxxxxxxxxxxxxxxx
Reported-by: syzbot+49a9bcf457723ecaf1cf@xxxxxxxxxxxxxxxxxxxxxxxxx
Reported-by: syzbot+b9914ed52d5b1d63f71d@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Marco Elver <elver@xxxxxxxxxx>
---
Note: These 4 data races are among the most frequently encountered by
syzbot:

https://syzkaller.appspot.com/bug?id=7994761095b9677fb8bccaf41a77a82d5f444839
https://syzkaller.appspot.com/bug?id=08193ca23b80ec0e9bcbefba039162cff4f5d7a3
https://syzkaller.appspot.com/bug?id=7c51c15438f963024c4a4b3a6d7e119f4bdb2199
https://syzkaller.appspot.com/bug?id=6436cb57d04e8c5d6f0f40926d7511232aa2b5d4
---
block/blk-mq-debugfs.c | 22 ++++++++++++----------
block/blk-mq-sched.c | 3 ++-
block/blk-mq.c | 9 ++++++---
3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 4de03da9a624..687d201f0d7b 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -554,15 +554,16 @@ static int hctx_dispatched_show(void *data, struct seq_file *m)
struct blk_mq_hw_ctx *hctx = data;
int i;

- seq_printf(m, "%8u\t%lu\n", 0U, hctx->dispatched[0]);
+ seq_printf(m, "%8u\t%lu\n", 0U, data_race(hctx->dispatched[0]));

for (i = 1; i < BLK_MQ_MAX_DISPATCH_ORDER - 1; i++) {
unsigned int d = 1U << (i - 1);

- seq_printf(m, "%8u\t%lu\n", d, hctx->dispatched[i]);
+ seq_printf(m, "%8u\t%lu\n", d, data_race(hctx->dispatched[i]));
}

- seq_printf(m, "%8u+\t%lu\n", 1U << (i - 1), hctx->dispatched[i]);
+ seq_printf(m, "%8u+\t%lu\n", 1U << (i - 1),
+ data_race(hctx->dispatched[i]));
return 0;
}

@@ -573,7 +574,7 @@ static ssize_t hctx_dispatched_write(void *data, const char __user *buf,
int i;

for (i = 0; i < BLK_MQ_MAX_DISPATCH_ORDER; i++)
- hctx->dispatched[i] = 0;
+ data_race(hctx->dispatched[i] = 0);
return count;
}

@@ -581,7 +582,7 @@ static int hctx_queued_show(void *data, struct seq_file *m)
{
struct blk_mq_hw_ctx *hctx = data;

- seq_printf(m, "%lu\n", hctx->queued);
+ seq_printf(m, "%lu\n", data_race(hctx->queued));
return 0;
}

@@ -590,7 +591,7 @@ static ssize_t hctx_queued_write(void *data, const char __user *buf,
{
struct blk_mq_hw_ctx *hctx = data;

- hctx->queued = 0;
+ data_race(hctx->queued = 0);
return count;
}

@@ -598,7 +599,7 @@ static int hctx_run_show(void *data, struct seq_file *m)
{
struct blk_mq_hw_ctx *hctx = data;

- seq_printf(m, "%lu\n", hctx->run);
+ seq_printf(m, "%lu\n", data_race(hctx->run));
return 0;
}

@@ -607,7 +608,7 @@ static ssize_t hctx_run_write(void *data, const char __user *buf, size_t count,
{
struct blk_mq_hw_ctx *hctx = data;

- hctx->run = 0;
+ data_race(hctx->run = 0);
return count;
}

@@ -702,7 +703,8 @@ static int ctx_completed_show(void *data, struct seq_file *m)
{
struct blk_mq_ctx *ctx = data;

- seq_printf(m, "%lu %lu\n", ctx->rq_completed[1], ctx->rq_completed[0]);
+ seq_printf(m, "%lu %lu\n", data_race(ctx->rq_completed[1]),
+ data_race(ctx->rq_completed[0]));
return 0;
}

@@ -711,7 +713,7 @@ static ssize_t ctx_completed_write(void *data, const char __user *buf,
{
struct blk_mq_ctx *ctx = data;

- ctx->rq_completed[0] = ctx->rq_completed[1] = 0;
+ data_race(ctx->rq_completed[0] = ctx->rq_completed[1] = 0);
return count;
}

diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index deff4e826e23..71a49835e89a 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -332,7 +332,8 @@ void blk_mq_sched_dispatch_requests(struct blk_mq_hw_ctx *hctx)
if (unlikely(blk_mq_hctx_stopped(hctx) || blk_queue_quiesced(q)))
return;

- hctx->run++;
+ /* data race ok: hctx->run only for debugfs stats. */
+ data_race(hctx->run++);

/*
* A return of -EAGAIN is an indication that hctx->dispatch is not
diff --git a/block/blk-mq.c b/block/blk-mq.c
index f285a9123a8b..1d8970602032 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -341,7 +341,8 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data,
}
}

- data->hctx->queued++;
+ /* data race ok: hctx->queued only for debugfs stats. */
+ data_race(data->hctx->queued++);
return rq;
}

@@ -519,7 +520,8 @@ void blk_mq_free_request(struct request *rq)
}
}

- ctx->rq_completed[rq_is_sync(rq)]++;
+ /* data race ok: ctx->rq_completed only for debugfs stats. */
+ data_race(ctx->rq_completed[rq_is_sync(rq)]++);
if (rq->rq_flags & RQF_MQ_INFLIGHT)
__blk_mq_dec_active_requests(hctx);

@@ -1419,7 +1421,8 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
if (!list_empty(&zone_list))
list_splice_tail_init(&zone_list, list);

- hctx->dispatched[queued_to_index(queued)]++;
+ /* data race ok: hctx->dispatched only for debugfs stats. */
+ data_race(hctx->dispatched[queued_to_index(queued)]++);

/* If we didn't flush the entire list, we could have told the driver
* there was more coming, but that turned out to be a lie.

base-commit: 61556703b610a104de324e4f061dc6cf7b218b46
--
2.30.0.365.g02bc693789-goog