On 10/25/21 8:29 AM, Marco Elver wrote:
> On Mon, 25 Oct 2021 at 15:36, Jens Axboe <axboe@xxxxxxxxx> wrote:
> [...]
>>> write to 0xffffe8ffffd145b8 of 4 bytes by interrupt on cpu 1:
>>> sbitmap_queue_clear+0xca/0xf0 lib/sbitmap.c:606
>>> blk_mq_put_tag+0x82/0x90
>>> __blk_mq_free_request+0x114/0x180 block/blk-mq.c:507
>>> blk_mq_free_request+0x2c8/0x340 block/blk-mq.c:541
>>> __blk_mq_end_request+0x214/0x230 block/blk-mq.c:565
>>> blk_mq_end_request+0x37/0x50 block/blk-mq.c:574
>>> lo_complete_rq+0xca/0x170 drivers/block/loop.c:541
>>> blk_complete_reqs block/blk-mq.c:584 [inline]
>>> blk_done_softirq+0x69/0x90 block/blk-mq.c:589
>>> __do_softirq+0x12c/0x26e kernel/softirq.c:558
>>> run_ksoftirqd+0x13/0x20 kernel/softirq.c:920
>>> smpboot_thread_fn+0x22f/0x330 kernel/smpboot.c:164
>>> kthread+0x262/0x280 kernel/kthread.c:319
>>> ret_from_fork+0x1f/0x30
>>>
>>> write to 0xffffe8ffffd145b8 of 4 bytes by interrupt on cpu 0:
>>> sbitmap_queue_clear+0xca/0xf0 lib/sbitmap.c:606
>>> blk_mq_put_tag+0x82/0x90
>>> __blk_mq_free_request+0x114/0x180 block/blk-mq.c:507
>>> blk_mq_free_request+0x2c8/0x340 block/blk-mq.c:541
>>> __blk_mq_end_request+0x214/0x230 block/blk-mq.c:565
>>> blk_mq_end_request+0x37/0x50 block/blk-mq.c:574
>>> lo_complete_rq+0xca/0x170 drivers/block/loop.c:541
>>> blk_complete_reqs block/blk-mq.c:584 [inline]
>>> blk_done_softirq+0x69/0x90 block/blk-mq.c:589
>>> __do_softirq+0x12c/0x26e kernel/softirq.c:558
>>> run_ksoftirqd+0x13/0x20 kernel/softirq.c:920
>>> smpboot_thread_fn+0x22f/0x330 kernel/smpboot.c:164
>>> kthread+0x262/0x280 kernel/kthread.c:319
>>> ret_from_fork+0x1f/0x30
>>
>> This is just a per-cpu alloc hint, it's racy by nature. What's the
>> preferred way to silence these?
>
> That was my guess, but couldn't quite say. We started looking at
> write/write races as more likely to be harmful (vs. just read/write),
> and are inclined to let syzbot send out more of such reports. Marking
> intentional ones would be ideal so we'll be left with the
> unintentional ones.
>
> I would probably use WRITE_ONCE(), just to make sure the compiler
> doesn't play games here; or if the code is entirely tolerant to even
> the compiler miscompiling things, wrap the thing in data_race().
It's entirely tolerant, so something like this would do it?
diff --git a/lib/sbitmap.c b/lib/sbitmap.c
index c6e2f1f2c4d2..2709ab825499 100644
--- a/lib/sbitmap.c
+++ b/lib/sbitmap.c
@@ -631,7 +631,7 @@ EXPORT_SYMBOL_GPL(sbitmap_queue_wake_up);
static inline void sbitmap_update_cpu_hint(struct sbitmap *sb, int cpu, int tag)
{
if (likely(!sb->round_robin && tag < sb->depth))
- *per_cpu_ptr(sb->alloc_hint, cpu) = tag;
+ data_race(*per_cpu_ptr(sb->alloc_hint, cpu) = tag);
}
void sbitmap_queue_clear_batch(struct sbitmap_queue *sbq, int offset,
--
Jens Axboe
