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;
To:     Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
Cc:     Christoph Hellwig <hch@lst.de>, Sagi Grimberg <sagi@grimberg.me>,
        Keith Busch <kbusch@kernel.org>, Jens Axboe <axboe@fb.com>,
        Max Gurtovoy <maxg@mellanox.com>,
        Stephen Bates <sbates@raithlin.com>
References: <20200514172253.6445-1-logang@deltatee.com>
 <20200514172253.6445-8-logang@deltatee.com>
 <BYAPR04MB4965379CE8694F52D684993386800@BYAPR04MB4965.namprd04.prod.outlook.com>
From:   Logan Gunthorpe <logang@deltatee.com>
Message-ID: <cbe637ec-e80c-def1-2453-70059715e19c@deltatee.com>
Date:   Thu, 11 Jun 2020 17:46:54 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.0
MIME-Version: 1.0
In-Reply-To: <BYAPR04MB4965379CE8694F52D684993386800@BYAPR04MB4965.namprd04.prod.outlook.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH v13 7/9] nvmet-passthru: Add passthru code to process
 commands
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 2020-06-11 5:45 p.m., Chaitanya Kulkarni wrote:
> On 5/14/20 10:23 AM, Logan Gunthorpe wrote:
>> +
>> +static void nvmet_passthru_execute_cmd(struct nvmet_req *req)
>> +{
>> +	struct nvme_ctrl *ctrl = nvmet_req_passthru_ctrl(req);
>> +	struct nvme_ns *ns = NULL;
>> +	struct request *rq = NULL;
>> +	struct request_queue *q;
>> +	u32 effects;
>> +	u16 status;
>> +	int ret;
>> +
>> +	if (likely(req->sq->qid != 0)) {
>> +		u32 nsid = le32_to_cpu(req->cmd->common.nsid);
>> +
>> +		ns = nvme_find_get_ns(ctrl, nsid);
>> +		if (unlikely(!ns)) {
>> +			pr_err("failed to get passthru ns nsid:%u\n", nsid);
>> +			status = NVME_SC_INVALID_NS | NVME_SC_DNR;
>> +			goto fail_out;
>> +		}
>> +	}
>> +
>> +	if (ns)
>> +		q = ns->queue;
>> +	else
>> +		q = ctrl->admin_q;
> 
> Is it possible to avoid explicit if.. else in fast path given that the
> condition exists and we can take an advantage of that, how about
> following ?
> 
> diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
> index 9e71fdfbdbb3..4f8e022d254c 100644
> --- a/drivers/nvme/target/passthru.c
> +++ b/drivers/nvme/target/passthru.c
> @@ -84,9 +84,9 @@ static int nvmet_passthru_map_sg(struct nvmet_req 
> *req, struct request *rq)
>   static void nvmet_passthru_execute_cmd(struct nvmet_req *req)
>   {
>          struct nvme_ctrl *ctrl = nvmet_req_passthru_ctrl(req);
> +       struct request_queue *q = ctrl->admin_q;
>          struct nvme_ns *ns = NULL;
>          struct request *rq = NULL;
> -       struct request_queue *q;
>          u32 effects;
>          u16 status;
>          int ret;
> @@ -100,12 +100,8 @@ static void nvmet_passthru_execute_cmd(struct 
> nvmet_req *req)
>                          status = NVME_SC_INVALID_NS | NVME_SC_DNR;
>                          goto fail_out;
>                  }
> -       }
> -
> -       if (ns)
>                  q = ns->queue;
> -       else
> -               q = ctrl->admin_q;
> +       }
> 
>          rq = nvme_alloc_request(q, req->cmd, BLK_MQ_REQ_NOWAIT, 
> NVME_QID_ANY);
>          if (IS_ERR(rq)) {
> 


This makes sense. Will fix for the next revision.

Thanks,

Logan