Re: [PATCH] blk-mq: plug request for shared sbitmap
From: John Garry <hidden>
Date: 2021-05-18 09:45:49
On 14/05/2021 03:20, Ming Lei wrote:
In case of shared sbitmap, request won't be held in plug list any more
sine commit 32bc15afed04 ("blk-mq: Facilitate a shared sbitmap per
tagset"), this way makes request merge from flush plug list & batching
submission not possible, so cause performance regression.
Yanhui reports performance regression when running sequential IO
test(libaio, 16 jobs, 8 depth for each job) in VM, and the VM disk
is emulated with image stored on xfs/megaraid_sas.
Fix the issue by recovering original behavior to allow to hold request
in plug list.Hi Ming, Since testing v5.13-rc2, I noticed that this patch made the hang I was seeing disappear: https://lore.kernel.org/linux-scsi/3d72d64d-314f-9d34-e039-7e508b2abe1b@huawei.com/ (local) I don't think that problem is solved, though. So I wonder about throughput performance (I had hoped to test before it was merged). I only have 6x SAS SSDs at hand, but I see some significant changes (good and bad) for mq-deadline for hisi_sas: Before 620K (read), 300K IOPs (randread) After 430K (read), 460-490K IOPs (randread) none IO sched is always about 450K (read) and 500K (randread) Do you guys have any figures? Are my results as expected? Thanks, John
quoted hunk ↗ jump to hunk
Cc: Yanhui Ma <redacted> Cc: John Garry <redacted> Cc: Bart Van Assche <bvanassche@acm.org> Cc: kashyap.desai@broadcom.com Fixes: 32bc15afed04 ("blk-mq: Facilitate a shared sbitmap per tagset") Signed-off-by: Ming Lei <redacted> --- block/blk-mq.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)diff --git a/block/blk-mq.c b/block/blk-mq.c index ae7f5ee41cd3..baf7a9546068 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c@@ -2236,8 +2236,9 @@ blk_qc_t blk_mq_submit_bio(struct bio *bio) /* Bypass scheduler for flush requests */ blk_insert_flush(rq); blk_mq_run_hw_queue(data.hctx, true); - } else if (plug && (q->nr_hw_queues == 1 || q->mq_ops->commit_rqs || - !blk_queue_nonrot(q))) { + } else if (plug && (q->nr_hw_queues == 1 || + blk_mq_is_sbitmap_shared(rq->mq_hctx->flags) || + q->mq_ops->commit_rqs || !blk_queue_nonrot(q))) { /* * Use plugging if we have a ->commit_rqs() hook as well, as * we know the driver uses bd->last in a smart fashion.