On 11/6/25 5:01 AM, Nilay Shroff wrote:
@@ -154,10 +153,8 @@ queue_ra_store(struct gendisk *disk, const char *page, size_t count) * calculated from the queue limits by queue_limits_commit_update. */ mutex_lock(&q->limits_lock);
- memflags = blk_mq_freeze_queue(q);
- disk->bdi->ra_pages = ra_kb >> (PAGE_SHIFT - 10);
- data_race(disk->bdi->ra_pages = ra_kb >> (PAGE_SHIFT - 10)); mutex_unlock(&q->limits_lock);
- blk_mq_unfreeze_queue(q, memflags);
return ret; }
I think we don't need data_race() here as disk->bdi->ra_pages is already protected by ->limits_lock. Furthermore, while you're at it, I’d suggest protecting the set/get access of ->ra_pages using ->limits_lock when it’s invoked from the ioctl context (BLKRASET/BLKRAGET).
I think that we really need the data_race() annotation here because there is plenty of code that reads ra_pages without using any locking.
@@ -480,9 +468,7 @@ static ssize_t queue_io_timeout_store(struct gendisk *disk, const char *page, if (err || val == 0) return -EINVAL;
- memflags = blk_mq_freeze_queue(q);
- blk_queue_rq_timeout(q, msecs_to_jiffies(val));
- blk_mq_unfreeze_queue(q, memflags);
- data_race((blk_queue_rq_timeout(q, msecs_to_jiffies(val)), 0));
return count; }
The use of data_race() above seems redundant, since the update to q->rq_timeout is already marked with WRITE_ONCE(). However, the read access to q->rq_timeout in a few places within the I/O hotpath is not marked and instead accessed directly using plain C-language loads.
That's fair. I will remove data_race() again from queue_io_timeout_store().
BUG: KCSAN: data-race in blk_add_timer+0x74/0x1f0
Based on the gdb trace:
(gdb) info line *(blk_add_timer+0x74) Line 138 of "block/blk-timeout.c" starts at address 0xc000000000d5637c <blk_add_timer+108> and ends at 0xc000000000d5638c <blk_add_timer+124>.
This corresponds to:
128 void blk_add_timer(struct request *req) 129 { 130 struct request_queue *q = req->q; 131 unsigned long expiry; 132 133 /* 134 * Some LLDs, like scsi, peek at the timeout to prevent a 135 * command from being retried forever. 136 */ 137 if (!req->timeout) 138 req->timeout = q->rq_timeout;
As seen above, the read access to q->rq_timeout is unmarked. To avoid the reported data race, we should replace this plain access with READ_ONCE(q->rq_timeout). This is one instance in the I/O hotpath where q->rq_timeout is read without annotation, and there are likely a few more similar cases that should be updated in the same way.
That's an existing issue and an issue that falls outside the scope of my patch.
Thanks,
Bart.