[PATCH 0/9] nvme: dead code cleanups

[PATCH 0/9] nvme: dead code cleanups

[Guixin Liu]

A round of dead code removals in the nvme host and target drivers,
all found by code inspection. Every patch stands on its own and can
be picked individually:

 - the NVMEQ_DELETE_ERROR flag lost its only reader in 2020 and is
   now write-only; dropping it also removes the nvme_del_cq_end()
   wrapper, which degenerates to nvme_del_queue_end() without it
 - struct fields never referenced since they were introduced:
   nvme_fc_queue.seqno, nvme_dhchap_queue_context.entry,
   nvmet_fc_ls_iod.fcpreq and nvmet_fc_tgt_queue.ninetypercent
 - a hostnqn already-assigned check that can never fire because
   opts->host is only set after the option parsing loop
 - the reconnect_delay "off" case, copied from fast_io_fail_tmo but
   unreachable for an unsigned value the parser bounds to positive
   values
 - an address family map entry that both show and store skip, an
   unreachable break in the passthru feature handling, and a drain
   of an SQ list that is never populated

Guixin Liu (9):
  nvme-pci: remove write-only NVMEQ_DELETE_ERROR flag
  nvme-fc: remove dead seqno field in nvme_fc_queue
  nvme-auth: remove dead entry field in nvme_dhchap_queue_context
  nvme-fabrics: remove dead hostnqn already-assigned check
  nvme: drop unreachable "off" case in reconnect_delay_show()
  nvmet-fc: remove dead fields in nvmet_fc_ls_iod and nvmet_fc_tgt_queue
  nvmet: remove unreachable "pcie" entry from the address family map
  nvmet: passthru: remove unreachable break statements
  nvmet-pci-epf: don't drain the never-populated SQ list

 drivers/nvme/host/auth.c       |  1 -
 drivers/nvme/host/fabrics.c    |  6 ------
 drivers/nvme/host/fc.c         |  1 -
 drivers/nvme/host/pci.c        | 20 +-------------------
 drivers/nvme/host/sysfs.c      |  2 --
 drivers/nvme/target/configfs.c |  5 ++---
 drivers/nvme/target/fc.c       |  3 ---
 drivers/nvme/target/passthru.c |  2 --
 drivers/nvme/target/pci-epf.c  |  2 --
 9 files changed, 3 insertions(+), 39 deletions(-)

-- 
2.43.7

[PATCH 1/9] nvme-pci: remove write-only NVMEQ_DELETE_ERROR flag

[Guixin Liu]

Commit fa46c6fb5d61b ("nvme/pci: move cqe check after device shutdown")
removed the only reader of the NVMEQ_DELETE_ERROR flag, but left the
flag and its two writes behind. Without a reader the flag carries no
state, and nvme_del_cq_end() degenerates to a plain wrapper around
nvme_del_queue_end(), which the SQ deletion path already uses directly.

Remove the flag, its writes, and the now redundant wrapper.

Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/nvme/host/pci.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 8a90639789c9..6f6bb9a2486c 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -388,7 +388,6 @@ struct nvme_queue {
 	unsigned long flags;
 #define NVMEQ_ENABLED		0
 #define NVMEQ_SQ_CMB		1
-#define NVMEQ_DELETE_ERROR	2
 #define NVMEQ_POLLED		3
 	__le32 *dbbuf_sq_db;
 	__le32 *dbbuf_cq_db;
@@ -2238,8 +2237,6 @@ static int nvme_create_queue(struct nvme_queue *nvmeq, int qid, bool polled)
 	int result;
 	u16 vector = 0;
 
-	clear_bit(NVMEQ_DELETE_ERROR, &nvmeq->flags);
-
 	/*
 	 * A queue's vector matches the queue identifier unless the controller
 	 * has only one vector available.
@@ -3101,18 +3098,6 @@ static enum rq_end_io_ret nvme_del_queue_end(struct request *req,
 	return RQ_END_IO_NONE;
 }
 
-static enum rq_end_io_ret nvme_del_cq_end(struct request *req,
-					  blk_status_t error,
-					  const struct io_comp_batch *iob)
-{
-	struct nvme_queue *nvmeq = req->end_io_data;
-
-	if (error)
-		set_bit(NVMEQ_DELETE_ERROR, &nvmeq->flags);
-
-	return nvme_del_queue_end(req, error, iob);
-}
-
 static int nvme_delete_queue(struct nvme_queue *nvmeq, u8 opcode)
 {
 	struct request_queue *q = nvmeq->dev->ctrl.admin_q;
@@ -3127,10 +3112,7 @@ static int nvme_delete_queue(struct nvme_queue *nvmeq, u8 opcode)
 		return PTR_ERR(req);
 	nvme_init_request(req, &cmd);
 
-	if (opcode == nvme_admin_delete_cq)
-		req->end_io = nvme_del_cq_end;
-	else
-		req->end_io = nvme_del_queue_end;
+	req->end_io = nvme_del_queue_end;
 	req->end_io_data = nvmeq;
 
 	init_completion(&nvmeq->delete_done);
-- 
2.43.7

[PATCH 2/9] nvme-fc: remove dead seqno field in nvme_fc_queue

[Guixin Liu]

The seqno field of struct nvme_fc_queue has never been referenced
since it was introduced in commit e399441de9115 ("nvme-fabrics: Add
host support for FC transport"). Drop it.

Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/nvme/host/fc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 0b9ff50d26ff..2cbd744ffd22 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -39,7 +39,6 @@ struct nvme_fc_queue {
 	size_t			cmnd_capsule_len;
 	u32			qnum;
 	u32			rqcnt;
-	u32			seqno;
 
 	u64			connection_id;
 	atomic_t		csn;
-- 
2.43.7

[PATCH 3/9] nvme-auth: remove dead entry field in nvme_dhchap_queue_context

[Guixin Liu]

The entry list_head of struct nvme_dhchap_queue_context has never been
used since it was introduced in commit f50fff73d620c ("nvme: implement
In-Band authentication"). Drop it.

Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/nvme/host/auth.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index 85f005a5efd0..9a3da74dec5a 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -19,7 +19,6 @@ static struct kmem_cache *nvme_chap_buf_cache;
 static mempool_t *nvme_chap_buf_pool;
 
 struct nvme_dhchap_queue_context {
-	struct list_head entry;
 	struct work_struct auth_work;
 	struct nvme_ctrl *ctrl;
 	struct crypto_kpp *dh_tfm;
-- 
2.43.7

[PATCH 4/9] nvme-fabrics: remove dead hostnqn already-assigned check

[Guixin Liu]

The opts->host check in the NVMF_OPT_HOSTNQN handler can never fire:
options are parsed into a freshly allocated struct, and opts->host is
only assigned after the parsing loop completes. A duplicate hostnqn
option is already silently overridden by the strscpy() into the local
buffer. Remove the dead check.

Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/nvme/host/fabrics.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 3d7ad86c1955..c262538523fb 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -872,12 +872,6 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 			opts->fast_io_fail_tmo = token;
 			break;
 		case NVMF_OPT_HOSTNQN:
-			if (opts->host) {
-				pr_err("hostnqn already user-assigned: %s\n",
-				       opts->host->nqn);
-				ret = -EADDRINUSE;
-				goto out;
-			}
 			p = match_strdup(args);
 			if (!p) {
 				ret = -ENOMEM;
-- 
2.43.7

[PATCH 5/9] nvme: drop unreachable "off" case in reconnect_delay_show()

[Guixin Liu]

The "off" case was copied from fast_io_fail_tmo_show(), but unlike
fast_io_fail_tmo, reconnect_delay is an unsigned int that the option
parser bounds to positive values and that has no off state. The
comparison can only trigger when the raw value 4294967295 is written
through sysfs, which is not a meaningful delay. Drop the case.

Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/nvme/host/sysfs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 10018c6356b6..3724b5ce03a0 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -618,8 +618,6 @@ static ssize_t nvme_ctrl_reconnect_delay_show(struct device *dev,
 {
 	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
 
-	if (ctrl->opts->reconnect_delay == -1)
-		return sysfs_emit(buf, "off\n");
 	return sysfs_emit(buf, "%d\n", ctrl->opts->reconnect_delay);
 }
 
-- 
2.43.7

[PATCH 6/9] nvmet-fc: remove dead fields in nvmet_fc_ls_iod and nvmet_fc_tgt_queue

[Guixin Liu]

The fcpreq field of struct nvmet_fc_ls_iod is only ever assigned NULL
and never read, and the ninetypercent field of struct nvmet_fc_tgt_queue
is never referenced as the 90% queue state is recomputed on demand.
Drop both.

Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/nvme/target/fc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 2b7dc3f03912..bccfbaea6dfc 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -27,7 +27,6 @@ struct nvmet_fc_tgt_assoc;
 
 struct nvmet_fc_ls_iod {		/* for an LS RQST RCV */
 	struct nvmefc_ls_rsp		*lsrsp;
-	struct nvmefc_tgt_fcp_req	*fcpreq;	/* only if RS */
 
 	struct list_head		ls_rcv_list; /* tgtport->ls_rcv_list */
 
@@ -129,7 +128,6 @@ struct nvmet_fc_defer_fcp_req {
 };
 
 struct nvmet_fc_tgt_queue {
-	bool				ninetypercent;
 	u16				qid;
 	u16				sqsize;
 	u16				ersp_ratio;
@@ -2076,7 +2074,6 @@ nvmet_fc_rcv_ls_req(struct nvmet_fc_target_port *target_port,
 	}
 
 	iod->lsrsp = lsrsp;
-	iod->fcpreq = NULL;
 	memcpy(iod->rqstbuf, lsreqbuf, lsreqbuf_len);
 	iod->rqstdatalen = lsreqbuf_len;
 	iod->hosthandle = hosthandle;
-- 
2.43.7

[PATCH 7/9] nvmet: remove unreachable "pcie" entry from the address family map

[Guixin Liu]

The first entry of nvmet_addr_family[] is never matched: both the
adrfam show and store handlers skip index 0, and the
NVMF_ADDR_FAMILY_PCI type is matched by the "pci" entry further down
the array. Drop the entry and start the iteration at index 0 so the
map no longer needs an explanation for the skipped element.

Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/nvme/target/configfs.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index b3a5242d06f9..874476277ffa 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -42,7 +42,6 @@ static struct nvmet_type_name_map nvmet_transport[] = {
 };
 
 static const struct nvmet_type_name_map nvmet_addr_family[] = {
-	{ NVMF_ADDR_FAMILY_PCI,		"pcie" },
 	{ NVMF_ADDR_FAMILY_IP4,		"ipv4" },
 	{ NVMF_ADDR_FAMILY_IP6,		"ipv6" },
 	{ NVMF_ADDR_FAMILY_IB,		"ib" },
@@ -68,7 +67,7 @@ static ssize_t nvmet_addr_adrfam_show(struct config_item *item, char *page)
 	u8 adrfam = to_nvmet_port(item)->disc_addr.adrfam;
 	int i;
 
-	for (i = 1; i < ARRAY_SIZE(nvmet_addr_family); i++) {
+	for (i = 0; i < ARRAY_SIZE(nvmet_addr_family); i++) {
 		if (nvmet_addr_family[i].type == adrfam)
 			return snprintf(page, PAGE_SIZE, "%s\n",
 					nvmet_addr_family[i].name);
@@ -86,7 +85,7 @@ static ssize_t nvmet_addr_adrfam_store(struct config_item *item,
 	if (nvmet_is_port_enabled(port, __func__))
 		return -EACCES;
 
-	for (i = 1; i < ARRAY_SIZE(nvmet_addr_family); i++) {
+	for (i = 0; i < ARRAY_SIZE(nvmet_addr_family); i++) {
 		if (sysfs_streq(page, nvmet_addr_family[i].name))
 			goto found;
 	}
-- 
2.43.7

[PATCH 8/9] nvmet: passthru: remove unreachable break statements

[Guixin Liu]

In the nvme_admin_set_features and nvme_admin_get_features handlers of
nvmet_passthru_parse_cmd(), every path of the inner switch over the
feature id returns, so the break statement following the switch can
never be reached. Remove them.

Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/nvme/target/passthru.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 6b67880c7bb2..32ae4b6b59bf 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -550,7 +550,6 @@ u16 nvmet_parse_passthru_admin_cmd(struct nvmet_req *req)
 		default:
 			return nvmet_passthru_get_set_features(req);
 		}
-		break;
 	case nvme_admin_get_features:
 		switch (le32_to_cpu(req->cmd->features.fid)) {
 		case NVME_FEAT_ASYNC_EVENT:
@@ -564,7 +563,6 @@ u16 nvmet_parse_passthru_admin_cmd(struct nvmet_req *req)
 		default:
 			return nvmet_passthru_get_set_features(req);
 		}
-		break;
 	case nvme_admin_identify:
 		switch (req->cmd->identify.cns) {
 		case NVME_ID_CNS_CS_CTRL:
-- 
2.43.7

[PATCH 9/9] nvmet-pci-epf: don't drain the never-populated SQ list

[Guixin Liu]

nvmet_pci_epf_delete_sq() calls nvmet_pci_epf_drain_queue() on the
submission queue, but IOD links are only ever added to the completion
queue list, so the SQ list is always empty and the call is a no-op.
Remove it.

Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/nvme/target/pci-epf.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
index a8df176c1a57..e5781e538a4a 100644
--- a/drivers/nvme/target/pci-epf.c
+++ b/drivers/nvme/target/pci-epf.c
@@ -1432,8 +1432,6 @@ static u16 nvmet_pci_epf_delete_sq(struct nvmet_ctrl *tctrl, u16 sqid)
 	destroy_workqueue(sq->iod_wq);
 	sq->iod_wq = NULL;
 
-	nvmet_pci_epf_drain_queue(sq);
-
 	if (sq->nvme_sq.ctrl)
 		nvmet_sq_destroy(&sq->nvme_sq);
 
-- 
2.43.7

Re: [PATCH 5/9] nvme: drop unreachable "off" case in reconnect_delay_show()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] nvme: Documentation/ABI/stable/sysfs-nvme out of sync for reconnect_delay
--

commit 0bdac7a94ae088ab38d8d6006c42a68c7d0447dc
Author: Guixin Liu <kanie@linux.alibaba.com>

nvme: drop unreachable "off" case in reconnect_delay_show()

This commit removes the check for ctrl->opts->reconnect_delay == -1 in
nvme_ctrl_reconnect_delay_show() so that reading reconnect_delay no longer
emits "off".

> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> index 10018c6356b62..3724b5ce03a07 100644
> --- a/drivers/nvme/host/sysfs.c
> +++ b/drivers/nvme/host/sysfs.c
> @@ -618,8 +618,6 @@ static ssize_t nvme_ctrl_reconnect_delay_show(struct device *dev,
>  {
>  	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
>  
> -	if (ctrl->opts->reconnect_delay == -1)
> -		return sysfs_emit(buf, "off\n");
>  	return sysfs_emit(buf, "%d\n", ctrl->opts->reconnect_delay);

[Severity: Low]
With this branch removed from nvme_ctrl_reconnect_delay_show() in
drivers/nvme/host/sysfs.c, reading /sys/class/nvme/nvmeX/reconnect_delay
never outputs "off". However, Documentation/ABI/stable/sysfs-nvme still
states:

		reconnect_delay: Shows or sets the reconnect delay in
		seconds. Reading returns the delay value, or "off" if
		disabled.

Should Documentation/ABI/stable/sysfs-nvme be updated to drop the "off"
description as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105021051.1667222-1-kanie@linux.alibaba.com?part=5