lkml.org 
[lkml]   [2017]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V5 1/7] blk-mq: issue rq directly in blk_mq_request_bypass_insert()
This patch does two many things at once and needs a split. I also
don't really understand why it's in this series and not your dm-mpath
performance one.

> +static void blk_mq_request_direct_insert(struct blk_mq_hw_ctx *hctx,
> + struct request *rq)
> +{
> + spin_lock(&hctx->lock);
> + list_add_tail(&rq->queuelist, &hctx->dispatch);
> + spin_unlock(&hctx->lock);
> +
> + blk_mq_run_hw_queue(hctx, false);
> +}

Why doesn't this share code with blk_mq_sched_bypass_insert?

> /*
> * Should only be used carefully, when the caller knows we want to
> * bypass a potential IO scheduler on the target device.
> */
> -void blk_mq_request_bypass_insert(struct request *rq)
> +blk_status_t blk_mq_request_bypass_insert(struct request *rq)
> {
> struct blk_mq_ctx *ctx = rq->mq_ctx;
> struct blk_mq_hw_ctx *hctx = blk_mq_map_queue(rq->q, ctx->cpu);
> + blk_qc_t cookie;
> + blk_status_t ret;
>
> - spin_lock(&hctx->lock);
> - list_add_tail(&rq->queuelist, &hctx->dispatch);
> - spin_unlock(&hctx->lock);
> -
> - blk_mq_run_hw_queue(hctx, false);
> + ret = blk_mq_try_issue_directly(hctx, rq, &cookie, true);
> + if (ret == BLK_STS_RESOURCE)
> + blk_mq_request_direct_insert(hctx, rq);
> + return ret;

If you actually insert the request on BLK_STS_RESOURCE why do you
pass the error on? In general BLK_STS_RESOURCE indicates a failure
to issue.

> +/*
> + * 'dispatch_only' means we only try to dispatch it out, and
> + * don't deal with dispatch failure if BLK_STS_RESOURCE or
> + * BLK_STS_IOERR happens.
> + */
> +static blk_status_t __blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx,
> + struct request *rq, blk_qc_t *cookie, bool may_sleep,
> + bool dispatch_only)

This dispatch_only argument that completely changes behavior is a
nightmare. Try to find a way to have a low-level helper that
always behaves as if dispatch_only is set, and then build another
helper that actually issues/completes around it.

\
 
 \ /
  Last update: 2017-10-03 10:59    [W:0.023 / U:1.248 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site