All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups
@ 2025-04-08 15:29 Daniel Wagner
  2025-04-08 15:29 ` [PATCH 1/8] nvmet-fcloop: swap list_add_tail arguments Daniel Wagner
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Daniel Wagner @ 2025-04-08 15:29 UTC (permalink / raw)
  To: James Smart, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Jens Axboe, Hannes Reinecke
  Cc: James Smart, linux-nvme, linux-kernel, Daniel Wagner

The refcount nvmet-fcloop series is getting a bit long [1]. Here are the
various fixes and small cleanups which are independent of the refcount
changes.

Only the first patch is new, the rest has already been revied by
Christoph and Hannes. I've gave this series a good test run and there
shouldn't be any regressions.

I've got a some more simple patches (with reviews) but they will
introduce regressions, because they depend on the refcount feature.

[1] https://lore.kernel.org/all/20250318-nvmet-fcloop-v3-0-05fec0fc02f6@kernel.org/

Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
Daniel Wagner (8):
      nvmet-fcloop: swap list_add_tail arguments
      nvmet-fcloop: replace kref with refcount
      nvmet-fcloop: add ref counting to lport
      nvmet-fc: inline nvmet_fc_delete_assoc
      nvmet-fc: inline nvmet_fc_free_hostport
      nvmet-fc: update tgtport ref per assoc
      nvmet-fc: take tgtport reference only once
      nvmet-fc: put ref when assoc->del_work is already scheduled

 drivers/nvme/target/fc.c     | 60 ++++++++++++-----------------------
 drivers/nvme/target/fcloop.c | 74 ++++++++++++++++++++++++--------------------
 2 files changed, 60 insertions(+), 74 deletions(-)
---
base-commit: 0514a1379e11c6b8038674f43a478b0857d47a5e
change-id: 20250408-nvmet-fcloop-part-one-3b62e48cea13

Best regards,
-- 
Daniel Wagner <wagi@kernel.org>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/8] nvmet-fcloop: swap list_add_tail arguments
  2025-04-08 15:29 [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Daniel Wagner
@ 2025-04-08 15:29 ` Daniel Wagner
  2025-04-09  6:09   ` Hannes Reinecke
  2025-04-09 10:58   ` Christoph Hellwig
  2025-04-08 15:29 ` [PATCH 2/8] nvmet-fcloop: replace kref with refcount Daniel Wagner
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 12+ messages in thread
From: Daniel Wagner @ 2025-04-08 15:29 UTC (permalink / raw)
  To: James Smart, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Jens Axboe, Hannes Reinecke
  Cc: James Smart, linux-nvme, linux-kernel, Daniel Wagner

The newly element to be added to the list is the first argument of
list_add_tail. This fix is missing dcfad4ab4d67 ("nvmet-fcloop: swap
the list_add_tail arguments").

Fixes: 437c0b824dbd ("nvme-fcloop: add target to host LS request support")
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 drivers/nvme/target/fcloop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
index e1abb27927ff74c9c55ddefd9581aab18bf3b00f..da195d61a9664cba21880a4b99ba0ee94a58f81a 100644
--- a/drivers/nvme/target/fcloop.c
+++ b/drivers/nvme/target/fcloop.c
@@ -478,7 +478,7 @@ fcloop_t2h_xmt_ls_rsp(struct nvme_fc_local_port *localport,
 	if (targetport) {
 		tport = targetport->private;
 		spin_lock(&tport->lock);
-		list_add_tail(&tport->ls_list, &tls_req->ls_list);
+		list_add_tail(&tls_req->ls_list, &tport->ls_list);
 		spin_unlock(&tport->lock);
 		queue_work(nvmet_wq, &tport->ls_work);
 	}

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/8] nvmet-fcloop: replace kref with refcount
  2025-04-08 15:29 [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Daniel Wagner
  2025-04-08 15:29 ` [PATCH 1/8] nvmet-fcloop: swap list_add_tail arguments Daniel Wagner
@ 2025-04-08 15:29 ` Daniel Wagner
  2025-04-08 15:29 ` [PATCH 3/8] nvmet-fcloop: add ref counting to lport Daniel Wagner
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Daniel Wagner @ 2025-04-08 15:29 UTC (permalink / raw)
  To: James Smart, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Jens Axboe, Hannes Reinecke
  Cc: James Smart, linux-nvme, linux-kernel, Daniel Wagner

The kref wrapper is not really adding any value ontop of refcount. Thus
replace the kref API with the refcount API.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 drivers/nvme/target/fcloop.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
index da195d61a9664cba21880a4b99ba0ee94a58f81a..67e24c7d59306fec4aa88cacc536c876c465af35 100644
--- a/drivers/nvme/target/fcloop.c
+++ b/drivers/nvme/target/fcloop.c
@@ -239,7 +239,7 @@ struct fcloop_nport {
 	struct fcloop_tport *tport;
 	struct fcloop_lport *lport;
 	struct list_head nport_list;
-	struct kref ref;
+	refcount_t ref;
 	u64 node_name;
 	u64 port_name;
 	u32 port_role;
@@ -274,7 +274,7 @@ struct fcloop_fcpreq {
 	u32				inistate;
 	bool				active;
 	bool				aborted;
-	struct kref			ref;
+	refcount_t			ref;
 	struct work_struct		fcp_rcv_work;
 	struct work_struct		abort_rcv_work;
 	struct work_struct		tio_done_work;
@@ -534,24 +534,18 @@ fcloop_tgt_discovery_evt(struct nvmet_fc_target_port *tgtport)
 }
 
 static void
-fcloop_tfcp_req_free(struct kref *ref)
+fcloop_tfcp_req_put(struct fcloop_fcpreq *tfcp_req)
 {
-	struct fcloop_fcpreq *tfcp_req =
-		container_of(ref, struct fcloop_fcpreq, ref);
+	if (!refcount_dec_and_test(&tfcp_req->ref))
+		return;
 
 	kfree(tfcp_req);
 }
 
-static void
-fcloop_tfcp_req_put(struct fcloop_fcpreq *tfcp_req)
-{
-	kref_put(&tfcp_req->ref, fcloop_tfcp_req_free);
-}
-
 static int
 fcloop_tfcp_req_get(struct fcloop_fcpreq *tfcp_req)
 {
-	return kref_get_unless_zero(&tfcp_req->ref);
+	return refcount_inc_not_zero(&tfcp_req->ref);
 }
 
 static void
@@ -748,7 +742,7 @@ fcloop_fcp_req(struct nvme_fc_local_port *localport,
 	INIT_WORK(&tfcp_req->fcp_rcv_work, fcloop_fcp_recv_work);
 	INIT_WORK(&tfcp_req->abort_rcv_work, fcloop_fcp_abort_recv_work);
 	INIT_WORK(&tfcp_req->tio_done_work, fcloop_tgt_fcprqst_done_work);
-	kref_init(&tfcp_req->ref);
+	refcount_set(&tfcp_req->ref, 1);
 
 	queue_work(nvmet_wq, &tfcp_req->fcp_rcv_work);
 
@@ -1001,24 +995,18 @@ fcloop_fcp_abort(struct nvme_fc_local_port *localport,
 }
 
 static void
-fcloop_nport_free(struct kref *ref)
+fcloop_nport_put(struct fcloop_nport *nport)
 {
-	struct fcloop_nport *nport =
-		container_of(ref, struct fcloop_nport, ref);
+	if (!refcount_dec_and_test(&nport->ref))
+		return;
 
 	kfree(nport);
 }
 
-static void
-fcloop_nport_put(struct fcloop_nport *nport)
-{
-	kref_put(&nport->ref, fcloop_nport_free);
-}
-
 static int
 fcloop_nport_get(struct fcloop_nport *nport)
 {
-	return kref_get_unless_zero(&nport->ref);
+	return refcount_inc_not_zero(&nport->ref);
 }
 
 static void
@@ -1249,7 +1237,7 @@ fcloop_alloc_nport(const char *buf, size_t count, bool remoteport)
 		newnport->port_role = opts->roles;
 	if (opts->mask & NVMF_OPT_FCADDR)
 		newnport->port_id = opts->fcaddr;
-	kref_init(&newnport->ref);
+	refcount_set(&newnport->ref, 1);
 
 	spin_lock_irqsave(&fcloop_lock, flags);
 

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/8] nvmet-fcloop: add ref counting to lport
  2025-04-08 15:29 [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Daniel Wagner
  2025-04-08 15:29 ` [PATCH 1/8] nvmet-fcloop: swap list_add_tail arguments Daniel Wagner
  2025-04-08 15:29 ` [PATCH 2/8] nvmet-fcloop: replace kref with refcount Daniel Wagner
@ 2025-04-08 15:29 ` Daniel Wagner
  2025-04-08 15:29 ` [PATCH 4/8] nvmet-fc: inline nvmet_fc_delete_assoc Daniel Wagner
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Daniel Wagner @ 2025-04-08 15:29 UTC (permalink / raw)
  To: James Smart, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Jens Axboe, Hannes Reinecke
  Cc: James Smart, linux-nvme, linux-kernel, Daniel Wagner

The fcloop_lport objects live time is controlled by the user interface
add_local_port and del_local_port. nport, rport and tport objects are
pointing to the lport objects but here is no clear tracking. Let's
introduce an explicit ref counter for the lport objects and prepare the
stage for restructuring how lports are used.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 drivers/nvme/target/fcloop.c | 44 +++++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
index 67e24c7d59306fec4aa88cacc536c876c465af35..641201e62c1bafa13986642c6c4067b35f784edd 100644
--- a/drivers/nvme/target/fcloop.c
+++ b/drivers/nvme/target/fcloop.c
@@ -208,6 +208,7 @@ struct fcloop_lport {
 	struct nvme_fc_local_port *localport;
 	struct list_head lport_list;
 	struct completion unreg_done;
+	refcount_t ref;
 };
 
 struct fcloop_lport_priv {
@@ -994,6 +995,27 @@ fcloop_fcp_abort(struct nvme_fc_local_port *localport,
 	}
 }
 
+static void
+fcloop_lport_put(struct fcloop_lport *lport)
+{
+	unsigned long flags;
+
+	if (!refcount_dec_and_test(&lport->ref))
+		return;
+
+	spin_lock_irqsave(&fcloop_lock, flags);
+	list_del(&lport->lport_list);
+	spin_unlock_irqrestore(&fcloop_lock, flags);
+
+	kfree(lport);
+}
+
+static int
+fcloop_lport_get(struct fcloop_lport *lport)
+{
+	return refcount_inc_not_zero(&lport->ref);
+}
+
 static void
 fcloop_nport_put(struct fcloop_nport *nport)
 {
@@ -1017,6 +1039,8 @@ fcloop_localport_delete(struct nvme_fc_local_port *localport)
 
 	/* release any threads waiting for the unreg to complete */
 	complete(&lport->unreg_done);
+
+	fcloop_lport_put(lport);
 }
 
 static void
@@ -1128,6 +1152,7 @@ fcloop_create_local_port(struct device *dev, struct device_attribute *attr,
 
 		lport->localport = localport;
 		INIT_LIST_HEAD(&lport->lport_list);
+		refcount_set(&lport->ref, 1);
 
 		spin_lock_irqsave(&fcloop_lock, flags);
 		list_add_tail(&lport->lport_list, &fcloop_lports);
@@ -1144,13 +1169,6 @@ fcloop_create_local_port(struct device *dev, struct device_attribute *attr,
 	return ret ? ret : count;
 }
 
-
-static void
-__unlink_local_port(struct fcloop_lport *lport)
-{
-	list_del(&lport->lport_list);
-}
-
 static int
 __wait_localport_unreg(struct fcloop_lport *lport)
 {
@@ -1163,8 +1181,6 @@ __wait_localport_unreg(struct fcloop_lport *lport)
 	if (!ret)
 		wait_for_completion(&lport->unreg_done);
 
-	kfree(lport);
-
 	return ret;
 }
 
@@ -1187,8 +1203,9 @@ fcloop_delete_local_port(struct device *dev, struct device_attribute *attr,
 	list_for_each_entry(tlport, &fcloop_lports, lport_list) {
 		if (tlport->localport->node_name == nodename &&
 		    tlport->localport->port_name == portname) {
+			if (!fcloop_lport_get(tlport))
+				break;
 			lport = tlport;
-			__unlink_local_port(lport);
 			break;
 		}
 	}
@@ -1198,6 +1215,7 @@ fcloop_delete_local_port(struct device *dev, struct device_attribute *attr,
 		return -ENOENT;
 
 	ret = __wait_localport_unreg(lport);
+	fcloop_lport_put(lport);
 
 	return ret ? ret : count;
 }
@@ -1625,17 +1643,17 @@ static void __exit fcloop_exit(void)
 	for (;;) {
 		lport = list_first_entry_or_null(&fcloop_lports,
 						typeof(*lport), lport_list);
-		if (!lport)
+		if (!lport || !fcloop_lport_get(lport))
 			break;
 
-		__unlink_local_port(lport);
-
 		spin_unlock_irqrestore(&fcloop_lock, flags);
 
 		ret = __wait_localport_unreg(lport);
 		if (ret)
 			pr_warn("%s: Failed deleting local port\n", __func__);
 
+		fcloop_lport_put(lport);
+
 		spin_lock_irqsave(&fcloop_lock, flags);
 	}
 

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/8] nvmet-fc: inline nvmet_fc_delete_assoc
  2025-04-08 15:29 [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Daniel Wagner
                   ` (2 preceding siblings ...)
  2025-04-08 15:29 ` [PATCH 3/8] nvmet-fcloop: add ref counting to lport Daniel Wagner
@ 2025-04-08 15:29 ` Daniel Wagner
  2025-04-08 15:29 ` [PATCH 5/8] nvmet-fc: inline nvmet_fc_free_hostport Daniel Wagner
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Daniel Wagner @ 2025-04-08 15:29 UTC (permalink / raw)
  To: James Smart, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Jens Axboe, Hannes Reinecke
  Cc: James Smart, linux-nvme, linux-kernel, Daniel Wagner

No need for this tiny helper with only one user, just inline it.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 drivers/nvme/target/fc.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 7318b736d41417bd974f58f1ef66bce8640db422..6487c46ebba8d12e573d19fe8c39d526492c506a 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -1075,13 +1075,6 @@ nvmet_fc_alloc_hostport(struct nvmet_fc_tgtport *tgtport, void *hosthandle)
 	return newhost;
 }
 
-static void
-nvmet_fc_delete_assoc(struct nvmet_fc_tgt_assoc *assoc)
-{
-	nvmet_fc_delete_target_assoc(assoc);
-	nvmet_fc_tgt_a_put(assoc);
-}
-
 static void
 nvmet_fc_delete_assoc_work(struct work_struct *work)
 {
@@ -1089,7 +1082,8 @@ nvmet_fc_delete_assoc_work(struct work_struct *work)
 		container_of(work, struct nvmet_fc_tgt_assoc, del_work);
 	struct nvmet_fc_tgtport *tgtport = assoc->tgtport;
 
-	nvmet_fc_delete_assoc(assoc);
+	nvmet_fc_delete_target_assoc(assoc);
+	nvmet_fc_tgt_a_put(assoc);
 	nvmet_fc_tgtport_put(tgtport);
 }
 

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/8] nvmet-fc: inline nvmet_fc_free_hostport
  2025-04-08 15:29 [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Daniel Wagner
                   ` (3 preceding siblings ...)
  2025-04-08 15:29 ` [PATCH 4/8] nvmet-fc: inline nvmet_fc_delete_assoc Daniel Wagner
@ 2025-04-08 15:29 ` Daniel Wagner
  2025-04-08 15:29 ` [PATCH 6/8] nvmet-fc: update tgtport ref per assoc Daniel Wagner
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Daniel Wagner @ 2025-04-08 15:29 UTC (permalink / raw)
  To: James Smart, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Jens Axboe, Hannes Reinecke
  Cc: James Smart, linux-nvme, linux-kernel, Daniel Wagner

No need for this tiny helper with only one user, let's inline it.

And since the hostport ref counter needs to stay in sync, it's not
optional anymore to give back the reference.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 drivers/nvme/target/fc.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 6487c46ebba8d12e573d19fe8c39d526492c506a..6d64dadcb356b78b522d0deaa433cc745bfcd8f6 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -995,16 +995,6 @@ nvmet_fc_hostport_get(struct nvmet_fc_hostport *hostport)
 	return kref_get_unless_zero(&hostport->ref);
 }
 
-static void
-nvmet_fc_free_hostport(struct nvmet_fc_hostport *hostport)
-{
-	/* if LLDD not implemented, leave as NULL */
-	if (!hostport || !hostport->hosthandle)
-		return;
-
-	nvmet_fc_hostport_put(hostport);
-}
-
 static struct nvmet_fc_hostport *
 nvmet_fc_match_hostport(struct nvmet_fc_tgtport *tgtport, void *hosthandle)
 {
@@ -1184,7 +1174,7 @@ nvmet_fc_target_assoc_free(struct kref *ref)
 	/* Send Disconnect now that all i/o has completed */
 	nvmet_fc_xmt_disconnect_assoc(assoc);
 
-	nvmet_fc_free_hostport(assoc->hostport);
+	nvmet_fc_hostport_put(assoc->hostport);
 	spin_lock_irqsave(&tgtport->lock, flags);
 	oldls = assoc->rcv_disconn;
 	spin_unlock_irqrestore(&tgtport->lock, flags);
@@ -1449,11 +1439,6 @@ nvmet_fc_free_tgtport(struct kref *ref)
 	struct nvmet_fc_tgtport *tgtport =
 		container_of(ref, struct nvmet_fc_tgtport, ref);
 	struct device *dev = tgtport->dev;
-	unsigned long flags;
-
-	spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
-	list_del(&tgtport->tgt_list);
-	spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
 
 	nvmet_fc_free_ls_iodlist(tgtport);
 
@@ -1614,6 +1599,11 @@ int
 nvmet_fc_unregister_targetport(struct nvmet_fc_target_port *target_port)
 {
 	struct nvmet_fc_tgtport *tgtport = targetport_to_tgtport(target_port);
+	unsigned long flags;
+
+	spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
+	list_del(&tgtport->tgt_list);
+	spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
 
 	nvmet_fc_portentry_unbind_tgt(tgtport);
 

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/8] nvmet-fc: update tgtport ref per assoc
  2025-04-08 15:29 [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Daniel Wagner
                   ` (4 preceding siblings ...)
  2025-04-08 15:29 ` [PATCH 5/8] nvmet-fc: inline nvmet_fc_free_hostport Daniel Wagner
@ 2025-04-08 15:29 ` Daniel Wagner
  2025-04-08 15:29 ` [PATCH 7/8] nvmet-fc: take tgtport reference only once Daniel Wagner
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Daniel Wagner @ 2025-04-08 15:29 UTC (permalink / raw)
  To: James Smart, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Jens Axboe, Hannes Reinecke
  Cc: James Smart, linux-nvme, linux-kernel, Daniel Wagner

We need to take for each unique association a reference.
nvmet_fc_alloc_hostport for each newly created association.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 drivers/nvme/target/fc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 6d64dadcb356b78b522d0deaa433cc745bfcd8f6..42613280c06e82a0236520d93470ec6fdede37ea 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -1127,6 +1127,7 @@ nvmet_fc_alloc_target_assoc(struct nvmet_fc_tgtport *tgtport, void *hosthandle)
 		goto out_ida;
 
 	assoc->tgtport = tgtport;
+	nvmet_fc_tgtport_get(tgtport);
 	assoc->a_id = idx;
 	INIT_LIST_HEAD(&assoc->a_list);
 	kref_init(&assoc->ref);
@@ -1228,6 +1229,8 @@ nvmet_fc_delete_target_assoc(struct nvmet_fc_tgt_assoc *assoc)
 	dev_info(tgtport->dev,
 		"{%d:%d} Association deleted\n",
 		tgtport->fc_target_port.port_num, assoc->a_id);
+
+	nvmet_fc_tgtport_put(tgtport);
 }
 
 static struct nvmet_fc_tgt_assoc *

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 7/8] nvmet-fc: take tgtport reference only once
  2025-04-08 15:29 [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Daniel Wagner
                   ` (5 preceding siblings ...)
  2025-04-08 15:29 ` [PATCH 6/8] nvmet-fc: update tgtport ref per assoc Daniel Wagner
@ 2025-04-08 15:29 ` Daniel Wagner
  2025-04-08 15:29 ` [PATCH 8/8] nvmet-fc: put ref when assoc->del_work is already scheduled Daniel Wagner
  2025-04-09 11:05 ` [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Christoph Hellwig
  8 siblings, 0 replies; 12+ messages in thread
From: Daniel Wagner @ 2025-04-08 15:29 UTC (permalink / raw)
  To: James Smart, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Jens Axboe, Hannes Reinecke
  Cc: James Smart, linux-nvme, linux-kernel, Daniel Wagner

The reference counting code can be simplified. Instead taking a tgtport
refrerence at the beginning of nvmet_fc_alloc_hostport and put it back
if not a new hostport object is allocated, only take it when a new
hostport object is allocated.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 drivers/nvme/target/fc.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 42613280c06e82a0236520d93470ec6fdede37ea..61e9eea3bee430bfdef541bfe0d1f2538a49d9eb 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -1018,33 +1018,24 @@ nvmet_fc_alloc_hostport(struct nvmet_fc_tgtport *tgtport, void *hosthandle)
 	struct nvmet_fc_hostport *newhost, *match = NULL;
 	unsigned long flags;
 
+	/*
+	 * Caller holds a reference on tgtport.
+	 */
+
 	/* if LLDD not implemented, leave as NULL */
 	if (!hosthandle)
 		return NULL;
 
-	/*
-	 * take reference for what will be the newly allocated hostport if
-	 * we end up using a new allocation
-	 */
-	if (!nvmet_fc_tgtport_get(tgtport))
-		return ERR_PTR(-EINVAL);
-
 	spin_lock_irqsave(&tgtport->lock, flags);
 	match = nvmet_fc_match_hostport(tgtport, hosthandle);
 	spin_unlock_irqrestore(&tgtport->lock, flags);
 
-	if (match) {
-		/* no new allocation - release reference */
-		nvmet_fc_tgtport_put(tgtport);
+	if (match)
 		return match;
-	}
 
 	newhost = kzalloc(sizeof(*newhost), GFP_KERNEL);
-	if (!newhost) {
-		/* no new allocation - release reference */
-		nvmet_fc_tgtport_put(tgtport);
+	if (!newhost)
 		return ERR_PTR(-ENOMEM);
-	}
 
 	spin_lock_irqsave(&tgtport->lock, flags);
 	match = nvmet_fc_match_hostport(tgtport, hosthandle);
@@ -1053,6 +1044,7 @@ nvmet_fc_alloc_hostport(struct nvmet_fc_tgtport *tgtport, void *hosthandle)
 		kfree(newhost);
 		newhost = match;
 	} else {
+		nvmet_fc_tgtport_get(tgtport);
 		newhost->tgtport = tgtport;
 		newhost->hosthandle = hosthandle;
 		INIT_LIST_HEAD(&newhost->host_list);

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 8/8] nvmet-fc: put ref when assoc->del_work is already scheduled
  2025-04-08 15:29 [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Daniel Wagner
                   ` (6 preceding siblings ...)
  2025-04-08 15:29 ` [PATCH 7/8] nvmet-fc: take tgtport reference only once Daniel Wagner
@ 2025-04-08 15:29 ` Daniel Wagner
  2025-04-09 11:05 ` [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Christoph Hellwig
  8 siblings, 0 replies; 12+ messages in thread
From: Daniel Wagner @ 2025-04-08 15:29 UTC (permalink / raw)
  To: James Smart, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Jens Axboe, Hannes Reinecke
  Cc: James Smart, linux-nvme, linux-kernel, Daniel Wagner

Do not leak the tgtport reference when the work is already scheduled.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 drivers/nvme/target/fc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 61e9eea3bee430bfdef541bfe0d1f2538a49d9eb..7b50130f10f6578e6e49fe8ea661de34dfbb3683 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -1073,7 +1073,8 @@ static void
 nvmet_fc_schedule_delete_assoc(struct nvmet_fc_tgt_assoc *assoc)
 {
 	nvmet_fc_tgtport_get(assoc->tgtport);
-	queue_work(nvmet_wq, &assoc->del_work);
+	if (!queue_work(nvmet_wq, &assoc->del_work))
+		nvmet_fc_tgtport_put(assoc->tgtport);
 }
 
 static bool

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/8] nvmet-fcloop: swap list_add_tail arguments
  2025-04-08 15:29 ` [PATCH 1/8] nvmet-fcloop: swap list_add_tail arguments Daniel Wagner
@ 2025-04-09  6:09   ` Hannes Reinecke
  2025-04-09 10:58   ` Christoph Hellwig
  1 sibling, 0 replies; 12+ messages in thread
From: Hannes Reinecke @ 2025-04-09  6:09 UTC (permalink / raw)
  To: Daniel Wagner, James Smart, Christoph Hellwig, Sagi Grimberg,
	Chaitanya Kulkarni, Jens Axboe
  Cc: James Smart, linux-nvme, linux-kernel

On 4/8/25 17:29, Daniel Wagner wrote:
> The newly element to be added to the list is the first argument of
> list_add_tail. This fix is missing dcfad4ab4d67 ("nvmet-fcloop: swap
> the list_add_tail arguments").
> 
> Fixes: 437c0b824dbd ("nvme-fcloop: add target to host LS request support")
> Signed-off-by: Daniel Wagner <wagi@kernel.org>
> ---
>   drivers/nvme/target/fcloop.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
> index e1abb27927ff74c9c55ddefd9581aab18bf3b00f..da195d61a9664cba21880a4b99ba0ee94a58f81a 100644
> --- a/drivers/nvme/target/fcloop.c
> +++ b/drivers/nvme/target/fcloop.c
> @@ -478,7 +478,7 @@ fcloop_t2h_xmt_ls_rsp(struct nvme_fc_local_port *localport,
>   	if (targetport) {
>   		tport = targetport->private;
>   		spin_lock(&tport->lock);
> -		list_add_tail(&tport->ls_list, &tls_req->ls_list);
> +		list_add_tail(&tls_req->ls_list, &tport->ls_list);
>   		spin_unlock(&tport->lock);
>   		queue_work(nvmet_wq, &tport->ls_work);
>   	}
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/8] nvmet-fcloop: swap list_add_tail arguments
  2025-04-08 15:29 ` [PATCH 1/8] nvmet-fcloop: swap list_add_tail arguments Daniel Wagner
  2025-04-09  6:09   ` Hannes Reinecke
@ 2025-04-09 10:58   ` Christoph Hellwig
  1 sibling, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2025-04-09 10:58 UTC (permalink / raw)
  To: Daniel Wagner
  Cc: James Smart, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Jens Axboe, Hannes Reinecke, James Smart, linux-nvme,
	linux-kernel

Looks good:

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups
  2025-04-08 15:29 [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Daniel Wagner
                   ` (7 preceding siblings ...)
  2025-04-08 15:29 ` [PATCH 8/8] nvmet-fc: put ref when assoc->del_work is already scheduled Daniel Wagner
@ 2025-04-09 11:05 ` Christoph Hellwig
  8 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2025-04-09 11:05 UTC (permalink / raw)
  To: Daniel Wagner
  Cc: James Smart, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Jens Axboe, Hannes Reinecke, James Smart, linux-nvme,
	linux-kernel

Thanks,

I've added the entire series to the nvme-6.15 branch.



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-04-09 11:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-08 15:29 [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Daniel Wagner
2025-04-08 15:29 ` [PATCH 1/8] nvmet-fcloop: swap list_add_tail arguments Daniel Wagner
2025-04-09  6:09   ` Hannes Reinecke
2025-04-09 10:58   ` Christoph Hellwig
2025-04-08 15:29 ` [PATCH 2/8] nvmet-fcloop: replace kref with refcount Daniel Wagner
2025-04-08 15:29 ` [PATCH 3/8] nvmet-fcloop: add ref counting to lport Daniel Wagner
2025-04-08 15:29 ` [PATCH 4/8] nvmet-fc: inline nvmet_fc_delete_assoc Daniel Wagner
2025-04-08 15:29 ` [PATCH 5/8] nvmet-fc: inline nvmet_fc_free_hostport Daniel Wagner
2025-04-08 15:29 ` [PATCH 6/8] nvmet-fc: update tgtport ref per assoc Daniel Wagner
2025-04-08 15:29 ` [PATCH 7/8] nvmet-fc: take tgtport reference only once Daniel Wagner
2025-04-08 15:29 ` [PATCH 8/8] nvmet-fc: put ref when assoc->del_work is already scheduled Daniel Wagner
2025-04-09 11:05 ` [PATCH 0/8] nvmet-fcloop: bunch of fixes and cleanups Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.