* [PATCH v2 1/5] scsi: target: core: Add RTPI field to target port
2022-10-06 10:50 [PATCH v2 0/5] scsi: target: make RTPI an TPG identifier Dmitry Bogdanov
@ 2022-10-06 10:50 ` Dmitry Bogdanov
2022-10-17 7:38 ` Christoph Hellwig
2022-10-06 10:50 ` [PATCH v2 2/5] scsi: target: core: Use RTPI from " Dmitry Bogdanov
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Dmitry Bogdanov @ 2022-10-06 10:50 UTC (permalink / raw)
To: Martin Petersen, target-devel
Cc: Mike Christie, linux-scsi, linux, Dmitry Bogdanov
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(-)
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 */
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/5] scsi: target: core: Add RTPI field to target port
2022-10-06 10:50 ` [PATCH v2 1/5] scsi: target: core: Add RTPI field to target port Dmitry Bogdanov
@ 2022-10-17 7:38 ` Christoph Hellwig
2022-10-19 14:21 ` Dmitry Bogdanov
0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2022-10-17 7:38 UTC (permalink / raw)
To: Dmitry Bogdanov
Cc: Martin Petersen, target-devel, Mike Christie, linux-scsi, linux
> +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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/5] scsi: target: core: Add RTPI field to target port
2022-10-17 7:38 ` Christoph Hellwig
@ 2022-10-19 14:21 ` Dmitry Bogdanov
0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Bogdanov @ 2022-10-19 14:21 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin Petersen, target-devel, Mike Christie, linux-scsi, linux
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/5] scsi: target: core: Use RTPI from target port
2022-10-06 10:50 [PATCH v2 0/5] scsi: target: make RTPI an TPG identifier Dmitry Bogdanov
2022-10-06 10:50 ` [PATCH v2 1/5] scsi: target: core: Add RTPI field to target port Dmitry Bogdanov
@ 2022-10-06 10:50 ` Dmitry Bogdanov
2022-10-17 7:39 ` Christoph Hellwig
2022-10-06 10:50 ` [PATCH v2 3/5] scsi: target: core: Drop device-based RTPI Dmitry Bogdanov
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Dmitry Bogdanov @ 2022-10-06 10:50 UTC (permalink / raw)
To: Martin Petersen, target-devel
Cc: Mike Christie, linux-scsi, linux, Roman Bolshakov, Dmitry Bogdanov
From: Roman Bolshakov <r.bolshakov@yadro.com>
Replace all references to RTPI from LUN field to se_portal_group field.
It introduces consitent reporting of RTPI for all LUNs and all target
ports.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
---
drivers/target/target_core_alua.c | 4 ++--
drivers/target/target_core_device.c | 2 +-
drivers/target/target_core_pr.c | 8 ++++----
drivers/target/target_core_spc.c | 2 +-
drivers/target/target_core_stat.c | 6 +++---
5 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index c8470e7c0e10..3372856319f7 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -225,7 +225,7 @@ target_emulate_report_target_port_groups(struct se_cmd *cmd)
/*
* Set RELATIVE TARGET PORT IDENTIFIER
*/
- put_unaligned_be16(lun->lun_rtpi, &buf[off]);
+ put_unaligned_be16(lun->lun_tpg->tpg_rtpi, &buf[off]);
off += 2;
rd_len += 4;
}
@@ -399,7 +399,7 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
spin_lock(&dev->se_port_lock);
list_for_each_entry(lun, &dev->dev_sep_list,
lun_dev_link) {
- if (lun->lun_rtpi != rtpi)
+ if (lun->lun_tpg->tpg_rtpi != rtpi)
continue;
// XXX: racy unlock
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index b7f16ee8aa0e..ad83338d6140 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -223,7 +223,7 @@ struct se_dev_entry *core_get_se_deve_from_rtpi(
tpg->se_tpg_tfo->fabric_name);
continue;
}
- if (lun->lun_rtpi != rtpi)
+ if (lun->lun_tpg->tpg_rtpi != rtpi)
continue;
kref_get(&deve->pr_kref);
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index a1d67554709f..ed0906fd7965 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -663,7 +663,7 @@ static struct t10_pr_registration *__core_scsi3_do_alloc_registration(
}
pr_reg->pr_res_mapped_lun = mapped_lun;
pr_reg->pr_aptpl_target_lun = lun->unpacked_lun;
- pr_reg->tg_pt_sep_rtpi = lun->lun_rtpi;
+ pr_reg->tg_pt_sep_rtpi = lun->lun_tpg->tpg_rtpi;
pr_reg->pr_res_key = sa_res_key;
pr_reg->pr_reg_all_tg_pt = all_tg_pt;
pr_reg->pr_reg_aptpl = aptpl;
@@ -967,7 +967,7 @@ static int __core_scsi3_check_aptpl_registration(
rcu_read_unlock();
pr_reg->pr_reg_nacl = nacl;
- pr_reg->tg_pt_sep_rtpi = lun->lun_rtpi;
+ pr_reg->tg_pt_sep_rtpi = lun->lun_tpg->tpg_rtpi;
list_del(&pr_reg->pr_reg_aptpl_list);
spin_unlock(&pr_tmpl->aptpl_reg_lock);
/*
@@ -1567,7 +1567,7 @@ core_scsi3_decode_spec_i_port(
*/
if (tmp_tpg->proto_id != proto_ident)
continue;
- dest_rtpi = tmp_lun->lun_rtpi;
+ dest_rtpi = tmp_lun->lun_tpg->tpg_rtpi;
iport_ptr = NULL;
i_str = target_parse_pr_out_transport_id(tmp_tpg,
@@ -3210,7 +3210,7 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
spin_lock(&dev->se_port_lock);
list_for_each_entry(tmp_lun, &dev->dev_sep_list, lun_dev_link) {
- if (tmp_lun->lun_rtpi != rtpi)
+ if (tmp_lun->lun_tpg->tpg_rtpi != rtpi)
continue;
dest_se_tpg = tmp_lun->lun_tpg;
dest_tf_ops = dest_se_tpg->se_tpg_tfo;
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 7cca3b15472b..b045b33983b8 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -321,7 +321,7 @@ spc_emulate_evpd_83(struct se_cmd *cmd, unsigned char *buf)
/* Skip over Obsolete field in RTPI payload
* in Table 472 */
off += 2;
- put_unaligned_be16(lun->lun_rtpi, &buf[off]);
+ put_unaligned_be16(lun->lun_tpg->tpg_rtpi, &buf[off]);
off += 2;
len += 8; /* Header size + Designation descriptor */
/*
diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c
index f85ee5b0fd80..c42cbde8a31b 100644
--- a/drivers/target/target_core_stat.c
+++ b/drivers/target/target_core_stat.c
@@ -455,7 +455,7 @@ static ssize_t target_stat_port_indx_show(struct config_item *item, char *page)
rcu_read_lock();
dev = rcu_dereference(lun->lun_se_dev);
if (dev)
- ret = snprintf(page, PAGE_SIZE, "%u\n", lun->lun_rtpi);
+ ret = snprintf(page, PAGE_SIZE, "%u\n", lun->lun_tpg->tpg_rtpi);
rcu_read_unlock();
return ret;
}
@@ -561,7 +561,7 @@ static ssize_t target_stat_tgt_port_indx_show(struct config_item *item,
rcu_read_lock();
dev = rcu_dereference(lun->lun_se_dev);
if (dev)
- ret = snprintf(page, PAGE_SIZE, "%u\n", lun->lun_rtpi);
+ ret = snprintf(page, PAGE_SIZE, "%u\n", lun->lun_tpg->tpg_rtpi);
rcu_read_unlock();
return ret;
}
@@ -579,7 +579,7 @@ static ssize_t target_stat_tgt_port_name_show(struct config_item *item,
if (dev)
ret = snprintf(page, PAGE_SIZE, "%sPort#%u\n",
tpg->se_tpg_tfo->fabric_name,
- lun->lun_rtpi);
+ lun->lun_tpg->tpg_rtpi);
rcu_read_unlock();
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/5] scsi: target: core: Drop device-based RTPI
2022-10-06 10:50 [PATCH v2 0/5] scsi: target: make RTPI an TPG identifier Dmitry Bogdanov
2022-10-06 10:50 ` [PATCH v2 1/5] scsi: target: core: Add RTPI field to target port Dmitry Bogdanov
2022-10-06 10:50 ` [PATCH v2 2/5] scsi: target: core: Use RTPI from " Dmitry Bogdanov
@ 2022-10-06 10:50 ` Dmitry Bogdanov
2022-10-17 7:39 ` Christoph Hellwig
2022-10-06 10:50 ` [PATCH v2 4/5] scsi: target: core: Add common port attributes Dmitry Bogdanov
2022-10-06 10:50 ` [PATCH v2 5/5] scsi: target: core: Add RTPI attribute for target port Dmitry Bogdanov
4 siblings, 1 reply; 12+ messages in thread
From: Dmitry Bogdanov @ 2022-10-06 10:50 UTC (permalink / raw)
To: Martin Petersen, target-devel
Cc: Mike Christie, linux-scsi, linux, Roman Bolshakov, Dmitry Bogdanov
From: Roman Bolshakov <r.bolshakov@yadro.com>
The code is not needed since target port-based RTPI allocation replaced
it.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
---
drivers/target/target_core_device.c | 41 ---------------------------
drivers/target/target_core_internal.h | 1 -
drivers/target/target_core_tpg.c | 6 ----
include/target/target_core_base.h | 4 ---
4 files changed, 52 deletions(-)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index ad83338d6140..2209660261d0 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -460,47 +460,6 @@ void core_clear_lun_from_tpg(struct se_lun *lun, struct se_portal_group *tpg)
mutex_unlock(&tpg->acl_node_mutex);
}
-int core_alloc_rtpi(struct se_lun *lun, struct se_device *dev)
-{
- struct se_lun *tmp;
-
- spin_lock(&dev->se_port_lock);
- if (dev->export_count == 0x0000ffff) {
- pr_warn("Reached dev->dev_port_count =="
- " 0x0000ffff\n");
- spin_unlock(&dev->se_port_lock);
- return -ENOSPC;
- }
-again:
- /*
- * Allocate the next RELATIVE TARGET PORT IDENTIFIER for this struct se_device
- * Here is the table from spc4r17 section 7.7.3.8.
- *
- * Table 473 -- RELATIVE TARGET PORT IDENTIFIER field
- *
- * Code Description
- * 0h Reserved
- * 1h Relative port 1, historically known as port A
- * 2h Relative port 2, historically known as port B
- * 3h to FFFFh Relative port 3 through 65 535
- */
- lun->lun_rtpi = dev->dev_rpti_counter++;
- if (!lun->lun_rtpi)
- goto again;
-
- list_for_each_entry(tmp, &dev->dev_sep_list, lun_dev_link) {
- /*
- * Make sure RELATIVE TARGET PORT IDENTIFIER is unique
- * for 16-bit wrap..
- */
- if (lun->lun_rtpi == tmp->lun_rtpi)
- goto again;
- }
- spin_unlock(&dev->se_port_lock);
-
- return 0;
-}
-
static void se_release_vpd_for_dev(struct se_device *dev)
{
struct t10_vpd *vpd, *vpd_tmp;
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index fb699f336736..4699c078f6f4 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -59,7 +59,6 @@ struct target_fabric_configfs {
extern struct t10_alua_lu_gp *default_lu_gp;
/* target_core_device.c */
-int core_alloc_rtpi(struct se_lun *lun, struct se_device *dev);
struct se_dev_entry *core_get_se_deve_from_rtpi(struct se_node_acl *, u16);
void target_pr_kref_release(struct kref *);
void core_free_device_list_for_node(struct se_node_acl *,
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 572241eca564..5838cd2d8a96 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -615,10 +615,6 @@ int core_tpg_add_lun(
if (ret < 0)
goto out;
- ret = core_alloc_rtpi(lun, dev);
- if (ret)
- goto out_kill_ref;
-
if (!(dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA) &&
!(dev->se_hba->hba_flags & HBA_FLAGS_INTERNAL_USE))
target_attach_tg_pt_gp(lun, dev->t10_alua.default_tg_pt_gp);
@@ -642,8 +638,6 @@ int core_tpg_add_lun(
return 0;
-out_kill_ref:
- percpu_ref_exit(&lun->lun_ref);
out:
return ret;
}
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 261c5f5228de..83b396457ed1 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -733,8 +733,6 @@ struct se_lun {
bool lun_access_ro;
u32 lun_index;
- /* RELATIVE TARGET PORT IDENTIFER */
- u16 lun_rtpi;
atomic_t lun_acl_count;
struct se_device __rcu *lun_se_dev;
@@ -786,8 +784,6 @@ struct se_device_queue {
};
struct se_device {
- /* RELATIVE TARGET PORT IDENTIFER Counter */
- u16 dev_rpti_counter;
/* Used for SAM Task Attribute ordering */
u32 dev_cur_ordered_id;
u32 dev_flags;
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/5] scsi: target: core: Add common port attributes
2022-10-06 10:50 [PATCH v2 0/5] scsi: target: make RTPI an TPG identifier Dmitry Bogdanov
` (2 preceding siblings ...)
2022-10-06 10:50 ` [PATCH v2 3/5] scsi: target: core: Drop device-based RTPI Dmitry Bogdanov
@ 2022-10-06 10:50 ` Dmitry Bogdanov
2022-10-17 7:42 ` Christoph Hellwig
2022-10-06 10:50 ` [PATCH v2 5/5] scsi: target: core: Add RTPI attribute for target port Dmitry Bogdanov
4 siblings, 1 reply; 12+ messages in thread
From: Dmitry Bogdanov @ 2022-10-06 10:50 UTC (permalink / raw)
To: Martin Petersen, target-devel
Cc: Mike Christie, linux-scsi, linux, Roman Bolshakov, Dmitry Bogdanov
From: Roman Bolshakov <r.bolshakov@yadro.com>
All SCSI target port attributes (tpgN/attribs/attrname) are defined and
implemented in fabric modules in existing implementation.
The change introduces a way to have common tpg attribs in configfs for
all fabrics without code duplication.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
---
drivers/target/target_core_configfs.c | 9 ++++-
drivers/target/target_core_fabric_configfs.c | 39 +++++++++++++++++++-
drivers/target/target_core_internal.h | 1 +
drivers/target/target_core_tpg.c | 4 ++
4 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index 416514c5c7ac..ca728852e2bb 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -464,13 +464,19 @@ int target_register_template(const struct target_core_fabric_ops *fo)
INIT_LIST_HEAD(&tf->tf_list);
atomic_set(&tf->tf_access_cnt, 0);
tf->tf_ops = fo;
- target_fabric_setup_cits(tf);
+ ret = target_fabric_setup_cits(tf);
+ if (ret)
+ goto out;
mutex_lock(&g_tf_lock);
list_add_tail(&tf->tf_list, &g_tf_list);
mutex_unlock(&g_tf_lock);
return 0;
+
+out:
+ kfree(tf);
+ return ret;
}
EXPORT_SYMBOL(target_register_template);
@@ -491,6 +497,7 @@ void target_unregister_template(const struct target_core_fabric_ops *fo)
*/
rcu_barrier();
kfree(t->tf_tpg_base_cit.ct_attrs);
+ kfree(t->tf_tpg_attrib_cit.ct_attrs);
kfree(t);
return;
}
diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
index c3a41d4d646b..810e8abc0b00 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -795,7 +795,38 @@ TF_CIT_SETUP(tpg_lun, NULL, &target_fabric_lun_group_ops, NULL);
/* End of tfc_tpg_lun_cit */
-TF_CIT_SETUP_DRV(tpg_attrib, NULL, NULL);
+static int
+target_fabric_setup_tpg_attrib_cit(struct target_fabric_configfs *tf)
+{
+ int i, k, len = 0;
+ struct config_item_type *cit = &tf->tf_tpg_attrib_cit;
+ struct configfs_attribute **attrs;
+
+ for (i = 0; core_tpg_attrib_attrs[i]; i++)
+ len += sizeof(struct configfs_attribute *);
+ if (tf->tf_ops->tfc_tpg_attrib_attrs)
+ for (i = 0; tf->tf_ops->tfc_tpg_attrib_attrs[i]; i++)
+ len += sizeof(struct configfs_attribute *);
+ len += sizeof(struct configfs_attribute *);
+
+ attrs = kzalloc(len, GFP_KERNEL);
+ if (!attrs)
+ return -ENOMEM;
+
+ for (i = 0; core_tpg_attrib_attrs[i]; i++)
+ attrs[i] = core_tpg_attrib_attrs[i];
+ if (tf->tf_ops->tfc_tpg_attrib_attrs)
+ for (k = 0; tf->tf_ops->tfc_tpg_attrib_attrs[k]; k++, i++)
+ attrs[i] = tf->tf_ops->tfc_tpg_attrib_attrs[k];
+ attrs[i] = NULL;
+
+ cit->ct_attrs = attrs;
+ cit->ct_owner = tf->tf_ops->module;
+ pr_debug("Setup generic tpg_attrib\n");
+
+ return 0;
+}
+
TF_CIT_SETUP_DRV(tpg_auth, NULL, NULL);
TF_CIT_SETUP_DRV(tpg_param, NULL, NULL);
@@ -1110,7 +1141,11 @@ int target_fabric_setup_cits(struct target_fabric_configfs *tf)
target_fabric_setup_tpg_lun_cit(tf);
target_fabric_setup_tpg_np_cit(tf);
target_fabric_setup_tpg_np_base_cit(tf);
- target_fabric_setup_tpg_attrib_cit(tf);
+
+ ret = target_fabric_setup_tpg_attrib_cit(tf);
+ if (ret)
+ return ret;
+
target_fabric_setup_tpg_auth_cit(tf);
target_fabric_setup_tpg_param_cit(tf);
target_fabric_setup_tpg_nacl_cit(tf);
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index 4699c078f6f4..06d87008615f 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -117,6 +117,7 @@ int core_tmr_lun_reset(struct se_device *, struct se_tmr_req *,
/* target_core_tpg.c */
extern struct se_device *g_lun0_dev;
+extern struct configfs_attribute *core_tpg_attrib_attrs[];
struct se_node_acl *__core_tpg_get_initiator_node_acl(struct se_portal_group *tpg,
const char *);
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 5838cd2d8a96..25ffc3d7a741 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -681,3 +681,7 @@ void core_tpg_remove_lun(
percpu_ref_exit(&lun->lun_ref);
}
+
+struct configfs_attribute *core_tpg_attrib_attrs[] = {
+ NULL,
+};
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/5] scsi: target: core: Add common port attributes
2022-10-06 10:50 ` [PATCH v2 4/5] scsi: target: core: Add common port attributes Dmitry Bogdanov
@ 2022-10-17 7:42 ` Christoph Hellwig
2022-10-19 15:13 ` Dmitry Bogdanov
0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2022-10-17 7:42 UTC (permalink / raw)
To: Dmitry Bogdanov
Cc: Martin Petersen, target-devel, Mike Christie, linux-scsi, linux,
Roman Bolshakov
On Thu, Oct 06, 2022 at 01:50:56PM +0300, Dmitry Bogdanov wrote:
> From: Roman Bolshakov <r.bolshakov@yadro.com>
>
> All SCSI target port attributes (tpgN/attribs/attrname) are defined and
> implemented in fabric modules in existing implementation.
>
> The change introduces a way to have common tpg attribs in configfs for
> all fabrics without code duplication.
But does it really buy you much? There is quite a lot of boilerplate
code vs just adding an attribute to an array in the drivers, which could
be further simplified by having a macro for all common ones.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/5] scsi: target: core: Add common port attributes
2022-10-17 7:42 ` Christoph Hellwig
@ 2022-10-19 15:13 ` Dmitry Bogdanov
0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Bogdanov @ 2022-10-19 15:13 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin Petersen, target-devel, Mike Christie, linux-scsi, linux,
Roman Bolshakov
On Mon, Oct 17, 2022 at 12:42:16AM -0700, Christoph Hellwig wrote:
>
> On Thu, Oct 06, 2022 at 01:50:56PM +0300, Dmitry Bogdanov wrote:
> > From: Roman Bolshakov <r.bolshakov@yadro.com>
> >
> > All SCSI target port attributes (tpgN/attribs/attrname) are defined and
> > implemented in fabric modules in existing implementation.
> >
> > The change introduces a way to have common tpg attribs in configfs for
> > all fabrics without code duplication.
>
> But does it really buy you much? There is quite a lot of boilerplate
> code vs just adding an attribute to an array in the drivers, which could
> be further simplified by having a macro for all common ones.
Yes, of course. Mostly it is not about deduplication, but for just a
logical place of the attribute. Without this patch the 5th patch would
be cloned for every fabric driver. Moreover it would look strange to
have a some private core logic (check enableness) in fabric driver.
BR,
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 5/5] scsi: target: core: Add RTPI attribute for target port
2022-10-06 10:50 [PATCH v2 0/5] scsi: target: make RTPI an TPG identifier Dmitry Bogdanov
` (3 preceding siblings ...)
2022-10-06 10:50 ` [PATCH v2 4/5] scsi: target: core: Add common port attributes Dmitry Bogdanov
@ 2022-10-06 10:50 ` Dmitry Bogdanov
4 siblings, 0 replies; 12+ messages in thread
From: Dmitry Bogdanov @ 2022-10-06 10:50 UTC (permalink / raw)
To: Martin Petersen, target-devel
Cc: Mike Christie, linux-scsi, linux, Dmitry Bogdanov, Roman Bolshakov
RELATIVE TARGET PORT IDENTIFIER can be read and configured via configfs:
$ echo 0x10 > $TARGET/tpgt_N/attrib/rtpi
RTPI can be changed only on disabled target ports.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
---
v2:
do not allow to change RTPI on enabled target port
---
drivers/target/target_core_tpg.c | 53 ++++++++++++++++++++++++++++++-
include/target/target_core_base.h | 1 +
2 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 25ffc3d7a741..09b37ba005ff 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -442,8 +442,23 @@ static void core_tpg_lun_ref_release(struct percpu_ref *ref)
static int core_tpg_register_rtpi(struct se_portal_group *se_tpg)
{
- return xa_alloc(&tpg_xa, &se_tpg->tpg_rtpi, se_tpg,
+ int ret;
+
+ if (se_tpg->rtpi_manual) {
+ ret = xa_insert(&tpg_xa, se_tpg->tpg_rtpi, se_tpg, GFP_KERNEL);
+ if (ret) {
+ pr_info("%s_TPG[%hu] - Can not set RTPI %#x, it is already busy",
+ se_tpg->se_tpg_tfo->fabric_name,
+ se_tpg->se_tpg_tfo->tpg_get_tag(se_tpg),
+ se_tpg->tpg_rtpi);
+ return -EINVAL;
+ }
+ } else {
+ ret = xa_alloc(&tpg_xa, &se_tpg->tpg_rtpi, se_tpg,
XA_LIMIT(1, USHRT_MAX), GFP_KERNEL);
+ }
+
+ return ret;
}
static void core_tpg_deregister_rtpi(struct se_portal_group *se_tpg)
@@ -682,6 +697,42 @@ void core_tpg_remove_lun(
percpu_ref_exit(&lun->lun_ref);
}
+static ssize_t core_tpg_rtpi_show(struct config_item *item, char *page)
+{
+ struct se_portal_group *se_tpg = attrib_to_tpg(item);
+
+ return sysfs_emit(page, "%#x\n", se_tpg->tpg_rtpi);
+}
+
+static ssize_t core_tpg_rtpi_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct se_portal_group *se_tpg = attrib_to_tpg(item);
+ u16 val;
+ int ret;
+
+ ret = kstrtou16(page, 0, &val);
+ if (ret < 0)
+ return ret;
+ if (val == 0)
+ return -EINVAL;
+
+ if (se_tpg->enabled) {
+ pr_info("%s_TPG[%hu] - Can not change RTPI on enabled TPG",
+ se_tpg->se_tpg_tfo->fabric_name,
+ se_tpg->se_tpg_tfo->tpg_get_tag(se_tpg));
+ return -EINVAL;
+ }
+
+ se_tpg->tpg_rtpi = val;
+ se_tpg->rtpi_manual = true;
+
+ return count;
+}
+
+CONFIGFS_ATTR(core_tpg_, rtpi);
+
struct configfs_attribute *core_tpg_attrib_attrs[] = {
+ &core_tpg_attr_rtpi,
NULL,
};
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 83b396457ed1..f2375280d6a4 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -901,6 +901,7 @@ struct se_portal_group {
bool enabled;
/* RELATIVE TARGET PORT IDENTIFIER */
u32 tpg_rtpi;
+ bool rtpi_manual;
/* Used for PR SPEC_I_PT=1 and REGISTER_AND_MOVE */
atomic_t tpg_pr_ref_count;
/* Spinlock for adding/removing ACLed Nodes */
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread