On Thu, Nov 30, 2017 at 04:08:46PM -0800, Bart Van Assche wrote:
> The blk_mq_sched_restart() call from inside blk_mq_free_request()
> only runs those queues for which BLK_MQ_S_SCHED_RESTART has been
> set. Hence set that flag from inside blk_mq_mark_tag_wait() whether
> or not a queue is shared.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxx>
> Cc: Ming Lei <ming.lei@xxxxxxxxxx>
> Cc: Omar Sandoval <osandov@xxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Hannes Reinecke <hare@xxxxxxx>
> Cc: Johannes Thumshirn <jthumshirn@xxxxxxx>
> ---
> block/blk-mq-sched.c | 2 +-
> block/blk-mq.c | 4 ++--
> block/blk-mq.h | 2 ++
> 3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> index c4e0cb5f6f1f..398545d94521 100644
> --- a/block/blk-mq-sched.c
> +++ b/block/blk-mq-sched.c
> @@ -54,7 +54,7 @@ void blk_mq_sched_assign_ioc(struct request *rq, struct bio *bio)
> * Mark a hardware queue as needing a restart. For shared queues, maintain
> * a count of how many hardware queues are marked for restart.
> */
> -static void blk_mq_sched_mark_restart_hctx(struct blk_mq_hw_ctx *hctx)
> +void blk_mq_sched_mark_restart_hctx(struct blk_mq_hw_ctx *hctx)
> {
> if (test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state))
> return;
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 26fec4dfa40f..3e0ce940377f 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1036,9 +1036,9 @@ static bool blk_mq_mark_tag_wait(struct blk_mq_hw_ctx **hctx,
> wait_queue_entry_t *wait;
> bool ret;
>
> + blk_mq_sched_mark_restart_hctx(this_hctx);
> +
> if (!shared_tags) {
> - if (!test_bit(BLK_MQ_S_SCHED_RESTART, &this_hctx->state))
> - set_bit(BLK_MQ_S_SCHED_RESTART, &this_hctx->state);
On the contrary, the above two lines should be removed simply, because
this bit has to be set after the req is added to hctx->dispatch_list,
otherwise the RESTART for this rq may be missed. Seems it is a issue
introduced by f906a6a0f4268(blk-mq: improve tag waiting setup for
non-shared tags).
And the bit can be set in blk_mq_sched_dispatch_requests() if the
following get_driver_tag fails.
> ret = blk_mq_get_driver_tag(rq, hctx, false);
> /*
> * Don't clear RESTART here, someone else could have set it.
> diff --git a/block/blk-mq.h b/block/blk-mq.h
> index 6c7c3ff5bf62..eb3c93aeb8b3 100644
> --- a/block/blk-mq.h
> +++ b/block/blk-mq.h
> @@ -139,6 +139,8 @@ static inline bool blk_mq_hw_queue_mapped(struct blk_mq_hw_ctx *hctx)
> void blk_mq_in_flight(struct request_queue *q, struct hd_struct *part,
> unsigned int inflight[2]);
>
> +void blk_mq_sched_mark_restart_hctx(struct blk_mq_hw_ctx *hctx);
> +
> static inline void blk_mq_put_dispatch_budget(struct blk_mq_hw_ctx *hctx)
> {
> struct request_queue *q = hctx->queue;
> --
> 2.15.0
>
--
Ming
