| Message ID | 20221006105057.30184-2-d.bogdanov@yadro.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi: target: make RTPI an TPG identifier | expand |
> +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.
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 --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 */
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(-)