> On Jan 22, 2025, at 12:33 PM, Christian Borntraeger > <[email protected]> wrote: > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > Am 22.01.25 um 15:44 schrieb Boyer, Andrew: > [...] > >>>>> --- 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. >>> >> >> Hello Michael and Christian and Jason, >> Thank you for taking a look. >> >> Is the performance concern that the vmexit might lead to the underlying >> virtual storage stack doing the work immediately? Any other job posting to >> the same queue would presumably be blocked on a vmexit when it goes to >> attempt its own notification. That would be almost the same as having the >> other job block on a lock during the operation, although I guess if you are >> skipping notifications somehow it would look different. > > The performance concern is that you hold a lock and then exit. Exits are > expensive and can schedule so you will increase the lock holding time > significantly. This is begging for lock holder preemption. > Really, dont do it.
If holding a lock at all is unworkable, do you have a suggestion for how we might fix the correctness issue? Because this is definitely a correctness issue. I don't see anything in the spec about requiring support for out-of-order notifications. -Andrew
