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
