On Fri, Dec 18, 2020 at 10:44:12PM +0100, Jan Kara wrote:
> Currently when non-mq aware IO scheduler (BFQ, mq-deadline) is used for
> a queue with multiple HW queues, the performance it rather bad. The
> problem is that these IO schedulers use queue-wide locking and their
> dispatch function does not respect the hctx it is passed in and returns
> any request it finds appropriate. Thus locality of request access is
> broken and dispatch from multiple CPUs just contends on IO scheduler
> locks. For these IO schedulers there's little point in dispatching from
> multiple CPUs. Instead dispatch always only from a single CPU to limit
> contention.
>
> Below is a comparison of dbench runs on XFS filesystem where the storage
> is a raid card with 64 HW queues and to it attached a single rotating
> disk. BFQ is used as IO scheduler:
>
> clients MQ SQ MQ-Patched
> Amean 1 39.12 (0.00%) 43.29 * -10.67%* 36.09 * 7.74%*
> Amean 2 128.58 (0.00%) 101.30 * 21.22%* 96.14 * 25.23%*
> Amean 4 577.42 (0.00%) 494.47 * 14.37%* 508.49 * 11.94%*
> Amean 8 610.95 (0.00%) 363.86 * 40.44%* 362.12 * 40.73%*
> Amean 16 391.78 (0.00%) 261.49 * 33.25%* 282.94 * 27.78%*
> Amean 32 324.64 (0.00%) 267.71 * 17.54%* 233.00 * 28.23%*
> Amean 64 295.04 (0.00%) 253.02 * 14.24%* 242.37 * 17.85%*
> Amean 512 10281.61 (0.00%) 10211.16 * 0.69%* 10447.53 * -1.61%*
>
> Numbers are times so lower is better. MQ is stock 5.10-rc6 kernel. SQ is
> the same kernel with megaraid_sas.host_tagset_enable=0 so that the card
> advertises just a single HW queue. MQ-Patched is a kernel with this
> patch applied.
>
> You can see multiple hardware queues heavily hurt performance in
> combination with BFQ. The patch restores the performance.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
> block/blk-mq.c | 37 +++++++++++++++++++++++++++++++++++++
> block/kyber-iosched.c | 1 +
> include/linux/elevator.h | 2 ++
> 3 files changed, 40 insertions(+)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 57d0461f2be5..6d80054c231b 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1663,6 +1663,31 @@ void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async)
> }
> EXPORT_SYMBOL(blk_mq_run_hw_queue);
>
> +static struct blk_mq_hw_ctx *blk_mq_sq_iosched_hctx(struct request_queue *q)
> +{
> + struct elevator_queue *e = q->elevator;
> + struct blk_mq_hw_ctx *hctx;
> +
> + /*
> + * The queue has multiple hardware queues but uses IO scheduler that
> + * does not respect hardware queues when dispatching? This is not a
> + * great setup but it can be sensible when we have a single rotational
> + * disk behind a raid card. Just don't bother with multiple HW queues
> + * and dispatch from hctx for the current CPU since running multiple
> + * queues just causes lock contention inside the scheduler and
> + * pointless cache bouncing because the hctx is not respected by the IO
> + * scheduler's dispatch function anyway.
> + */
> + if (q->nr_hw_queues > 1 && e && e->type->ops.dispatch_request &&
> + !(e->type->elevator_features & ELEVATOR_F_MQ_AWARE)) {
> + hctx = blk_mq_map_queue_type(q, HCTX_TYPE_DEFAULT,
> + raw_smp_processor_id());
> + if (!blk_mq_hctx_stopped(hctx))
> + return hctx;
> + }
> + return NULL;
> +}
> +
> /**
> * blk_mq_run_hw_queues - Run all hardware queues in a request queue.
> * @q: Pointer to the request queue to run.
> @@ -1673,6 +1698,12 @@ void blk_mq_run_hw_queues(struct request_queue *q, bool async)
> struct blk_mq_hw_ctx *hctx;
> int i;
>
> + hctx = blk_mq_sq_iosched_hctx(q);
> + if (hctx) {
> + blk_mq_run_hw_queue(hctx, async);
> + return;
> + }
> +
This approach looks reasonable, just wondering which code path is wrt.
blk_mq_run_hw_queues() improvement by this patch.
Since ed5dd6a67d5e ("scsi: core: Only re-run queue in scsi_end_request() if device
queue is busy") is merged, blk_mq_run_hw_queues() is only called from scsi_end_request()
when the scsi device is busy for megaraid.
Another one is bfq_schedule_dispatch(), in which blk_mq_run_hw_queues()
is still be called, if that is the reason, maybe it is easier to optimize
bfq_schedule_dispatch() by avoiding to call blk_mq_run_hw_queues().
thanks,
Ming
