diff mbox series

[v2,1/5] scsi: target: core: Add RTPI field to target port

Message ID 20221006105057.30184-2-d.bogdanov@yadro.com
State New
Headers show
Series scsi: target: make RTPI an TPG identifier | expand

Commit Message

Dmitry Bogdanov Oct. 6, 2022, 10:50 a.m. UTC
SAM-5 4.6.5.2 (Relative Port Identifier attribute) defines the attribute
as unique across SCSI target ports.

The change introduces RTPI attribute to se_portal group. The value is
unique across all enabled SCSI target ports. It also limits number of
SCSI target ports to 65535.

Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
---
 v2:
   rewrite using XArray to track usage of RTPI
---
 drivers/target/target_core_fabric_configfs.c |  5 +--
 drivers/target/target_core_internal.h        |  1 +
 drivers/target/target_core_tpg.c             | 37 ++++++++++++++++++++
 include/target/target_core_base.h            |  2 ++
 4 files changed, 41 insertions(+), 4 deletions(-)

Comments

Christoph Hellwig Oct. 17, 2022, 7:38 a.m. UTC | #1
> +DEFINE_XARRAY_ALLOC(tpg_xa);

I think this wants to be marked static.

> +static int core_tpg_register_rtpi(struct se_portal_group *se_tpg)

Can you use target_ instead of the weird historic core_ prefixes for
everything here?

> +int core_tpg_enable(struct se_portal_group *se_tpg, bool enable)
> +{
> +	int ret;
> +
> +	if (enable) {
> +		ret = core_tpg_register_rtpi(se_tpg);
> +		if (ret)
> +			return ret;
> +	} else {
> +		core_tpg_deregister_rtpi(se_tpg);
> +	}
> +	ret = se_tpg->se_tpg_tfo->fabric_enable_tpg(se_tpg, enable);
> +	if (ret) {
> +		core_tpg_deregister_rtpi(se_tpg);
> +		return ret;
> +	}
> +
> +	se_tpg->enabled = enable;

This bool enable logic is a bit weird and splitting the enable and
disable case would seem more sensible to me, but maybe there is
something later on that makes it more relevant.
Dmitry Bogdanov Oct. 19, 2022, 2:21 p.m. UTC | #2
On Mon, Oct 17, 2022 at 12:38:50AM -0700, Christoph Hellwig wrote:
> 
> > +DEFINE_XARRAY_ALLOC(tpg_xa);
> 
> I think this wants to be marked static.

Agree.

> > +static int core_tpg_register_rtpi(struct se_portal_group *se_tpg)
> 
> Can you use target_ instead of the weird historic core_ prefixes for
> everything here?

Every function in this file is with core_ prefix, but OK, I will change
name of new functions to target_*.

> > +int core_tpg_enable(struct se_portal_group *se_tpg, bool enable)
> > +{
> > +     int ret;
> > +
> > +     if (enable) {
> > +             ret = core_tpg_register_rtpi(se_tpg);
> > +             if (ret)
> > +                     return ret;
> > +     } else {
> > +             core_tpg_deregister_rtpi(se_tpg);
> > +     }
> > +     ret = se_tpg->se_tpg_tfo->fabric_enable_tpg(se_tpg, enable);
> > +     if (ret) {
> > +             core_tpg_deregister_rtpi(se_tpg);
> > +             return ret;
> > +     }
> > +
> > +     se_tpg->enabled = enable;
> 
> This bool enable logic is a bit weird and splitting the enable and
> disable case would seem more sensible to me, but maybe there is
> something later on that makes it more relevant.

Ok, will split this function to enable/disable functions.

BR,
 Dmitry
diff mbox series

Patch

diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
index 95a88f6224cd..c3a41d4d646b 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -836,12 +836,9 @@  static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item,
 	if (se_tpg->enabled == op)
 		return count;
 
-	ret = se_tpg->se_tpg_tfo->fabric_enable_tpg(se_tpg, op);
+	ret = core_tpg_enable(se_tpg, op);
 	if (ret)
 		return ret;
-
-	se_tpg->enabled = op;
-
 	return count;
 }
 
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index 30fcf69e1a1d..fb699f336736 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -131,6 +131,7 @@  void core_tpg_remove_lun(struct se_portal_group *, struct se_lun *);
 struct se_node_acl *core_tpg_add_initiator_node_acl(struct se_portal_group *tpg,
 		const char *initiatorname);
 void core_tpg_del_initiator_node_acl(struct se_node_acl *acl);
+int core_tpg_enable(struct se_portal_group *se_tpg, bool enable);
 
 /* target_core_transport.c */
 int	init_se_kmem_caches(void);
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 736847c933e5..572241eca564 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -31,6 +31,7 @@ 
 #include "target_core_ua.h"
 
 extern struct se_device *g_lun0_dev;
+DEFINE_XARRAY_ALLOC(tpg_xa);
 
 /*	__core_tpg_get_initiator_node_acl():
  *
@@ -439,6 +440,40 @@  static void core_tpg_lun_ref_release(struct percpu_ref *ref)
 	complete(&lun->lun_shutdown_comp);
 }
 
+static int core_tpg_register_rtpi(struct se_portal_group *se_tpg)
+{
+	return xa_alloc(&tpg_xa, &se_tpg->tpg_rtpi, se_tpg,
+			       XA_LIMIT(1, USHRT_MAX), GFP_KERNEL);
+}
+
+static void core_tpg_deregister_rtpi(struct se_portal_group *se_tpg)
+{
+	if (se_tpg->tpg_rtpi && se_tpg->enabled)
+		xa_erase(&tpg_xa, se_tpg->tpg_rtpi);
+}
+
+int core_tpg_enable(struct se_portal_group *se_tpg, bool enable)
+{
+	int ret;
+
+	if (enable) {
+		ret = core_tpg_register_rtpi(se_tpg);
+		if (ret)
+			return ret;
+	} else {
+		core_tpg_deregister_rtpi(se_tpg);
+	}
+	ret = se_tpg->se_tpg_tfo->fabric_enable_tpg(se_tpg, enable);
+	if (ret) {
+		core_tpg_deregister_rtpi(se_tpg);
+		return ret;
+	}
+
+	se_tpg->enabled = enable;
+
+	return 0;
+}
+
 /* Does not change se_wwn->priv. */
 int core_tpg_register(
 	struct se_wwn *se_wwn,
@@ -535,6 +570,8 @@  int core_tpg_deregister(struct se_portal_group *se_tpg)
 		kfree_rcu(se_tpg->tpg_virt_lun0, rcu_head);
 	}
 
+	core_tpg_deregister_rtpi(se_tpg);
+
 	return 0;
 }
 EXPORT_SYMBOL(core_tpg_deregister);
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 8c920456edd9..261c5f5228de 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -903,6 +903,8 @@  struct se_portal_group {
 	 */
 	int			proto_id;
 	bool			enabled;
+	/* RELATIVE TARGET PORT IDENTIFIER */
+	u32			tpg_rtpi;
 	/* Used for PR SPEC_I_PT=1 and REGISTER_AND_MOVE */
 	atomic_t		tpg_pr_ref_count;
 	/* Spinlock for adding/removing ACLed Nodes */