On Tue, Dec 06, 2022 at 11:11:25PM +0900, Suwan Kim wrote:
> This patch adds completion batching to the IRQ path. It reuses batch
> completion code of virtblk_poll(). It collects requests to io_comp_batch
> and processes them all at once. It can boost up the performance by 2%.
>
> To validate the performance improvement and stabilty, I did fio test with
> 4 vCPU VM and 12 vCPU VM respectively. Both VMs have 8GB ram and the same
> number of HW queues as vCPU.
> The fio cammad is as follows and I ran the fio 5 times and got IOPS average.
> (io_uring, randread, direct=1, bs=512, iodepth=64 numjobs=2,4)
>
> Test result shows about 2% improvement.
>
> 4 vcpu VM | numjobs=2 | numjobs=4
> -----------------------------------------------------------
> fio without patch | 367.2K IOPS | 397.6K IOPS
> -----------------------------------------------------------
> fio with patch | 372.8K IOPS | 407.7K IOPS
>
> 12 vcpu VM | numjobs=2 | numjobs=4
> -----------------------------------------------------------
> fio without patch | 363.6K IOPS | 374.8K IOPS
> -----------------------------------------------------------
> fio with patch | 373.8K IOPS | 385.3K IOPS
>
> Signed-off-by: Suwan Kim <suwan.kim027@xxxxxxxxx>
> ---
> drivers/block/virtio_blk.c | 38 +++++++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 15 deletions(-)
Cool, thanks for doing this!
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index cf64d256787e..48fcf745f007 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -272,6 +272,18 @@ static inline void virtblk_request_done(struct request *req)
> blk_mq_end_request(req, virtblk_result(vbr));
> }
>
> +static void virtblk_complete_batch(struct io_comp_batch *iob)
> +{
> + struct request *req;
> +
> + rq_list_for_each(&iob->req_list, req) {
> + virtblk_unmap_data(req, blk_mq_rq_to_pdu(req));
> + virtblk_cleanup_cmd(req);
> + blk_mq_set_request_complete(req);
> + }
> + blk_mq_end_request_batch(iob);
> +}
> +
> static void virtblk_done(struct virtqueue *vq)
> {
> struct virtio_blk *vblk = vq->vdev->priv;
> @@ -280,6 +292,7 @@ static void virtblk_done(struct virtqueue *vq)
> struct virtblk_req *vbr;
> unsigned long flags;
> unsigned int len;
> + DEFINE_IO_COMP_BATCH(iob);
>
> spin_lock_irqsave(&vblk->vqs[qid].lock, flags);
> do {
> @@ -287,7 +300,9 @@ static void virtblk_done(struct virtqueue *vq)
> while ((vbr = virtqueue_get_buf(vblk->vqs[qid].vq, &len)) != NULL) {
> struct request *req = blk_mq_rq_from_pdu(vbr);
>
> - if (likely(!blk_should_fake_timeout(req->q)))
> + if (likely(!blk_should_fake_timeout(req->q)) &&
> + !blk_mq_add_to_batch(req, &iob, vbr->status,
> + virtblk_complete_batch))
> blk_mq_complete_request(req);
> req_done = true;
> }
> @@ -295,9 +310,14 @@ static void virtblk_done(struct virtqueue *vq)
> break;
> } while (!virtqueue_enable_cb(vq));
>
> - /* In case queue is stopped waiting for more buffers. */
> - if (req_done)
> + if (req_done) {
> + if (!rq_list_empty(iob.req_list))
> + virtblk_complete_batch(&iob);
A little optimization to avoid the indirect call: iob.complete(&iob) :).
Not sure if it's good style to do that but it works in this case because
we know it can only be virtblk_complete_batch().
Reviewed-by: Stefan Hajnoczi <stefanha@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
