RE: [PATCH RFC v7 00/12] blk-mq/scsi: Provide hostwide shared tags for SCSI HBAs

Kashyap Desai
2020-06-23T12:11:26Z

> > > >
> > > Would it make sense to move it into the elevator itself?
>
> I am not sure where exactly I should add this counter since I need
counter per
> hctx. Elevator data is per request object.
> Please suggest.
>
> >
> > That is my initial suggestion, and the counter is just done for bfq &
> > mq- deadline, then we needn't to pay the cost for others.
>
> I have updated patch -
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index
a1123d4..3e0005c
> 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -4640,6 +4640,12 @@ static bool bfq_has_work(struct blk_mq_hw_ctx
> *hctx) {
> struct bfq_data *bfqd = hctx->queue->elevator->elevator_data;
>
> + /* If current hctx has not queued any request, there is no need
to run.
> + * blk_mq_run_hw_queue() on hctx which has queued IO will handle
> + * running specific hctx.
> + */
> + if (!atomic_read(&hctx->elevator_queued))
> + return false;
> /*
> * Avoiding lock: a race on bfqd->busy_queues should cause at
> * most a call to dispatch for nothing @@ -5554,6 +5561,7 @@
static void
> bfq_insert_requests(struct blk_mq_hw_ctx *hctx,
> rq = list_first_entry(list, struct request, queuelist);
> list_del_init(&rq->queuelist);
> bfq_insert_request(hctx, rq, at_head);
> + atomic_inc(&hctx->elevator_queued);
> }
> }
>
> @@ -5925,6 +5933,7 @@ static void bfq_finish_requeue_request(struct
> request *rq)
>
> if (likely(rq->rq_flags & RQF_STARTED)) {
> unsigned long flags;
> + struct blk_mq_hw_ctx *mq_hctx = rq->mq_hctx;
>
> spin_lock_irqsave(&bfqd->lock, flags);
>
> @@ -5934,6 +5943,7 @@ static void bfq_finish_requeue_request(struct
> request *rq)
> bfq_completed_request(bfqq, bfqd);
> bfq_finish_requeue_request_body(bfqq);
>
> + atomic_dec(&hctx->elevator_queued);
> spin_unlock_irqrestore(&bfqd->lock, flags);
> } else {
> /*
> diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h index
> 126021f..946b47a 100644
> --- a/block/blk-mq-sched.h
> +++ b/block/blk-mq-sched.h
> @@ -74,6 +74,13 @@ static inline bool blk_mq_sched_has_work(struct
> blk_mq_hw_ctx *hctx) {
> struct elevator_queue *e = hctx->queue->elevator;
>
> + /* If current hctx has not queued any request, there is no need
to run.
> + * blk_mq_run_hw_queue() on hctx which has queued IO will handle
> + * running specific hctx.
> + */
> + if (!atomic_read(&hctx->elevator_queued))
> + return false;
> +

I have missed this. I will remove above code since it is now managed
within mq-deadline and bfq-iosched *has_work* callback.

> if (e && e->type->ops.has_work)
> return e->type->ops.has_work(hctx);
>