Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 7 Sep 2021 22:29:01 +0800
From:   Ming Lei <ming.lei@redhat.com>
To:     Niklas Cassel <Niklas.Cassel@wdc.com>
Cc:     Jens Axboe <axboe@kernel.dk>, Bart Van Assche <bvanassche@acm.org>,
        Damien Le Moal <Damien.LeMoal@wdc.com>,
        Paolo Valente <paolo.valente@linaro.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] blk-mq: don't call callbacks for requests that
 bypassed the scheduler
Message-ID: <YTd3LRI8A7K+Ctin@T590>
References: <20210907142145.112096-1-Niklas.Cassel@wdc.com>
 <20210907142145.112096-2-Niklas.Cassel@wdc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210907142145.112096-2-Niklas.Cassel@wdc.com>
Precedence: bulk

On Tue, Sep 07, 2021 at 02:21:55PM +0000, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Currently, __blk_mq_alloc_request() (via blk_mq_rq_ctx_init()) calls the
> I/O scheduler callback e->type->ops.prepare_request(), which will set
> RQF_ELVPRIV, even though passthrough (and flush) requests will later
> bypass the I/O scheduler in blk_mq_submit_bio().
> 
> Later, blk_mq_free_request() checks if the RQF_ELVPRIV flag is set,
> if it is, the e->type->ops.finish_request() I/O scheduler callback
> will be called.
> 
> i.e., the prepare_request and finish_request I/O scheduler callbacks
> will be called for requests which were never inserted to the I/O
> scheduler.
> 
> Fix this by not calling e->type->ops.prepare_request(), nor setting
> the RQF_ELVPRIV flag for passthrough requests.
> Since the RQF_ELVPRIV flag will not get set for passthrough requests,
> e->type->ops.prepare_request() will no longer get called for
> passthrough requests which were never inserted to the I/O scheduler.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  block/blk-mq.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 65d3a63aecc6..0816af125059 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -328,7 +328,12 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data,
>  	data->ctx->rq_dispatched[op_is_sync(data->cmd_flags)]++;
>  	refcount_set(&rq->ref, 1);
>  
> -	if (!op_is_flush(data->cmd_flags)) {
> +	/*
> +	 * Flush/passthrough requests are special and go directly to the
> +	 * dispatch list, bypassing the scheduler.
> +	 */
> +	if (!op_is_flush(data->cmd_flags) &&
> +	    !blk_op_is_passthrough(data->cmd_flags)) {

Looks fine:

Reviewed-by: Ming Lei <ming.lei@redhat.com>

-- 
Ming