[PATCH 0/3] few small nvmet fixes/cleanups

[PATCH 0/3] few small nvmet fixes/cleanups

[Sagi Grimberg]

Just some useful patches for 4.15

Sagi Grimberg (3):
  nvmet-rdma: removed queues cleanup from module exit
  nvmet: remove redundant memset if failed to get_smart_log failed
  nvmet: remove redundant local variable

 drivers/nvme/target/admin-cmd.c | 17 +++++------------
 drivers/nvme/target/rdma.c      | 17 -----------------
 2 files changed, 5 insertions(+), 29 deletions(-)

-- 
2.14.1

[PATCH 1/3] nvmet-rdma: removed queues cleanup from module exit

[Sagi Grimberg]

We already do that when we are notified in device removal
which is triggered when unregistering as an ib client.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/target/rdma.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 98d7f2ded511..efcdc4991359 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -1537,24 +1537,7 @@ static int __init nvmet_rdma_init(void)
 
 static void __exit nvmet_rdma_exit(void)
 {
-	struct nvmet_rdma_queue *queue;
-
 	nvmet_unregister_transport(&nvmet_rdma_ops);
-
-	flush_scheduled_work();
-
-	mutex_lock(&nvmet_rdma_queue_mutex);
-	while ((queue = list_first_entry_or_null(&nvmet_rdma_queue_list,
-			struct nvmet_rdma_queue, queue_list))) {
-		list_del_init(&queue->queue_list);
-
-		mutex_unlock(&nvmet_rdma_queue_mutex);
-		__nvmet_rdma_queue_disconnect(queue);
-		mutex_lock(&nvmet_rdma_queue_mutex);
-	}
-	mutex_unlock(&nvmet_rdma_queue_mutex);
-
-	flush_scheduled_work();
 	ib_unregister_client(&nvmet_rdma_ib_client);
 	ida_destroy(&nvmet_rdma_queue_ida);
 }
-- 
2.14.1

[PATCH 1/3] nvmet-rdma: removed queues cleanup from module exit

[Christoph Hellwig]

On Wed, Nov 08, 2017@12:00:28PM +0200, Sagi Grimberg wrote:
> We already do that when we are notified in device removal
> which is triggered when unregistering as an ib client.
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>  drivers/nvme/target/rdma.c | 17 -----------------
>  1 file changed, 17 deletions(-)
> 
> diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
> index 98d7f2ded511..efcdc4991359 100644
> --- a/drivers/nvme/target/rdma.c
> +++ b/drivers/nvme/target/rdma.c
> @@ -1537,24 +1537,7 @@ static int __init nvmet_rdma_init(void)
>  
>  static void __exit nvmet_rdma_exit(void)
>  {
> -	struct nvmet_rdma_queue *queue;
> -
>  	nvmet_unregister_transport(&nvmet_rdma_ops);
> -
> -	flush_scheduled_work();
> -
> -	mutex_lock(&nvmet_rdma_queue_mutex);
> -	while ((queue = list_first_entry_or_null(&nvmet_rdma_queue_list,
> -			struct nvmet_rdma_queue, queue_list))) {
> -		list_del_init(&queue->queue_list);
> -
> -		mutex_unlock(&nvmet_rdma_queue_mutex);
> -		__nvmet_rdma_queue_disconnect(queue);
> -		mutex_lock(&nvmet_rdma_queue_mutex);
> -	}
> -	mutex_unlock(&nvmet_rdma_queue_mutex);
> -
> -	flush_scheduled_work();

Should we thrown in an assert here that the queue really is empty?

Otherwise this looks good to me:

Reviewed-by: Christoph Hellwig <hch at lst.de>

[PATCH 2/3] nvmet: remove redundant memset if failed to get_smart_log failed

[Sagi Grimberg]

We already allocated the buffer with kzalloc.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/target/admin-cmd.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 9b02a6bd58d6..871c2e5e17a0 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -144,10 +144,8 @@ static void nvmet_execute_get_log_page(struct nvmet_req *req)
 		}
 		smart_log = buf;
 		status = nvmet_get_smart_log(req, smart_log);
-		if (status) {
-			memset(buf, '\0', data_len);
+		if (status)
 			goto err;
-		}
 		break;
 	case NVME_LOG_FW_SLOT:
 		/*
-- 
2.14.1

[PATCH 2/3] nvmet: remove redundant memset if failed to get_smart_log failed

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch at lst.de>

[PATCH 2/3] nvmet: remove redundant memset if failed to get_smart_log failed

[Christoph Hellwig]

I've applied patches 2 and 3 to nvme-4.15.

[PATCH 3/3] nvmet: remove redundant local variable

[Sagi Grimberg]

the status is either success or some status id and
we don't need a local variable for it.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/target/admin-cmd.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 871c2e5e17a0..6b921c253618 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -35,17 +35,14 @@ u32 nvmet_get_log_page_len(struct nvme_command *cmd)
 static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req,
 		struct nvme_smart_log *slog)
 {
-	u16 status;
 	struct nvmet_ns *ns;
 	u64 host_reads, host_writes, data_units_read, data_units_written;
 
-	status = NVME_SC_SUCCESS;
 	ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->get_log_page.nsid);
 	if (!ns) {
-		status = NVME_SC_INVALID_NS;
 		pr_err("nvmet : Could not find namespace id : %d\n",
 				le32_to_cpu(req->cmd->get_log_page.nsid));
-		goto out;
+		return NVME_SC_INVALID_NS;
 	}
 
 	host_reads = part_stat_read(ns->bdev->bd_part, ios[READ]);
@@ -58,20 +55,18 @@ static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req,
 	put_unaligned_le64(host_writes, &slog->host_writes[0]);
 	put_unaligned_le64(data_units_written, &slog->data_units_written[0]);
 	nvmet_put_namespace(ns);
-out:
-	return status;
+
+	return NVME_SC_SUCCESS;
 }
 
 static u16 nvmet_get_smart_log_all(struct nvmet_req *req,
 		struct nvme_smart_log *slog)
 {
-	u16 status;
 	u64 host_reads = 0, host_writes = 0;
 	u64 data_units_read = 0, data_units_written = 0;
 	struct nvmet_ns *ns;
 	struct nvmet_ctrl *ctrl;
 
-	status = NVME_SC_SUCCESS;
 	ctrl = req->sq->ctrl;
 
 	rcu_read_lock();
@@ -91,7 +86,7 @@ static u16 nvmet_get_smart_log_all(struct nvmet_req *req,
 	put_unaligned_le64(host_writes, &slog->host_writes[0]);
 	put_unaligned_le64(data_units_written, &slog->data_units_written[0]);
 
-	return status;
+	return NVME_SC_SUCCESS;
 }
 
 static u16 nvmet_get_smart_log(struct nvmet_req *req,
-- 
2.14.1

[PATCH 3/3] nvmet: remove redundant local variable

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch at lst.de>