linux-kernel - Re: [PATCH v13 7/9] nvmet-passthru: Add passthru code to process commands

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BYAPR04MB4965379CE8694F52D684993386800@BYAPR04MB4965.namprd04.prod.outlook.com>
Date:   Thu, 11 Jun 2020 23:45:10 +0000
From:   Chaitanya Kulkarni <Chaitanya.Kulkarni@....com>
To:     Logan Gunthorpe <logang@...tatee.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>
CC:     Christoph Hellwig <hch@....de>, Sagi Grimberg <sagi@...mberg.me>,
        Keith Busch <kbusch@...nel.org>, Jens Axboe <axboe@...com>,
        Max Gurtovoy <maxg@...lanox.com>,
        Stephen Bates <sbates@...thlin.com>
Subject: Re: [PATCH v13 7/9] nvmet-passthru: Add passthru code to process
 commands

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)) {