On Thu, Mar 24, 2022 at 11:04:50PM +0900, Suwan Kim wrote: > @@ -367,6 +381,66 @@ static blk_status_t virtio_queue_rq(struct blk_mq_hw_ctx *hctx, > return BLK_STS_OK; > } > > +static bool virtblk_prep_rq_batch(struct virtio_blk_vq *vq, struct request *req) > +{ > + struct virtio_blk *vblk = req->mq_hctx->queue->queuedata; > + struct virtblk_req *vbr = blk_mq_rq_to_pdu(req); > + unsigned long flags; > + int num, err; > + > + req->mq_hctx->tags->rqs[req->tag] = req; > + > + if (virtblk_prep_rq(req->mq_hctx, vblk, req, vbr, &num) != BLK_STS_OK) > + return false; > + > + spin_lock_irqsave(&vq->lock, flags); > + err = virtblk_add_req(vq->vq, vbr, vbr->sg_table.sgl, num); > + if (err) { > + spin_unlock_irqrestore(&vq->lock, flags); > + virtblk_unmap_data(req, vbr); > + virtblk_cleanup_cmd(req); > + return false; > + } > + spin_unlock_irqrestore(&vq->lock, flags); Simplification: spin_lock_irqsave(&vq->lock, flags); err = virtblk_add_req(vq->vq, vbr, vbr->sg_table.sgl, num); spin_unlock_irqrestore(&vq->lock, flags); if (err) { virtblk_unmap_data(req, vbr); virtblk_cleanup_cmd(req); return false; } > + > + return true; > +} > + > +static void virtio_queue_rqs(struct request **rqlist) > +{ > + struct request *req, *next, *prev = NULL; > + struct request *requeue_list = NULL; > + > + rq_list_for_each_safe(rqlist, req, next) { > + struct virtio_blk_vq *vq = req->mq_hctx->driver_data; > + unsigned long flags; > + bool kick; > + > + if (!virtblk_prep_rq_batch(vq, req)) { > + rq_list_move(rqlist, &requeue_list, req, prev); > + req = prev; > + > + if (!req) > + continue; > + } > + > + if (!next || req->mq_hctx != next->mq_hctx) { > + spin_lock_irqsave(&vq->lock, flags); Did you try calling virtblk_add_req() here to avoid acquiring and releasing the lock multiple times? In other words, do virtblk_prep_rq() but wait until we get here to do virtblk_add_req(). I don't know if it has any measurable effect on performance or maybe the code would become too complex, but I noticed that we're not fully exploiting batching. > + kick = virtqueue_kick_prepare(vq->vq); > + spin_unlock_irqrestore(&vq->lock, flags); > + if (kick) > + virtqueue_notify(vq->vq); > + > + req->rq_next = NULL; > + *rqlist = next; > + prev = NULL; > + } else > + prev = req; What guarantees that req is still alive after we called virtblk_add_req()? The device may have seen it and completed it already by the time we get here.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization