On 08/30/2017 09:19 AM, Ming Lei wrote:
> It is more reasonable to add requests to ->dispatch in way
> of FIFO style, instead of LIFO style.
> 
> Also in this way, we can allow to insert request at the front
> of hw queue, which function is needed to fix one bug
> in blk-mq's implementation of blk_execute_rq()
> 
> Reported-by: Oleksandr Natalenko <[email protected]>
> Tested-by: Oleksandr Natalenko <[email protected]>
> Signed-off-by: Ming Lei <[email protected]>
> ---
>  block/blk-mq-sched.c | 2 +-
>  block/blk-mq.c       | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> index 4ab69435708c..8d97df40fc28 100644
> --- a/block/blk-mq-sched.c
> +++ b/block/blk-mq-sched.c
> @@ -272,7 +272,7 @@ static bool blk_mq_sched_bypass_insert(struct 
> blk_mq_hw_ctx *hctx,
>        * the dispatch list.
>        */
>       spin_lock(&hctx->lock);
> -     list_add(&rq->queuelist, &hctx->dispatch);
> +     list_add_tail(&rq->queuelist, &hctx->dispatch);
>       spin_unlock(&hctx->lock);
>       return true;
>  }
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 4603b115e234..fed3d0c16266 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1067,7 +1067,7 @@ bool blk_mq_dispatch_rq_list(struct request_queue *q, 
> struct list_head *list)
>               blk_mq_put_driver_tag(rq);
>  
>               spin_lock(&hctx->lock);
> -             list_splice_init(list, &hctx->dispatch);
> +             list_splice_tail_init(list, &hctx->dispatch);
>               spin_unlock(&hctx->lock);

I'm not convinced this is safe, there's actually a reason why the
request is added to the front and not the back. We do have
reorder_tags_to_front() as a safe guard, but I'd much rather get rid of
that than make this change.

What's your reasoning here? Your changelog doesn't really explain why
this fixes anything, it's very vague.

-- 
Jens Axboe

Reply via email to