On 2019/11/15 7:51, longli@xxxxxxxxxxxxxxxxx wrote:
> From: Long Li <longli@xxxxxxxxxxxxx>
>
> SCSI layer calls blk_mq_run_hw_queues() in scsi_end_request(), for every
> completed I/O. blk_mq_run_hw_queues() in turn schedules some works to run
> the hardware queues.
>
> The actual work is queued by mod_delayed_work_on(), it turns out the cost of
> this function is high on locking and CPU usage, when the I/O workload has
> high queue depth. Most of these calls are not necessary since the queue is
> already scheduled to run, and has not run yet.
>
> This patch tries to solve this problem by avoiding scheduling work when it's
> already scheduled.
>
> Benchmark results:
> The following tests are run on a RAM backed virtual disk on Hyper-V, with 8
> FIO jobs with 4k random read I/O. The test numbers are for IOPS.
>
> queue_depth pre-patch after-patch improvement
> 16 190k 190k 0%
> 64 235k 240k 2%
> 256 180k 256k 42%
> 1024 156k 250k 60%
>
> Signed-off-by: Long Li <longli@xxxxxxxxxxxxx>
> ---
> block/blk-mq.c | 12 ++++++++++++
> include/linux/blk-mq.h | 1 +
> 2 files changed, 13 insertions(+)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index ec791156e9cc..a882bd65167a 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1476,6 +1476,16 @@ static void __blk_mq_delay_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async,
> put_cpu();
> }
>
> + /*
> + * Queue a work to run queue. If this is a non-delay run and the
> + * work is already scheduled, avoid scheduling the same work again.
> + */
> + if (!msecs) {
> + if (test_bit(BLK_MQ_S_WORK_QUEUED, &hctx->state))
> + return;
With this change, if the kblockd work is already scheduled with a delay,
then the current no-delay run request will incur a delay because
kblockd_mod_delayed_work_on() is not called, implying that
__queue_delayed_work() does not execute __queue_work() as mandated by
the 0 delay. The work is *not* started immediately.
While your results show improvements of IOPS at high queue depth,
doesn't this change degrade IOPS and especially latency at low queue depth ?
> + set_bit(BLK_MQ_S_WORK_QUEUED, &hctx->state);
> + }
> +
> kblockd_mod_delayed_work_on(blk_mq_hctx_next_cpu(hctx), &hctx->run_work,
> msecs_to_jiffies(msecs));
> }
> @@ -1561,6 +1571,7 @@ void blk_mq_stop_hw_queue(struct blk_mq_hw_ctx *hctx)
> cancel_delayed_work(&hctx->run_work);
>
> set_bit(BLK_MQ_S_STOPPED, &hctx->state);
> + clear_bit(BLK_MQ_S_WORK_QUEUED, &hctx->state);
> }
> EXPORT_SYMBOL(blk_mq_stop_hw_queue);
>
> @@ -1626,6 +1637,7 @@ static void blk_mq_run_work_fn(struct work_struct *work)
> struct blk_mq_hw_ctx *hctx;
>
> hctx = container_of(work, struct blk_mq_hw_ctx, run_work.work);
> + clear_bit(BLK_MQ_S_WORK_QUEUED, &hctx->state);
>
> /*
> * If we are stopped, don't run the queue.
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 0bf056de5cc3..98269d3fd141 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -234,6 +234,7 @@ enum {
> BLK_MQ_S_STOPPED = 0,
> BLK_MQ_S_TAG_ACTIVE = 1,
> BLK_MQ_S_SCHED_RESTART = 2,
> + BLK_MQ_S_WORK_QUEUED = 3,
>
> BLK_MQ_MAX_DEPTH = 10240,
>
>
--
Damien Le Moal
Western Digital Research
