On Thu, Jan 09, 2025 at 01:01:20PM +0100, Christian Borntraeger wrote:
>
>
> Am 07.01.25 um 19:25 schrieb Andrew Boyer:
> > Commit af8ececda185 ("virtio: add VIRTIO_F_NOTIFICATION_DATA feature
> > support") added notification data support to the core virtio driver
> > code. When this feature is enabled, the notification includes the
> > updated producer index for the queue. Thus it is now critical that
> > notifications arrive in order.
> >
> > The virtio_blk driver has historically not worried about notification
> > ordering. Modify it so that the prepare and kick steps are both done
> > under the vq lock.
> >
> > Signed-off-by: Andrew Boyer <[email protected]>
> > Reviewed-by: Brett Creeley <[email protected]>
> > Fixes: af8ececda185 ("virtio: add VIRTIO_F_NOTIFICATION_DATA feature
> > support")
> > Cc: Viktor Prutyanov <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > drivers/block/virtio_blk.c | 19 ++++---------------
> > 1 file changed, 4 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> > index 3efe378f1386..14d9e66bb844 100644
> > --- a/drivers/block/virtio_blk.c
> > +++ b/drivers/block/virtio_blk.c
> > @@ -379,14 +379,10 @@ static void virtio_commit_rqs(struct blk_mq_hw_ctx
> > *hctx)
> > {
> > struct virtio_blk *vblk = hctx->queue->queuedata;
> > struct virtio_blk_vq *vq = &vblk->vqs[hctx->queue_num];
> > - bool kick;
> > spin_lock_irq(&vq->lock);
> > - kick = virtqueue_kick_prepare(vq->vq);
> > + virtqueue_kick(vq->vq);
> > spin_unlock_irq(&vq->lock);
> > -
> > - if (kick)
> > - virtqueue_notify(vq->vq);
> > }
>
> I would assume this will be a performance nightmare for normal IO.
Indeed.
AMD guys, can't device survive with reordered notifications?
Basically just drop a notification if you see index
going back?
--
MST
