* [PATCH v2 1/2] virtio-blk: set req->state to MQ_RQ_COMPLETE after polling I/O is finished
2022-12-20 15:36 [PATCH v2 0/2] virtio-blk: set req->state to MQ_RQ_COMPLETE and support completion batching for the IRQ path Suwan Kim
@ 2022-12-20 15:36 ` Suwan Kim
2022-12-21 10:45 ` Christoph Hellwig
2022-12-20 15:36 ` [PATCH v2 2/2] virtio-blk: support completion batching for the IRQ path Suwan Kim
1 sibling, 1 reply; 7+ messages in thread
From: Suwan Kim @ 2022-12-20 15:36 UTC (permalink / raw)
To: mst, jasowang, stefanha, pbonzini, hch, axboe
Cc: virtualization, linux-block, Suwan Kim
Driver should set req->state to MQ_RQ_COMPLETE after it finishes to process
req. But virtio-blk doesn't set MQ_RQ_COMPLETE after virtblk_poll() handles
req and req->state still remains MQ_RQ_IN_FLIGHT. Fortunately so far there
is no issue about it because blk_mq_end_request_batch() sets req->state to
MQ_RQ_IDLE.
In this patch, virblk_poll() calls blk_mq_complete_request_remote() to set
req->state to MQ_RQ_COMPLETE before it adds req to a batch completion list.
So it properly sets req->state after polling I/O is finished.
Fixes: 4e0400525691 ("virtio-blk: support polling I/O")
Signed-off-by: Suwan Kim <suwan.kim027@gmail.com>
---
drivers/block/virtio_blk.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 19da5defd734..75ee51aba964 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -858,9 +858,10 @@ static int virtblk_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
struct request *req = blk_mq_rq_from_pdu(vbr);
found++;
- if (!blk_mq_add_to_batch(req, iob, vbr->status,
+ if (!blk_mq_complete_request_remote(req) &&
+ !blk_mq_add_to_batch(req, iob, vbr->status,
virtblk_complete_batch))
- blk_mq_complete_request(req);
+ virtblk_request_done(req);
}
if (found)
--
2.26.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 2/2] virtio-blk: support completion batching for the IRQ path
2022-12-20 15:36 [PATCH v2 0/2] virtio-blk: set req->state to MQ_RQ_COMPLETE and support completion batching for the IRQ path Suwan Kim
2022-12-20 15:36 ` [PATCH v2 1/2] virtio-blk: set req->state to MQ_RQ_COMPLETE after polling I/O is finished Suwan Kim
@ 2022-12-20 15:36 ` Suwan Kim
2022-12-21 10:48 ` Christoph Hellwig
1 sibling, 1 reply; 7+ messages in thread
From: Suwan Kim @ 2022-12-20 15:36 UTC (permalink / raw)
To: mst, jasowang, stefanha, pbonzini, hch, axboe
Cc: virtualization, linux-block, Suwan Kim
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@gmail.com>
---
drivers/block/virtio_blk.c | 82 +++++++++++++++++++++-----------------
1 file changed, 45 insertions(+), 37 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 75ee51aba964..0ac7b570300c 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -272,33 +272,63 @@ 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_end_request_batch(iob);
+}
+
+static int virtblk_handle_req(struct virtio_blk_vq *vq,
+ struct io_comp_batch *iob)
+{
+ struct virtblk_req *vbr;
+ int req_done = 0;
+ unsigned int len;
+
+ while ((vbr = virtqueue_get_buf(vq->vq, &len)) != NULL) {
+ struct request *req = blk_mq_rq_from_pdu(vbr);
+
+ if (likely(!blk_should_fake_timeout(req->q)) &&
+ !blk_mq_complete_request_remote(req) &&
+ !blk_mq_add_to_batch(req, iob, vbr->status,
+ virtblk_complete_batch))
+ virtblk_request_done(req);
+ req_done++;
+ }
+
+ return req_done;
+}
+
static void virtblk_done(struct virtqueue *vq)
{
struct virtio_blk *vblk = vq->vdev->priv;
- bool req_done = false;
- int qid = vq->index;
- struct virtblk_req *vbr;
+ struct virtio_blk_vq *vblk_vq = &vblk->vqs[vq->index];
+ int req_done = 0;
unsigned long flags;
- unsigned int len;
+ DEFINE_IO_COMP_BATCH(iob);
- spin_lock_irqsave(&vblk->vqs[qid].lock, flags);
+ spin_lock_irqsave(&vblk_vq->lock, flags);
do {
virtqueue_disable_cb(vq);
- while ((vbr = virtqueue_get_buf(vblk->vqs[qid].vq, &len)) != NULL) {
- struct request *req = blk_mq_rq_from_pdu(vbr);
+ req_done += virtblk_handle_req(vblk_vq, &iob);
- if (likely(!blk_should_fake_timeout(req->q)))
- blk_mq_complete_request(req);
- req_done = true;
- }
if (unlikely(virtqueue_is_broken(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))
+ iob.complete(&iob);
+
+ /* In case queue is stopped waiting for more buffers. */
blk_mq_start_stopped_hw_queues(vblk->disk->queue, true);
- spin_unlock_irqrestore(&vblk->vqs[qid].lock, flags);
+ }
+ spin_unlock_irqrestore(&vblk_vq->lock, flags);
}
static void virtio_commit_rqs(struct blk_mq_hw_ctx *hctx)
@@ -832,37 +862,15 @@ static void virtblk_map_queues(struct blk_mq_tag_set *set)
}
}
-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_end_request_batch(iob);
-}
-
static int virtblk_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
{
struct virtio_blk *vblk = hctx->queue->queuedata;
struct virtio_blk_vq *vq = get_virtio_blk_vq(hctx);
- struct virtblk_req *vbr;
unsigned long flags;
- unsigned int len;
int found = 0;
spin_lock_irqsave(&vq->lock, flags);
-
- while ((vbr = virtqueue_get_buf(vq->vq, &len)) != NULL) {
- struct request *req = blk_mq_rq_from_pdu(vbr);
-
- found++;
- if (!blk_mq_complete_request_remote(req) &&
- !blk_mq_add_to_batch(req, iob, vbr->status,
- virtblk_complete_batch))
- virtblk_request_done(req);
- }
+ found = virtblk_handle_req(vq, iob);
if (found)
blk_mq_start_stopped_hw_queues(vblk->disk->queue, true);
--
2.26.3
^ permalink raw reply related [flat|nested] 7+ messages in thread