From f720a8edbc6470fad8b47d0d4ae092a6c63340bb Mon Sep 17 00:00:00 2001 From: Jiapeng Chong Date: Tue, 2 Feb 2021 15:06:17 +0800 Subject: [PATCH 01/22] nvme: convert sysfs sprintf/snprintf family to sysfs_emit Fix the following coccicheck warning: ./drivers/nvme/host/core.c:3580:8-16: WARNING: use scnprintf or sprintf. ./drivers/nvme/host/core.c:3570:8-16: WARNING: use scnprintf or sprintf. ./drivers/nvme/host/core.c:3560:8-16: WARNING: use scnprintf or sprintf. ./drivers/nvme/host/core.c:3526:8-16: WARNING: use scnprintf or sprintf. ./drivers/nvme/host/core.c:2833:8-16: WARNING: use scnprintf or sprintf. Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong Reviewed-by: Sagi Grimberg Reviewed-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 4e8e310033c9b..0befaad788a09 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2848,7 +2848,7 @@ static ssize_t nvme_subsys_show_nqn(struct device *dev, struct nvme_subsystem *subsys = container_of(dev, struct nvme_subsystem, dev); - return snprintf(buf, PAGE_SIZE, "%s\n", subsys->subnqn); + return sysfs_emit(buf, "%s\n", subsys->subnqn); } static SUBSYS_ATTR_RO(subsysnqn, S_IRUGO, nvme_subsys_show_nqn); @@ -3541,7 +3541,7 @@ static ssize_t nvme_sysfs_show_transport(struct device *dev, { struct nvme_ctrl *ctrl = dev_get_drvdata(dev); - return snprintf(buf, PAGE_SIZE, "%s\n", ctrl->ops->name); + return sysfs_emit(buf, "%s\n", ctrl->ops->name); } static DEVICE_ATTR(transport, S_IRUGO, nvme_sysfs_show_transport, NULL); @@ -3575,7 +3575,7 @@ static ssize_t nvme_sysfs_show_subsysnqn(struct device *dev, { struct nvme_ctrl *ctrl = dev_get_drvdata(dev); - return snprintf(buf, PAGE_SIZE, "%s\n", ctrl->subsys->subnqn); + return sysfs_emit(buf, "%s\n", ctrl->subsys->subnqn); } static DEVICE_ATTR(subsysnqn, S_IRUGO, nvme_sysfs_show_subsysnqn, NULL); @@ -3585,7 +3585,7 @@ static ssize_t nvme_sysfs_show_hostnqn(struct device *dev, { struct nvme_ctrl *ctrl = dev_get_drvdata(dev); - return snprintf(buf, PAGE_SIZE, "%s\n", ctrl->opts->host->nqn); + return sysfs_emit(buf, "%s\n", ctrl->opts->host->nqn); } static DEVICE_ATTR(hostnqn, S_IRUGO, nvme_sysfs_show_hostnqn, NULL); @@ -3595,7 +3595,7 @@ static ssize_t nvme_sysfs_show_hostid(struct device *dev, { struct nvme_ctrl *ctrl = dev_get_drvdata(dev); - return snprintf(buf, PAGE_SIZE, "%pU\n", &ctrl->opts->host->id); + return sysfs_emit(buf, "%pU\n", &ctrl->opts->host->id); } static DEVICE_ATTR(hostid, S_IRUGO, nvme_sysfs_show_hostid, NULL); From 83fba8c8114748a18e20391565cfdfdf8466075c Mon Sep 17 00:00:00 2001 From: Chao Leng Date: Mon, 1 Feb 2021 11:49:38 +0800 Subject: [PATCH 02/22] blk-mq: introduce blk_mq_set_request_complete nvme drivers need to set the state of request to MQ_RQ_COMPLETE when directly complete request in queue_rq. So add blk_mq_set_request_complete. Signed-off-by: Chao Leng Signed-off-by: Christoph Hellwig --- include/linux/blk-mq.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h index aabbf6830ffc4..2c473c9b89908 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -490,6 +490,18 @@ static inline int blk_mq_request_completed(struct request *rq) return blk_mq_rq_state(rq) == MQ_RQ_COMPLETE; } +/* + * + * Set the state to complete when completing a request from inside ->queue_rq. + * This is used by drivers that want to ensure special complete actions that + * need access to the request are called on failure, e.g. by nvme for + * multipathing. + */ +static inline void blk_mq_set_request_complete(struct request *rq) +{ + WRITE_ONCE(rq->state, MQ_RQ_COMPLETE); +} + void blk_mq_start_request(struct request *rq); void blk_mq_end_request(struct request *rq, blk_status_t error); void __blk_mq_end_request(struct request *rq, blk_status_t error); From dda3248e7fc306e0ce3612ae96bdd9a36e2ab04f Mon Sep 17 00:00:00 2001 From: Chao Leng Date: Thu, 4 Feb 2021 08:55:11 +0100 Subject: [PATCH 03/22] nvme: introduce a nvme_host_path_error helper When using nvme native multipathing, if a path related error occurs during ->queue_rq, the request needs to be completed with NVME_SC_HOST_PATH_ERROR so that the request can be failed over. Introduce a helper to complete the command from ->queue_rq in a wait that invokes nvme_complete_rq. Signed-off-by: Chao Leng [hch: renamed, added a return value to clean up the callers a bit] Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 15 +++++++++++++++ drivers/nvme/host/nvme.h | 1 + 2 files changed, 16 insertions(+) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 0befaad788a09..02579f4f776c7 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -355,6 +355,21 @@ void nvme_complete_rq(struct request *req) } EXPORT_SYMBOL_GPL(nvme_complete_rq); +/* + * Called to unwind from ->queue_rq on a failed command submission so that the + * multipathing code gets called to potentially failover to another path. + * The caller needs to unwind all transport specific resource allocations and + * must return propagate the return value. + */ +blk_status_t nvme_host_path_error(struct request *req) +{ + nvme_req(req)->status = NVME_SC_HOST_PATH_ERROR; + blk_mq_set_request_complete(req); + nvme_complete_rq(req); + return BLK_STS_OK; +} +EXPORT_SYMBOL_GPL(nvme_host_path_error); + bool nvme_cancel_request(struct request *req, void *data, bool reserved) { dev_dbg_ratelimited(((struct nvme_ctrl *) data)->device, diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index a72f071810910..5819f03810414 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -575,6 +575,7 @@ static inline bool nvme_is_aen_req(u16 qid, __u16 command_id) } void nvme_complete_rq(struct request *req); +blk_status_t nvme_host_path_error(struct request *req); bool nvme_cancel_request(struct request *req, void *data, bool reserved); void nvme_cancel_tagset(struct nvme_ctrl *ctrl); void nvme_cancel_admin_tagset(struct nvme_ctrl *ctrl); From ea5e5f42cd2c80d19862dd63a2f3a4e7a99c6a20 Mon Sep 17 00:00:00 2001 From: Chao Leng Date: Mon, 1 Feb 2021 11:49:39 +0800 Subject: [PATCH 04/22] nvme-fabrics: avoid double completions in nvmf_fail_nonready_command When reconnecting, the request may be completed with NVME_SC_HOST_PATH_ERROR in nvmf_fail_nonready_command, which currently set the state of the request to MQ_RQ_IN_FLIGHT before calling nvme_complete_rq. When this happens for a request that is freed by the caller, such as nvme_submit_user_cmd, in the worst case the request could be completed again in tear down process. Instead of calling blk_mq_start_request from nvmf_fail_nonready_command, just use the new nvme_host_path_error helper to complete the command without starting it. Signed-off-by: Chao Leng Signed-off-by: Christoph Hellwig --- drivers/nvme/host/fabrics.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 72ac00173500f..5dfd806fc2d28 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -552,11 +552,7 @@ blk_status_t nvmf_fail_nonready_command(struct nvme_ctrl *ctrl, !test_bit(NVME_CTRL_FAILFAST_EXPIRED, &ctrl->flags) && !blk_noretry_request(rq) && !(rq->cmd_flags & REQ_NVME_MPATH)) return BLK_STS_RESOURCE; - - nvme_req(rq)->status = NVME_SC_HOST_PATH_ERROR; - blk_mq_start_request(rq); - nvme_complete_rq(rq); - return BLK_STS_OK; + return nvme_host_path_error(rq); } EXPORT_SYMBOL_GPL(nvmf_fail_nonready_command); From 62eca39722fd997e3621fc903229917b9f0fb271 Mon Sep 17 00:00:00 2001 From: Chao Leng Date: Mon, 1 Feb 2021 11:49:40 +0800 Subject: [PATCH 05/22] nvme-rdma: handle nvme_rdma_post_send failures better nvme_rdma_post_send failing is a path related error and should bounce to another path when using nvme-multipath. Call nvme_host_path_error when nvme_rdma_post_send returns -EIO to ensure nvme_complete_rq gets invoked to fail over to another path if there is one. Signed-off-by: Chao Leng Signed-off-by: Christoph Hellwig --- drivers/nvme/host/rdma.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c index 6700d8bab68ac..53ac4d7442ba9 100644 --- a/drivers/nvme/host/rdma.c +++ b/drivers/nvme/host/rdma.c @@ -2098,7 +2098,9 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx, err_unmap: nvme_rdma_unmap_data(queue, rq); err: - if (err == -ENOMEM || err == -EAGAIN) + if (err == -EIO) + ret = nvme_host_path_error(rq); + else if (err == -ENOMEM || err == -EAGAIN) ret = BLK_STS_RESOURCE; else ret = BLK_STS_IOERR; From fda871c0ba5d2eed2cd1c881573168129da70058 Mon Sep 17 00:00:00 2001 From: Sagi Grimberg Date: Wed, 3 Feb 2021 15:00:01 -0800 Subject: [PATCH 06/22] nvmet-tcp: fix receive data digest calculation for multiple h2cdata PDUs When a host sends multiple h2cdata PDUs for a single command, we should verify the data digest calculation per PDU and not per command. Fixes: 872d26a391da ("nvmet-tcp: add NVMe over TCP target driver") Reported-by: Narayan Ayalasomayajula Tested-by: Narayan Ayalasomayajula Signed-off-by: Sagi Grimberg Signed-off-by: Christoph Hellwig --- drivers/nvme/target/tcp.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c index dc1f0f6471896..c3da50f776fac 100644 --- a/drivers/nvme/target/tcp.c +++ b/drivers/nvme/target/tcp.c @@ -378,7 +378,7 @@ static int nvmet_tcp_map_data(struct nvmet_tcp_cmd *cmd) return NVME_SC_INTERNAL; } -static void nvmet_tcp_ddgst(struct ahash_request *hash, +static void nvmet_tcp_send_ddgst(struct ahash_request *hash, struct nvmet_tcp_cmd *cmd) { ahash_request_set_crypt(hash, cmd->req.sg, @@ -386,6 +386,23 @@ static void nvmet_tcp_ddgst(struct ahash_request *hash, crypto_ahash_digest(hash); } +static void nvmet_tcp_recv_ddgst(struct ahash_request *hash, + struct nvmet_tcp_cmd *cmd) +{ + struct scatterlist sg; + struct kvec *iov; + int i; + + crypto_ahash_init(hash); + for (i = 0, iov = cmd->iov; i < cmd->nr_mapped; i++, iov++) { + sg_init_one(&sg, iov->iov_base, iov->iov_len); + ahash_request_set_crypt(hash, &sg, NULL, iov->iov_len); + crypto_ahash_update(hash); + } + ahash_request_set_crypt(hash, NULL, (void *)&cmd->exp_ddgst, 0); + crypto_ahash_final(hash); +} + static void nvmet_setup_c2h_data_pdu(struct nvmet_tcp_cmd *cmd) { struct nvme_tcp_data_pdu *pdu = cmd->data_pdu; @@ -410,7 +427,7 @@ static void nvmet_setup_c2h_data_pdu(struct nvmet_tcp_cmd *cmd) if (queue->data_digest) { pdu->hdr.flags |= NVME_TCP_F_DDGST; - nvmet_tcp_ddgst(queue->snd_hash, cmd); + nvmet_tcp_send_ddgst(queue->snd_hash, cmd); } if (cmd->queue->hdr_digest) { @@ -1059,7 +1076,7 @@ static void nvmet_tcp_prep_recv_ddgst(struct nvmet_tcp_cmd *cmd) { struct nvmet_tcp_queue *queue = cmd->queue; - nvmet_tcp_ddgst(queue->rcv_hash, cmd); + nvmet_tcp_recv_ddgst(queue->rcv_hash, cmd); queue->offset = 0; queue->left = NVME_TCP_DIGEST_LENGTH; queue->rcv_state = NVMET_TCP_RECV_DDGST; @@ -1080,14 +1097,14 @@ static int nvmet_tcp_try_recv_data(struct nvmet_tcp_queue *queue) cmd->rbytes_done += ret; } + if (queue->data_digest) { + nvmet_tcp_prep_recv_ddgst(cmd); + return 0; + } nvmet_tcp_unmap_pdu_iovec(cmd); if (!(cmd->flags & NVMET_TCP_F_INIT_FAILED) && cmd->rbytes_done == cmd->req.transfer_len) { - if (queue->data_digest) { - nvmet_tcp_prep_recv_ddgst(cmd); - return 0; - } cmd->req.execute(&cmd->req); } From 0fbcfb089a3f2f2a731d01f0aec8f7697a849c28 Mon Sep 17 00:00:00 2001 From: Sagi Grimberg Date: Fri, 5 Feb 2021 11:47:25 -0800 Subject: [PATCH 07/22] nvmet-tcp: fix potential race of tcp socket closing accept_work When we accept a TCP connection and allocate an nvmet-tcp queue we should make sure not to fully establish it or reference it as the connection may be already closing, which triggers queue release work, which does not fence against queue establishment. In order to address such a race, we make sure to check the sk_state and contain the queue reference to be done underneath the sk_callback_lock such that the queue release work correctly fences against it. Fixes: 872d26a391da ("nvmet-tcp: add NVMe over TCP target driver") Reported-by: Elad Grupi Signed-off-by: Sagi Grimberg Signed-off-by: Christoph Hellwig --- drivers/nvme/target/tcp.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c index c3da50f776fac..ac2d9ed23ceaf 100644 --- a/drivers/nvme/target/tcp.c +++ b/drivers/nvme/target/tcp.c @@ -1484,17 +1484,27 @@ static int nvmet_tcp_set_queue_sock(struct nvmet_tcp_queue *queue) if (inet->rcv_tos > 0) ip_sock_set_tos(sock->sk, inet->rcv_tos); + ret = 0; write_lock_bh(&sock->sk->sk_callback_lock); - sock->sk->sk_user_data = queue; - queue->data_ready = sock->sk->sk_data_ready; - sock->sk->sk_data_ready = nvmet_tcp_data_ready; - queue->state_change = sock->sk->sk_state_change; - sock->sk->sk_state_change = nvmet_tcp_state_change; - queue->write_space = sock->sk->sk_write_space; - sock->sk->sk_write_space = nvmet_tcp_write_space; + if (sock->sk->sk_state != TCP_ESTABLISHED) { + /* + * If the socket is already closing, don't even start + * consuming it + */ + ret = -ENOTCONN; + } else { + sock->sk->sk_user_data = queue; + queue->data_ready = sock->sk->sk_data_ready; + sock->sk->sk_data_ready = nvmet_tcp_data_ready; + queue->state_change = sock->sk->sk_state_change; + sock->sk->sk_state_change = nvmet_tcp_state_change; + queue->write_space = sock->sk->sk_write_space; + sock->sk->sk_write_space = nvmet_tcp_write_space; + queue_work_on(queue_cpu(queue), nvmet_tcp_wq, &queue->io_work); + } write_unlock_bh(&sock->sk->sk_callback_lock); - return 0; + return ret; } static int nvmet_tcp_alloc_queue(struct nvmet_tcp_port *port, @@ -1542,8 +1552,6 @@ static int nvmet_tcp_alloc_queue(struct nvmet_tcp_port *port, if (ret) goto out_destroy_sq; - queue_work_on(queue_cpu(queue), nvmet_tcp_wq, &queue->io_work); - return 0; out_destroy_sq: mutex_lock(&nvmet_tcp_queue_mutex); From 73a1a2298f3e9df24cea7a9aab412ba9470f6159 Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Fri, 5 Feb 2021 11:50:02 -0800 Subject: [PATCH 08/22] nvme-multipath: set nr_zones for zoned namespaces The bio based drivers only require the request_queue's nr_zones is set, so set this field in the head if the namespace path is zoned. Fixes: 240e6ee272c07 ("nvme: support for zoned namespaces") Reported-by: Minwoo Im Cc: Damien Le Moal Signed-off-by: Keith Busch Signed-off-by: Christoph Hellwig --- drivers/nvme/host/multipath.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index 65bd6efa5e1c6..0696319adaf64 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -677,6 +677,10 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id) if (blk_queue_stable_writes(ns->queue) && ns->head->disk) blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, ns->head->disk->queue); +#ifdef CONFIG_BLK_DEV_ZONED + if (blk_queue_is_zoned(ns->queue) && ns->head->disk) + ns->head->disk->queue->nr_zones = ns->queue->nr_zones; +#endif } void nvme_mpath_remove_disk(struct nvme_ns_head *head) From b5df8e79a293739f031f25eb45de350165033ea4 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sun, 7 Feb 2021 17:17:34 +0100 Subject: [PATCH 09/22] nvmet-fc: add a missing __rcu annotation to nvmet_fc_tgt_assoc.queues Make sparse happy after the recent conversion to RCU lookups. Fixes: 4e2f02bf77da ("nvmet-fc: use RCU proctection for assoc_list") Signed-off-by: Christoph Hellwig Reviewed-by: Sagi Grimberg Reviewed-by: Chaitanya Kulkarni Reviewed-by: James Smart --- drivers/nvme/target/fc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c index c14c60bfdf85c..d375745fc4ed3 100644 --- a/drivers/nvme/target/fc.c +++ b/drivers/nvme/target/fc.c @@ -165,7 +165,7 @@ struct nvmet_fc_tgt_assoc { struct nvmet_fc_hostport *hostport; struct nvmet_fc_ls_iod *rcv_disconn; struct list_head a_list; - struct nvmet_fc_tgt_queue *queues[NVMET_NR_QUEUES + 1]; + struct nvmet_fc_tgt_queue __rcu *queues[NVMET_NR_QUEUES + 1]; struct kref ref; struct work_struct del_work; struct rcu_head rcu; From 40244ad36bcfb796a6bb9e95bdcbf8ddf3134509 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Tue, 9 Feb 2021 21:47:52 -0800 Subject: [PATCH 10/22] nvmet: set status to 0 in case for invalid nsid For unallocated namespace in nvmet_execute_identify_ns() don't set the status to NVME_SC_INVALID_NS, set it to zero. Fixes: bffcd507780e ("nvmet: set right status on error in id-ns handler") Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/target/admin-cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c index 613a4d8feac12..5070ea5cf260d 100644 --- a/drivers/nvme/target/admin-cmd.c +++ b/drivers/nvme/target/admin-cmd.c @@ -485,7 +485,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req) /* return an all zeroed buffer if we can't find an active namespace */ req->ns = nvmet_find_namespace(ctrl, req->cmd->identify.nsid); if (!req->ns) { - status = NVME_SC_INVALID_NS; + status = 0; goto done; } From aa0aff604a60627b9f6c51c99dd5f63634322668 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Tue, 9 Feb 2021 21:47:53 -0800 Subject: [PATCH 11/22] nvmet: return uniform error for invalid ns For nvmet_find_namespace() error case we have inconsistent error code mapping in the function nvmet_get_smart_log_nsid() and nvmet_set_feat_write_protect(). There is no point in retrying for the invalid namesapce from the host side. Set the error code to the NVME_SC_INVALID_NS | NVME_SC_DNR which matches what we have in nvmet_execute_identify_desclist(). Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/target/admin-cmd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c index 5070ea5cf260d..e938064254a57 100644 --- a/drivers/nvme/target/admin-cmd.c +++ b/drivers/nvme/target/admin-cmd.c @@ -82,7 +82,7 @@ static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req, pr_err("Could not find namespace id : %d\n", le32_to_cpu(req->cmd->get_log_page.nsid)); req->error_loc = offsetof(struct nvme_rw_command, nsid); - return NVME_SC_INVALID_NS; + return NVME_SC_INVALID_NS | NVME_SC_DNR; } /* we don't have the right data for file backed ns */ @@ -697,7 +697,7 @@ static u16 nvmet_set_feat_write_protect(struct nvmet_req *req) req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->rw.nsid); if (unlikely(!req->ns)) { req->error_loc = offsetof(struct nvme_common_command, nsid); - return status; + return status = NVME_SC_INVALID_NS | NVME_SC_DNR; } mutex_lock(&subsys->lock); From 3a1f7c79ae6d3dfdc16082daa44b3cf8dbe4f238 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Tue, 9 Feb 2021 21:47:54 -0800 Subject: [PATCH 12/22] nvmet: make nvmet_find_namespace() req based The six callers of nvmet_find_namespace() duplicate the error log page update and status setting code for each call on failure. All callers are nvmet requests based functions, so we can pass req to the nvmet_find_namesapce() & derive ctrl from req, that'll allow us to update the error log page in nvmet_find_namespace(). Now that we pass the request we can also get rid of the local variable in nvmet_find_namespace() and use the req->ns and return the error code. Replace the ctrl parameter with nvmet_req for nvmet_find_namespace(), centralize the error log page update for non allocated namesapces, and return uniform error for non-allocated namespace. The nvmet_find_namespace() takes nsid parameter which is from NVMe commands structures such as get_log_page, identify, rw and common. All these commands have same offset for the nsid field. Derive nsid from req->cmd->common.nsid) & remove the extra parameter from the nvmet_find_namespace(). Lastly now we associate the ns to the req parameter that we pass to the nvmet_find_namespace(), rename nvmet_find_namespace() to nvmet_req_find_ns(). Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/target/admin-cmd.c | 46 +++++++++++++-------------------- drivers/nvme/target/core.c | 24 +++++++++-------- drivers/nvme/target/nvmet.h | 2 +- 3 files changed, 32 insertions(+), 40 deletions(-) diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c index e938064254a57..f32533480e66f 100644 --- a/drivers/nvme/target/admin-cmd.c +++ b/drivers/nvme/target/admin-cmd.c @@ -75,15 +75,11 @@ static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req, struct nvme_smart_log *slog) { u64 host_reads, host_writes, data_units_read, data_units_written; + u16 status; - req->ns = nvmet_find_namespace(req->sq->ctrl, - req->cmd->get_log_page.nsid); - if (!req->ns) { - pr_err("Could not find namespace id : %d\n", - le32_to_cpu(req->cmd->get_log_page.nsid)); - req->error_loc = offsetof(struct nvme_rw_command, nsid); - return NVME_SC_INVALID_NS | NVME_SC_DNR; - } + status = nvmet_req_find_ns(req); + if (status) + return status; /* we don't have the right data for file backed ns */ if (!req->ns->bdev) @@ -468,7 +464,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req) { struct nvmet_ctrl *ctrl = req->sq->ctrl; struct nvme_id_ns *id; - u16 status = 0; + u16 status; if (le32_to_cpu(req->cmd->identify.nsid) == NVME_NSID_ALL) { req->error_loc = offsetof(struct nvme_identify, nsid); @@ -483,8 +479,8 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req) } /* return an all zeroed buffer if we can't find an active namespace */ - req->ns = nvmet_find_namespace(ctrl, req->cmd->identify.nsid); - if (!req->ns) { + status = nvmet_req_find_ns(req); + if (status) { status = 0; goto done; } @@ -604,15 +600,12 @@ static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len, static void nvmet_execute_identify_desclist(struct nvmet_req *req) { - u16 status = 0; off_t off = 0; + u16 status; - req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid); - if (!req->ns) { - req->error_loc = offsetof(struct nvme_identify, nsid); - status = NVME_SC_INVALID_NS | NVME_SC_DNR; + status = nvmet_req_find_ns(req); + if (status) goto out; - } if (memchr_inv(&req->ns->uuid, 0, sizeof(req->ns->uuid))) { status = nvmet_copy_ns_identifier(req, NVME_NIDT_UUID, @@ -692,13 +685,11 @@ static u16 nvmet_set_feat_write_protect(struct nvmet_req *req) { u32 write_protect = le32_to_cpu(req->cmd->common.cdw11); struct nvmet_subsys *subsys = req->sq->ctrl->subsys; - u16 status = NVME_SC_FEATURE_NOT_CHANGEABLE; + u16 status; - req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->rw.nsid); - if (unlikely(!req->ns)) { - req->error_loc = offsetof(struct nvme_common_command, nsid); - return status = NVME_SC_INVALID_NS | NVME_SC_DNR; - } + status = nvmet_req_find_ns(req); + if (status) + return status; mutex_lock(&subsys->lock); switch (write_protect) { @@ -799,11 +790,10 @@ static u16 nvmet_get_feat_write_protect(struct nvmet_req *req) struct nvmet_subsys *subsys = req->sq->ctrl->subsys; u32 result; - req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->common.nsid); - if (!req->ns) { - req->error_loc = offsetof(struct nvme_common_command, nsid); - return NVME_SC_INVALID_NS | NVME_SC_DNR; - } + result = nvmet_req_find_ns(req); + if (result) + return result; + mutex_lock(&subsys->lock); if (req->ns->readonly == true) result = NVME_NS_WRITE_PROTECT; diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c index 8ce4d59cc9e75..95b58d4b1af28 100644 --- a/drivers/nvme/target/core.c +++ b/drivers/nvme/target/core.c @@ -417,15 +417,18 @@ void nvmet_stop_keep_alive_timer(struct nvmet_ctrl *ctrl) cancel_delayed_work_sync(&ctrl->ka_work); } -struct nvmet_ns *nvmet_find_namespace(struct nvmet_ctrl *ctrl, __le32 nsid) +u16 nvmet_req_find_ns(struct nvmet_req *req) { - struct nvmet_ns *ns; + u32 nsid = le32_to_cpu(req->cmd->common.nsid); - ns = xa_load(&ctrl->subsys->namespaces, le32_to_cpu(nsid)); - if (ns) - percpu_ref_get(&ns->ref); + req->ns = xa_load(&req->sq->ctrl->subsys->namespaces, nsid); + if (unlikely(!req->ns)) { + req->error_loc = offsetof(struct nvme_common_command, nsid); + return NVME_SC_INVALID_NS | NVME_SC_DNR; + } - return ns; + percpu_ref_get(&req->ns->ref); + return NVME_SC_SUCCESS; } static void nvmet_destroy_namespace(struct percpu_ref *ref) @@ -862,11 +865,10 @@ static u16 nvmet_parse_io_cmd(struct nvmet_req *req) if (nvmet_req_passthru_ctrl(req)) return nvmet_parse_passthru_io_cmd(req); - req->ns = nvmet_find_namespace(req->sq->ctrl, cmd->rw.nsid); - if (unlikely(!req->ns)) { - req->error_loc = offsetof(struct nvme_common_command, nsid); - return NVME_SC_INVALID_NS | NVME_SC_DNR; - } + ret = nvmet_req_find_ns(req); + if (unlikely(ret)) + return ret; + ret = nvmet_check_ana_state(req->port, req->ns); if (unlikely(ret)) { req->error_loc = offsetof(struct nvme_common_command, nsid); diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h index 8776dd1a0490e..954b3d8451f55 100644 --- a/drivers/nvme/target/nvmet.h +++ b/drivers/nvme/target/nvmet.h @@ -443,7 +443,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn, void nvmet_subsys_put(struct nvmet_subsys *subsys); void nvmet_subsys_del_ctrls(struct nvmet_subsys *subsys); -struct nvmet_ns *nvmet_find_namespace(struct nvmet_ctrl *ctrl, __le32 nsid); +u16 nvmet_req_find_ns(struct nvmet_req *req); void nvmet_put_namespace(struct nvmet_ns *ns); int nvmet_ns_enable(struct nvmet_ns *ns); void nvmet_ns_disable(struct nvmet_ns *ns); From 3999434b6ce6fa452128c36cbb5017f0cd347615 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Tue, 9 Feb 2021 21:47:55 -0800 Subject: [PATCH 13/22] nvmet: remove extra variable in id-ns handler In nvmet_execute_identify_ns() local variable ctrl is accessed only in one place, remove that and directly use it from nvmet_req->sq->ctrl. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/target/admin-cmd.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c index f32533480e66f..552da813da185 100644 --- a/drivers/nvme/target/admin-cmd.c +++ b/drivers/nvme/target/admin-cmd.c @@ -462,7 +462,6 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req) static void nvmet_execute_identify_ns(struct nvmet_req *req) { - struct nvmet_ctrl *ctrl = req->sq->ctrl; struct nvme_id_ns *id; u16 status; @@ -523,7 +522,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req) id->lbaf[0].ds = req->ns->blksize_shift; - if (ctrl->pi_support && nvmet_ns_has_pi(req->ns)) { + if (req->sq->ctrl->pi_support && nvmet_ns_has_pi(req->ns)) { id->dpc = NVME_NS_DPC_PI_FIRST | NVME_NS_DPC_PI_LAST | NVME_NS_DPC_PI_TYPE1 | NVME_NS_DPC_PI_TYPE2 | NVME_NS_DPC_PI_TYPE3; From d81d57cf1b4702b7c2fa8ce8f1d5c6961a0c20b5 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Tue, 9 Feb 2021 21:47:56 -0800 Subject: [PATCH 14/22] nvmet: add helper to report invalid opcode In the NVMeOF block device backend, file backend, and passthru backend we reject and report the commands if opcode is not handled. Add an helper and use it in block device backend to keep the code and error message uniform. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/target/core.c | 9 +++++++++ drivers/nvme/target/io-cmd-bdev.c | 5 +---- drivers/nvme/target/nvmet.h | 1 + 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c index 95b58d4b1af28..35ad96261b8f8 100644 --- a/drivers/nvme/target/core.c +++ b/drivers/nvme/target/core.c @@ -82,6 +82,15 @@ inline u16 errno_to_nvme_status(struct nvmet_req *req, int errno) return status; } +u16 nvmet_report_invalid_opcode(struct nvmet_req *req) +{ + pr_debug("unhandled cmd %d on qid %d\n", req->cmd->common.opcode, + req->sq->qid); + + req->error_loc = offsetof(struct nvme_common_command, opcode); + return NVME_SC_INVALID_OPCODE | NVME_SC_DNR; +} + static struct nvmet_subsys *nvmet_find_get_subsys(struct nvmet_port *port, const char *subsysnqn); diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c index 23095bdfce06c..105ef2b125cfe 100644 --- a/drivers/nvme/target/io-cmd-bdev.c +++ b/drivers/nvme/target/io-cmd-bdev.c @@ -449,9 +449,6 @@ u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req) req->execute = nvmet_bdev_execute_write_zeroes; return 0; default: - pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode, - req->sq->qid); - req->error_loc = offsetof(struct nvme_common_command, opcode); - return NVME_SC_INVALID_OPCODE | NVME_SC_DNR; + return nvmet_report_invalid_opcode(req); } } diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h index 954b3d8451f55..00f78e41d8c85 100644 --- a/drivers/nvme/target/nvmet.h +++ b/drivers/nvme/target/nvmet.h @@ -589,6 +589,7 @@ nvmet_req_passthru_ctrl(struct nvmet_req *req) } u16 errno_to_nvme_status(struct nvmet_req *req, int errno); +u16 nvmet_report_invalid_opcode(struct nvmet_req *req); /* Convert a 32-bit number to a 16-bit 0's based number */ static inline __le16 to0based(u32 a) From 1c2c76136875d2329339275d431484a33dbb612d Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Tue, 9 Feb 2021 21:47:57 -0800 Subject: [PATCH 15/22] nvmet: use invalid cmd opcode helper In the NVMeOF block device backend, file backend, and passthru backend we reject and report the commands if opcode is not handled. Use the previously introduced helper in file backend to reduce the duplicate code and make the error message uniform. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/target/io-cmd-file.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c index 0abbefd9925e3..715d4376c9979 100644 --- a/drivers/nvme/target/io-cmd-file.c +++ b/drivers/nvme/target/io-cmd-file.c @@ -400,9 +400,6 @@ u16 nvmet_file_parse_io_cmd(struct nvmet_req *req) req->execute = nvmet_file_execute_write_zeroes; return 0; default: - pr_err("unhandled cmd for file ns %d on qid %d\n", - cmd->common.opcode, req->sq->qid); - req->error_loc = offsetof(struct nvme_common_command, opcode); - return NVME_SC_INVALID_OPCODE | NVME_SC_DNR; + return nvmet_report_invalid_opcode(req); } } From 07116ea50fd3a3b58725389e4abaf1c03bcae641 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Tue, 9 Feb 2021 21:47:58 -0800 Subject: [PATCH 16/22] nvmet: use invalid cmd opcode helper In the NVMeOF block device backend, file backend, and passthru backend we reject and report the commands if opcode is not handled. Use the previously introduced helper in the passthru backend to make the error message uniform. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/target/passthru.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c index cbc88acdd2336..3b22f4a868f43 100644 --- a/drivers/nvme/target/passthru.c +++ b/drivers/nvme/target/passthru.c @@ -494,7 +494,7 @@ u16 nvmet_parse_passthru_admin_cmd(struct nvmet_req *req) return nvmet_setup_passthru_command(req); default: /* Reject commands not in the allowlist above */ - return NVME_SC_INVALID_OPCODE | NVME_SC_DNR; + return nvmet_report_invalid_opcode(req); } } From d86481e924a7d6e8a40477ffa98077c6c0d77ed5 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Tue, 9 Feb 2021 21:47:59 -0800 Subject: [PATCH 17/22] nvmet: use min of device_path and disk len MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In function __assign_req_name() instead of using the DEVICE_NAME_LEN in strncpy() use min of DISK_NAME_LEN and strlen(req->ns->device_path). This is needed to turn off the following warnings:- In file included from drivers/nvme/target/core.c:14: In function ‘__assign_req_name’, inlined from ‘trace_event_raw_event_nvmet_req_init’ at drivers/nvme/target/./trace.h:58:1: drivers/nvme/target/trace.h:52:3: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation] strncpy(name, req->ns->device_path, DISK_NAME_LEN); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In function ‘__assign_req_name’, inlined from ‘perf_trace_nvmet_req_complete’ at drivers/nvme/target/./trace.h:100:1: drivers/nvme/target/trace.h:52:3: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation] strncpy(name, req->ns->device_path, DISK_NAME_LEN); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In function ‘__assign_req_name’, inlined from ‘perf_trace_nvmet_req_init’ at drivers/nvme/target/./trace.h:58:1: drivers/nvme/target/trace.h:52:3: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation] strncpy(name, req->ns->device_path, DISK_NAME_LEN); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In function ‘__assign_req_name’, inlined from ‘trace_event_raw_event_nvmet_req_complete’ at drivers/nvme/target/./trace.h:100:1: drivers/nvme/target/trace.h:52:3: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation] strncpy(name, req->ns->device_path, DISK_NAME_LEN); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/target/trace.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/nvme/target/trace.h b/drivers/nvme/target/trace.h index c14e3249a14dc..6109b3806b12b 100644 --- a/drivers/nvme/target/trace.h +++ b/drivers/nvme/target/trace.h @@ -48,10 +48,13 @@ static inline struct nvmet_ctrl *nvmet_req_to_ctrl(struct nvmet_req *req) static inline void __assign_req_name(char *name, struct nvmet_req *req) { - if (req->ns) - strncpy(name, req->ns->device_path, DISK_NAME_LEN); - else + if (!req->ns) { memset(name, 0, DISK_NAME_LEN); + return; + } + + strncpy(name, req->ns->device_path, + min_t(size_t, DISK_NAME_LEN, strlen(req->ns->device_path))); } #endif From 20c2c3bb83f26c42bf62cc773f96f30848ed11a2 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Tue, 9 Feb 2021 21:48:01 -0800 Subject: [PATCH 18/22] nvmet: add nvmet_req_subsys() helper Just like what we have to get the passthru ctrl from the req, add an helper to get the subsystem associated with the nvmet_req() instead of open coding the chain of structures. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/target/admin-cmd.c | 10 +++++----- drivers/nvme/target/core.c | 2 +- drivers/nvme/target/nvmet.h | 7 ++++++- drivers/nvme/target/passthru.c | 4 ++-- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c index 552da813da185..bc6a774f21244 100644 --- a/drivers/nvme/target/admin-cmd.c +++ b/drivers/nvme/target/admin-cmd.c @@ -683,7 +683,7 @@ static u16 nvmet_write_protect_flush_sync(struct nvmet_req *req) static u16 nvmet_set_feat_write_protect(struct nvmet_req *req) { u32 write_protect = le32_to_cpu(req->cmd->common.cdw11); - struct nvmet_subsys *subsys = req->sq->ctrl->subsys; + struct nvmet_subsys *subsys = nvmet_req_subsys(req); u16 status; status = nvmet_req_find_ns(req); @@ -742,7 +742,7 @@ u16 nvmet_set_feat_async_event(struct nvmet_req *req, u32 mask) void nvmet_execute_set_features(struct nvmet_req *req) { - struct nvmet_subsys *subsys = req->sq->ctrl->subsys; + struct nvmet_subsys *subsys = nvmet_req_subsys(req); u32 cdw10 = le32_to_cpu(req->cmd->common.cdw10); u32 cdw11 = le32_to_cpu(req->cmd->common.cdw11); u16 status = 0; @@ -786,7 +786,7 @@ void nvmet_execute_set_features(struct nvmet_req *req) static u16 nvmet_get_feat_write_protect(struct nvmet_req *req) { - struct nvmet_subsys *subsys = req->sq->ctrl->subsys; + struct nvmet_subsys *subsys = nvmet_req_subsys(req); u32 result; result = nvmet_req_find_ns(req); @@ -816,7 +816,7 @@ void nvmet_get_feat_async_event(struct nvmet_req *req) void nvmet_execute_get_features(struct nvmet_req *req) { - struct nvmet_subsys *subsys = req->sq->ctrl->subsys; + struct nvmet_subsys *subsys = nvmet_req_subsys(req); u32 cdw10 = le32_to_cpu(req->cmd->common.cdw10); u16 status = 0; @@ -923,7 +923,7 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req) if (nvme_is_fabrics(cmd)) return nvmet_parse_fabrics_cmd(req); - if (req->sq->ctrl->subsys->type == NVME_NQN_DISC) + if (nvmet_req_subsys(req)->type == NVME_NQN_DISC) return nvmet_parse_discovery_cmd(req); ret = nvmet_check_ctrl_status(req, cmd); diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c index 35ad96261b8f8..7e3b194203a46 100644 --- a/drivers/nvme/target/core.c +++ b/drivers/nvme/target/core.c @@ -430,7 +430,7 @@ u16 nvmet_req_find_ns(struct nvmet_req *req) { u32 nsid = le32_to_cpu(req->cmd->common.nsid); - req->ns = xa_load(&req->sq->ctrl->subsys->namespaces, nsid); + req->ns = xa_load(&nvmet_req_subsys(req)->namespaces, nsid); if (unlikely(!req->ns)) { req->error_loc = offsetof(struct nvme_common_command, nsid); return NVME_SC_INVALID_NS | NVME_SC_DNR; diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h index 00f78e41d8c85..cdfa537b1c0af 100644 --- a/drivers/nvme/target/nvmet.h +++ b/drivers/nvme/target/nvmet.h @@ -551,6 +551,11 @@ static inline u32 nvmet_dsm_len(struct nvmet_req *req) sizeof(struct nvme_dsm_range); } +static inline struct nvmet_subsys *nvmet_req_subsys(struct nvmet_req *req) +{ + return req->sq->ctrl->subsys; +} + #ifdef CONFIG_NVME_TARGET_PASSTHRU void nvmet_passthru_subsys_free(struct nvmet_subsys *subsys); int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys); @@ -585,7 +590,7 @@ static inline struct nvme_ctrl *nvmet_passthru_ctrl(struct nvmet_subsys *subsys) static inline struct nvme_ctrl * nvmet_req_passthru_ctrl(struct nvmet_req *req) { - return nvmet_passthru_ctrl(req->sq->ctrl->subsys); + return nvmet_passthru_ctrl(nvmet_req_subsys(req)); } u16 errno_to_nvme_status(struct nvmet_req *req, int errno); diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c index 3b22f4a868f43..f50c7b2bf21c4 100644 --- a/drivers/nvme/target/passthru.c +++ b/drivers/nvme/target/passthru.c @@ -239,9 +239,9 @@ static void nvmet_passthru_execute_cmd(struct nvmet_req *req) } q = ns->queue; - timeout = req->sq->ctrl->subsys->io_timeout; + timeout = nvmet_req_subsys(req)->io_timeout; } else { - timeout = req->sq->ctrl->subsys->admin_timeout; + timeout = nvmet_req_subsys(req)->admin_timeout; } rq = nvme_alloc_request(q, req->cmd, 0); From 295a39f5a56f3276bae6a0ae5c26ce06bb8aa21c Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Tue, 9 Feb 2021 21:48:02 -0800 Subject: [PATCH 19/22] nvmet: remove else at the end of the function The function nvmet_parse_io_cmd() returns value from nvmet_file_parse_io_cmd() or nvmet_bdev_parse_io_cmd() based on which backend is set for the request. Remove the else and just return the value from nvmet_bdev_parse_io_cmd(). Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/target/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c index 7e3b194203a46..67bbf0e3b5079 100644 --- a/drivers/nvme/target/core.c +++ b/drivers/nvme/target/core.c @@ -891,8 +891,8 @@ static u16 nvmet_parse_io_cmd(struct nvmet_req *req) if (req->ns->file) return nvmet_file_parse_io_cmd(req); - else - return nvmet_bdev_parse_io_cmd(req); + + return nvmet_bdev_parse_io_cmd(req); } bool nvmet_req_init(struct nvmet_req *req, struct nvmet_cq *cq, From ed7770f6628691c13c9423bce7eee7cff2399c12 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Tue, 19 Jan 2021 07:43:18 +0100 Subject: [PATCH 20/22] nvme-hwmon: rework to avoid devm allocation The original design to use device-managed resource allocation doesn't really work as the NVMe controller has a vastly different lifetime than the hwmon sysfs attributes, causing warning about duplicate sysfs entries upon reconnection. This patch reworks the hwmon allocation to avoid device-managed resource allocation, and uses the NVMe controller as parent for the sysfs attributes. Cc: Guenter Roeck Signed-off-by: Hannes Reinecke Tested-by: Enzo Matsumiya Tested-by: Daniel Wagner Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 1 + drivers/nvme/host/hwmon.c | 31 +++++++++++++++++++++---------- drivers/nvme/host/nvme.h | 8 ++++++++ 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 02579f4f776c7..d77f3f26d8d38 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -4471,6 +4471,7 @@ EXPORT_SYMBOL_GPL(nvme_start_ctrl); void nvme_uninit_ctrl(struct nvme_ctrl *ctrl) { + nvme_hwmon_exit(ctrl); nvme_fault_inject_fini(&ctrl->fault_inject); dev_pm_qos_hide_latency_tolerance(ctrl->device); cdev_device_del(&ctrl->cdev, ctrl->device); diff --git a/drivers/nvme/host/hwmon.c b/drivers/nvme/host/hwmon.c index 552dbc04567bc..8f9e96986780e 100644 --- a/drivers/nvme/host/hwmon.c +++ b/drivers/nvme/host/hwmon.c @@ -223,12 +223,12 @@ static const struct hwmon_chip_info nvme_hwmon_chip_info = { int nvme_hwmon_init(struct nvme_ctrl *ctrl) { - struct device *dev = ctrl->dev; + struct device *dev = ctrl->device; struct nvme_hwmon_data *data; struct device *hwmon; int err; - data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + data = kzalloc(sizeof(*data), GFP_KERNEL); if (!data) return 0; @@ -237,19 +237,30 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl) err = nvme_hwmon_get_smart_log(data); if (err) { - dev_warn(ctrl->device, - "Failed to read smart log (error %d)\n", err); - devm_kfree(dev, data); + dev_warn(dev, "Failed to read smart log (error %d)\n", err); + kfree(data); return err; } - hwmon = devm_hwmon_device_register_with_info(dev, "nvme", data, - &nvme_hwmon_chip_info, - NULL); + hwmon = hwmon_device_register_with_info(dev, "nvme", + data, &nvme_hwmon_chip_info, + NULL); if (IS_ERR(hwmon)) { dev_warn(dev, "Failed to instantiate hwmon device\n"); - devm_kfree(dev, data); + kfree(data); } - + ctrl->hwmon_device = hwmon; return 0; } + +void nvme_hwmon_exit(struct nvme_ctrl *ctrl) +{ + if (ctrl->hwmon_device) { + struct nvme_hwmon_data *data = + dev_get_drvdata(ctrl->hwmon_device); + + hwmon_device_unregister(ctrl->hwmon_device); + ctrl->hwmon_device = NULL; + kfree(data); + } +} diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 5819f03810414..2efb87642d180 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -246,6 +246,9 @@ struct nvme_ctrl { struct rw_semaphore namespaces_rwsem; struct device ctrl_device; struct device *device; /* char device */ +#ifdef CONFIG_NVME_HWMON + struct device *hwmon_device; +#endif struct cdev cdev; struct work_struct reset_work; struct work_struct delete_work; @@ -812,11 +815,16 @@ static inline struct nvme_ns *nvme_get_ns_from_dev(struct device *dev) #ifdef CONFIG_NVME_HWMON int nvme_hwmon_init(struct nvme_ctrl *ctrl); +void nvme_hwmon_exit(struct nvme_ctrl *ctrl); #else static inline int nvme_hwmon_init(struct nvme_ctrl *ctrl) { return 0; } + +static inline void nvme_hwmon_exit(struct nvme_ctrl *ctrl) +{ +} #endif u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns, From 4bdf260362b3be529d170b04662638fd6dc52241 Mon Sep 17 00:00:00 2001 From: Filippo Sironi Date: Wed, 10 Feb 2021 01:39:42 +0100 Subject: [PATCH 21/22] nvme: add 48-bit DMA address quirk for Amazon NVMe controllers Some Amazon NVMe controllers do not follow the NVMe specification and are limited to 48-bit DMA addresses. Add a quirk to force bounce buffering if needed and limit the IOVA allocation for these devices. This affects all current Amazon NVMe controllers that expose EBS volumes (0x0061, 0x0065, 0x8061) and local instance storage (0xcd00, 0xcd01, 0xcd02). Signed-off-by: Filippo Sironi Signed-off-by: Christoph Hellwig --- drivers/nvme/host/nvme.h | 6 ++++++ drivers/nvme/host/pci.c | 21 ++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 2efb87642d180..07b34175c6ce6 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -144,6 +144,12 @@ enum nvme_quirks { * NVMe 1.3 compliance. */ NVME_QUIRK_NO_NS_DESC_LIST = (1 << 15), + + /* + * The controller does not properly handle DMA addresses over + * 48 bits. + */ + NVME_QUIRK_DMA_ADDRESS_BITS_48 = (1 << 16), }; /* diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 5b78e68be9a15..0045c5edf629e 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -2362,13 +2362,16 @@ static int nvme_pci_enable(struct nvme_dev *dev) { int result = -ENOMEM; struct pci_dev *pdev = to_pci_dev(dev->dev); + int dma_address_bits = 64; if (pci_enable_device_mem(pdev)) return result; pci_set_master(pdev); - if (dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(64))) + if (dev->ctrl.quirks & NVME_QUIRK_DMA_ADDRESS_BITS_48) + dma_address_bits = 48; + if (dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(dma_address_bits))) goto disable; if (readl(dev->bar + NVME_REG_CSTS) == -1) { @@ -3257,6 +3260,22 @@ static const struct pci_device_id nvme_id_table[] = { .driver_data = NVME_QUIRK_DISABLE_WRITE_ZEROES, }, { PCI_DEVICE(0x15b7, 0x2001), /* Sandisk Skyhawk */ .driver_data = NVME_QUIRK_DISABLE_WRITE_ZEROES, }, + { PCI_DEVICE(0x1d97, 0x2263), /* SPCC */ + .driver_data = NVME_QUIRK_DISABLE_WRITE_ZEROES, }, + { PCI_DEVICE(0x2646, 0x2263), /* KINGSTON A2000 NVMe SSD */ + .driver_data = NVME_QUIRK_NO_DEEPEST_PS, }, + { PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0x0061), + .driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, }, + { PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0x0065), + .driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, }, + { PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0x8061), + .driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, }, + { PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0xcd00), + .driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, }, + { PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0xcd01), + .driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, }, + { PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0xcd02), + .driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, }, { PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2001), .driver_data = NVME_QUIRK_SINGLE_VECTOR }, { PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2003) }, From e11e5116171dedeaf63735931e72ad5de0f30ed5 Mon Sep 17 00:00:00 2001 From: Sagi Grimberg Date: Wed, 10 Feb 2021 14:04:00 -0800 Subject: [PATCH 22/22] nvme-tcp: fix crash triggered with a dataless request submission write-zeros has a bio, but does not have any data buffers associated with it. Hence should not initialize the request iter for it (which attempts to reference the bi_io_vec (and crash). -- run blktests nvme/012 at 2021-02-05 21:53:34 BUG: kernel NULL pointer dereference, address: 0000000000000008 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 0 P4D 0 Oops: 0000 [#1] SMP NOPTI CPU: 15 PID: 12069 Comm: kworker/15:2H Tainted: G S I 5.11.0-rc6+ #1 Hardware name: Dell Inc. PowerEdge R640/06NR82, BIOS 2.10.0 11/12/2020 Workqueue: kblockd blk_mq_run_work_fn RIP: 0010:nvme_tcp_init_iter+0x7d/0xd0 [nvme_tcp] RSP: 0018:ffffbd084447bd18 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffffa0bba9f3ce80 RCX: 0000000000000000 RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000002000000 RBP: ffffa0ba8ac6fec0 R08: 0000000002000000 R09: 0000000000000000 R10: 0000000002800809 R11: 0000000000000000 R12: 0000000000000000 R13: ffffa0bba9f3cf90 R14: 0000000000000000 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffffa0c9ff9c0000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000008 CR3: 00000001c9c6c005 CR4: 00000000007706e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Call Trace: nvme_tcp_queue_rq+0xef/0x330 [nvme_tcp] blk_mq_dispatch_rq_list+0x11c/0x7c0 ? blk_mq_flush_busy_ctxs+0xf6/0x110 __blk_mq_sched_dispatch_requests+0x12b/0x170 blk_mq_sched_dispatch_requests+0x30/0x60 __blk_mq_run_hw_queue+0x2b/0x60 process_one_work+0x1cb/0x360 ? process_one_work+0x360/0x360 worker_thread+0x30/0x370 ? process_one_work+0x360/0x360 kthread+0x116/0x130 ? kthread_park+0x80/0x80 ret_from_fork+0x1f/0x30 -- Fixes: cb9b870fba3e ("nvme-tcp: fix wrong setting of request iov_iter") Reported-by: Yi Zhang Signed-off-by: Sagi Grimberg Reviewed-by: Keith Busch Reviewed-by: Chaitanya Kulkarni Tested-by: Yi Zhang Signed-off-by: Christoph Hellwig --- drivers/nvme/host/tcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 619b0d8f6e382..69f59d2c5799b 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -2271,7 +2271,7 @@ static blk_status_t nvme_tcp_setup_cmd_pdu(struct nvme_ns *ns, req->data_len = blk_rq_nr_phys_segments(rq) ? blk_rq_payload_bytes(rq) : 0; req->curr_bio = rq->bio; - if (req->curr_bio) + if (req->curr_bio && req->data_len) nvme_tcp_init_iter(req, rq_data_dir(rq)); if (rq_data_dir(rq) == WRITE &&