On Thu, 12 Sept 2024 at 16:45, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote: > > event_triggered is fundamentally racy. There are races of 2 types: > 1. vq processing can read false value while interrupt > triggered and set it to true. > result will be a bit of extra work when disabling cbs, no big deal. > > 1. vq processing can set false value then interrupt > immediately sets true value > since interrupt then triggers a callback which will > process buffers, this is also not an issue. > > However, looks like KCSAN isn't smart enough to figure this out. > Tag the field __data_racy for now. > We should probably look at ways to make this more straight-forwardly > correct. > > Cc: Marco Elver <elver@xxxxxxxxxx> > Reported-by: syzbot+8a02104389c2e0ef5049@xxxxxxxxxxxxxxxxxxxxxxxxx > Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx> > --- > drivers/virtio/virtio_ring.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index be7309b1e860..724aa9c27c6b 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -194,7 +194,7 @@ struct vring_virtqueue { > u16 last_used_idx; > > /* Hint for event idx: already triggered no need to disable. */ > - bool event_triggered; > + bool __data_racy event_triggered; I guess if you don't care about any data races on this variable, this is reasonable. Although note that data race is more subtle than just a "race": https://lwn.net/Articles/816850/ Acked-by: Marco Elver <elver@xxxxxxxxxx>