Re: [PATCH 3/4] blk-mq: establish new mapping before cpu starts handling requests

[Akinobu Mita]

2015-06-26 0:40 GMT+09:00 Ming Lei <tom.leiming@xxxxxxxxx>:
> On Thu, 25 Jun 2015 21:49:43 +0900
> Akinobu Mita <akinobu.mita@xxxxxxxxx> wrote:
>> For example, there is a single hw queue (hctx) and two CPU queues
>> (ctx0 for CPU0, and ctx1 for CPU1).  Now CPU1 is just onlined and
>> a request is inserted into ctx1->rq_list and set bit0 in pending
>> bitmap as ctx1->index_hw is still zero.
>>
>> And then while running hw queue, flush_busy_ctxs() finds bit0 is set
>> in pending bitmap and tries to retrieve requests in
>> hctx->ctxs[0].rq_list.  But htx->ctxs[0] is ctx0, so the request in
>> ctx1->rq_list is ignored.
>
> Per current design, the request should have been inserted into ctx0 instead
> of ctx1 because ctx1 isn't mapped yet even though ctx1->cpu becomes ONLINE.
>
> So how about the following patch? which looks much simpler.

OK, I'll try this patch to see if the problem disappears.

> ---
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index f537796..2f45b73 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1034,7 +1034,12 @@ void blk_mq_insert_request(struct request *rq, bool at_head, bool run_queue,
>         struct blk_mq_ctx *ctx = rq->mq_ctx, *current_ctx;
>
>         current_ctx = blk_mq_get_ctx(q);
> -       if (!cpu_online(ctx->cpu))
> +       /*
> +        * ctx->cpu may become ONLINE but ctx hasn't been mapped to
> +        * hctx yet because there is a tiny race window between
> +        * ctx->cpu ONLINE and doing the remap
> +        */
> +       if (!blk_mq_ctx_mapped(ctx))
>                 rq->mq_ctx = ctx = current_ctx;
>
>         hctx = q->mq_ops->map_queue(q, ctx->cpu);
> @@ -1063,7 +1068,7 @@ static void blk_mq_insert_requests(struct request_queue *q,
>
>         current_ctx = blk_mq_get_ctx(q);
>
> -       if (!cpu_online(ctx->cpu))
> +       if (!blk_mq_ctx_mapped(ctx))
>                 ctx = current_ctx;
>         hctx = q->mq_ops->map_queue(q, ctx->cpu);
>
> @@ -1816,13 +1821,16 @@ static void blk_mq_map_swqueue(struct request_queue *q)
>          */
>         queue_for_each_ctx(q, ctx, i) {
>                 /* If the cpu isn't online, the cpu is mapped to first hctx */
> -               if (!cpu_online(i))
> +               if (!cpu_online(i)) {
> +                       ctx->mapped = 0;
>                         continue;
> +               }
>
>                 hctx = q->mq_ops->map_queue(q, i);
>                 cpumask_set_cpu(i, hctx->cpumask);
>                 cpumask_set_cpu(i, hctx->tags->cpumask);
>                 ctx->index_hw = hctx->nr_ctx;
> +               ctx->mapped = 1;
>                 hctx->ctxs[hctx->nr_ctx++] = ctx;
>         }
>
> diff --git a/block/blk-mq.h b/block/blk-mq.h
> index 6a48c4c..52819ad 100644
> --- a/block/blk-mq.h
> +++ b/block/blk-mq.h
> @@ -10,7 +10,8 @@ struct blk_mq_ctx {
>         }  ____cacheline_aligned_in_smp;
>
>         unsigned int            cpu;
> -       unsigned int            index_hw;
> +       unsigned int            index_hw : 16;
> +       unsigned int            mapped : 1;
>
>         unsigned int            last_tag ____cacheline_aligned_in_smp;
>
> @@ -123,4 +124,9 @@ static inline bool blk_mq_hw_queue_mapped(struct blk_mq_hw_ctx *hctx)
>         return hctx->nr_ctx && hctx->tags;
>  }
>
> +static inline bool blk_mq_ctx_mapped(struct blk_mq_ctx *ctx)
> +{
> +       return ctx->mapped;
> +}
> +
>  #endif
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/