[PATCH v2 0/2] kref and passthrough cleanup

[PATCH v2 0/2] kref and passthrough cleanup

[Kanchan Joshi]

first patch is to avoid opencoding ns->kref, while second one is to remove
double memset in passthough io path.

since v1:
- (patch 2) move clearing-cmd part to nvme_setup_cmd (Christoph's feedback)

Kanchan Joshi (2):
  nvme: add nvme_get_ns helper
  nvme: avoid memset for passthrough requests

 drivers/nvme/host/core.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

-- 
2.25.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH v2 1/2] nvme: add nvme_get_ns helper

[Kanchan Joshi]

Add a helper to avoid opencoding ns->kref increment.
Decrement is already done via nvme_put_ns helper.

Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
---
 drivers/nvme/host/core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 2f45e8fcdd7c..f6d7c397f2ee 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -576,6 +576,11 @@ static void nvme_free_ns(struct kref *kref)
 	kfree(ns);
 }
 
+static inline bool nvme_get_ns(struct nvme_ns *ns)
+{
+	return kref_get_unless_zero(&ns->kref);
+}
+
 void nvme_put_ns(struct nvme_ns *ns)
 {
 	kref_put(&ns->kref, nvme_free_ns);
@@ -1504,7 +1509,7 @@ static int nvme_ns_open(struct nvme_ns *ns)
 	/* should never be called due to GENHD_FL_HIDDEN */
 	if (WARN_ON_ONCE(nvme_ns_head_multipath(ns->head)))
 		goto fail;
-	if (!kref_get_unless_zero(&ns->kref))
+	if (!nvme_get_ns(ns))
 		goto fail;
 	if (!try_module_get(ns->ctrl->ops->module))
 		goto fail_put_ns;
@@ -3613,7 +3618,7 @@ struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 	down_read(&ctrl->namespaces_rwsem);
 	list_for_each_entry(ns, &ctrl->namespaces, list) {
 		if (ns->head->ns_id == nsid) {
-			if (!kref_get_unless_zero(&ns->kref))
+			if (!nvme_get_ns(ns))
 				continue;
 			ret = ns;
 			break;
-- 
2.25.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH v2 2/2] nvme: avoid memset for passthrough requests

[Kanchan Joshi]

nvme_clear_nvme_request() clears the nvme_command, which is unncessary for
passthrough requests as nvme_command is overwritten immediately.
Move clearing part from this helper to the caller, so that double memset
for passthrough requests is avoided.

Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
---
 drivers/nvme/host/core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f6d7c397f2ee..a380eb3847d6 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -589,9 +589,6 @@ EXPORT_SYMBOL_NS_GPL(nvme_put_ns, NVME_TARGET_PASSTHRU);
 
 static inline void nvme_clear_nvme_request(struct request *req)
 {
-	struct nvme_command *cmd = nvme_req(req)->cmd;
-
-	memset(cmd, 0, sizeof(*cmd));
 	nvme_req(req)->retries = 0;
 	nvme_req(req)->flags = 0;
 	req->rq_flags |= RQF_DONTPREP;
@@ -903,8 +900,10 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req)
 	struct nvme_command *cmd = nvme_req(req)->cmd;
 	blk_status_t ret = BLK_STS_OK;
 
-	if (!(req->rq_flags & RQF_DONTPREP))
+	if (!(req->rq_flags & RQF_DONTPREP)) {
 		nvme_clear_nvme_request(req);
+		memset(cmd, 0, sizeof(struct nvme_command));
+	}
 
 	switch (req_op(req)) {
 	case REQ_OP_DRV_IN:
-- 
2.25.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH v2 1/2] nvme: add nvme_get_ns helper

[Chaitanya Kulkarni]

On 4/22/21 05:14, Kanchan Joshi wrote:
> Add a helper to avoid opencoding ns->kref increment.
> Decrement is already done via nvme_put_ns helper.
>
> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH v2 2/2] nvme: avoid memset for passthrough requests

[Chaitanya Kulkarni]

On 4/22/21 05:18, Kanchan Joshi wrote:
> +		memset(cmd, 0, sizeof(struct nvme_command));

The original code uses following :-

-	memset(cmd, 0, sizeof(*cmd));


Let's not change that.



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH v2 2/2] nvme: avoid memset for passthrough requests

[Kanchan Joshi]

On Fri, Apr 23, 2021 at 12:25 AM Chaitanya Kulkarni
<Chaitanya.Kulkarni@wdc.com> wrote:
>
> On 4/22/21 05:18, Kanchan Joshi wrote:
> > +             memset(cmd, 0, sizeof(struct nvme_command));
>
> The original code uses following :-
>
> -       memset(cmd, 0, sizeof(*cmd));
>
>
> Let's not change that.

Ok, I'll change and repost.

-- 
Joshi

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH v2 2/2] nvme: avoid memset for passthrough requests

[Chaitanya Kulkarni]

On 4/22/21 05:18, Kanchan Joshi wrote:
> nvme_clear_nvme_request() clears the nvme_command, which is unncessary for

The above line seems longer, please check.

> passthrough requests as nvme_command is overwritten immediately.
> Move clearing part from this helper to the caller, so that double memset
> for passthrough requests is avoided.
>
> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>




_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme