| Message ID | 20220906154519.27487-8-d.bogdanov@yadro.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: target: make RTPI an TPG identifier | expand |
On 9/6/22 10:45 AM, Dmitry Bogdanov wrote: > Garantee uniquity of RTPI only for enabled target port groups. > Allow any RPTI for disabled tpg until it is enabled. > > Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> > --- > drivers/target/target_core_fabric_configfs.c | 29 +++++++++++++- > drivers/target/target_core_internal.h | 2 + > drivers/target/target_core_tpg.c | 40 +++++++++++++------- > 3 files changed, 56 insertions(+), 15 deletions(-) > > diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c > index a34b5db4eec5..fc1b8f54fb54 100644 > --- a/drivers/target/target_core_fabric_configfs.c > +++ b/drivers/target/target_core_fabric_configfs.c > @@ -857,6 +857,7 @@ static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item, > size_t count) > { > struct se_portal_group *se_tpg = to_tpg(item); > + struct se_portal_group *tpg; > int ret; > bool op; > > @@ -867,11 +868,37 @@ static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item, > if (se_tpg->enabled == op) > return count; > > + spin_lock(&g_tpg_lock); > + > + if (op) { > + tpg = core_get_tpg_by_rtpi(se_tpg->tpg_rtpi); > + if (tpg) { > + spin_unlock(&g_tpg_lock); > + > + pr_err("TARGET_CORE[%s]->TPG[%u] - RTPI %#x conflicts with TARGET_CORE[%s]->TPG[%u]\n", > + se_tpg->se_tpg_tfo->fabric_name, > + se_tpg->se_tpg_tfo->tpg_get_tag(tpg), > + se_tpg->tpg_rtpi, > + tpg->se_tpg_tfo->fabric_name, > + tpg->se_tpg_tfo->tpg_get_tag(tpg)); > + return -EINVAL; > + } > + } > + > + se_tpg->enabled |= 0x10; /* transient state */ Just use a mutex and hold it the entire time if you can and drop this. Or add a proper enum/define for the states and make a real API since this is exported to userspace, and it's going to be difficult to explain to users that state. > + spin_unlock(&g_tpg_lock); > + > ret = se_tpg->se_tpg_tfo->fabric_enable_tpg(se_tpg, op); > - if (ret) > + > + spin_lock(&g_tpg_lock); > + if (ret < 0) { > + se_tpg->enabled &= ~0x10; /* clear transient state */ > + spin_unlock(&g_tpg_lock); > return ret; > + } > > se_tpg->enabled = op; > + spin_unlock(&g_tpg_lock); > > return count; > } > diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h > index d662cdc9a04c..d4ace697edb0 100644 > --- a/drivers/target/target_core_internal.h > +++ b/drivers/target/target_core_internal.h > @@ -116,6 +116,7 @@ int core_tmr_lun_reset(struct se_device *, struct se_tmr_req *, > struct list_head *, struct se_cmd *); > > /* target_core_tpg.c */ > +extern struct spinlock g_tpg_lock; > extern struct se_device *g_lun0_dev; > extern struct configfs_attribute *core_tpg_attrib_attrs[]; > > @@ -131,6 +132,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); > +struct se_portal_group *core_get_tpg_by_rtpi(u16 rtpi); > > /* target_core_transport.c */ > extern struct kmem_cache *se_tmr_req_cache; > diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c > index 85c9473a38fd..ef2adbe628e0 100644 > --- a/drivers/target/target_core_tpg.c > +++ b/drivers/target/target_core_tpg.c > @@ -34,7 +34,7 @@ extern struct se_device *g_lun0_dev; > static u16 g_tpg_count; > static u16 g_tpg_rtpi_counter = 1; > static LIST_HEAD(g_tpg_list); > -static DEFINE_SPINLOCK(g_tpg_lock); > +DEFINE_SPINLOCK(g_tpg_lock); > > /* __core_tpg_get_initiator_node_acl(): > * > @@ -476,7 +476,7 @@ static int core_tpg_register_rtpi(struct se_portal_group *se_tpg) > * Make sure RELATIVE TARGET PORT IDENTIFIER is unique > * for 16-bit wrap.. > */ > - if (se_tpg->tpg_rtpi == tpg->tpg_rtpi) > + if (tpg->enabled && se_tpg->tpg_rtpi == tpg->tpg_rtpi) > goto again; > } > list_add(&se_tpg->tpg_list, &g_tpg_list); > @@ -738,12 +738,26 @@ static ssize_t core_tpg_rtpi_show(struct config_item *item, char *page) > return sprintf(page, "%#x\n", se_tpg->tpg_rtpi); > } > > +struct se_portal_group * > +core_get_tpg_by_rtpi(u16 rtpi) > +{ > + struct se_portal_group *tpg; > + > + lockdep_assert_held(&g_tpg_lock); > + > + list_for_each_entry(tpg, &g_tpg_list, tpg_list) { > + if (tpg->enabled && rtpi == tpg->tpg_rtpi) > + return tpg; > + } > + > + return NULL; > +} > + > 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); > struct se_portal_group *tpg; > - bool rtpi_changed = false; > u16 val; > int ret; > > @@ -753,11 +767,14 @@ static ssize_t core_tpg_rtpi_store(struct config_item *item, > if (val == 0) > return -EINVAL; > > - /* RTPI shouldn't conflict with values of existing ports */ > + if (se_tpg->tpg_rtpi == val) > + return count; This test above and the chunk at the end looks like it should have been in patch 6. > + > spin_lock(&g_tpg_lock); > > - list_for_each_entry(tpg, &g_tpg_list, tpg_list) { > - if (se_tpg != tpg && val == tpg->tpg_rtpi) { > + if (se_tpg->enabled) { > + tpg = core_get_tpg_by_rtpi(val); > + if (tpg) { > spin_unlock(&g_tpg_lock); > pr_err("TARGET_CORE[%s]->TPG[%u] - RTPI %#x conflicts with TARGET_CORE[%s]->TPG[%u]\n", > se_tpg->se_tpg_tfo->fabric_name, > @@ -769,17 +786,12 @@ static ssize_t core_tpg_rtpi_store(struct config_item *item, > } > } > > - if (se_tpg->tpg_rtpi != val) { > - se_tpg->tpg_rtpi = val; > - rtpi_changed = true; > - } > + se_tpg->tpg_rtpi = val; > spin_unlock(&g_tpg_lock); > > - if (rtpi_changed) > - core_tpg_ua(se_tpg, 0x3f, ASCQ_3FH_INQUIRY_DATA_HAS_CHANGED); > - ret = count; > + core_tpg_ua(se_tpg, 0x3f, ASCQ_3FH_INQUIRY_DATA_HAS_CHANGED); > > - return ret; > + return count; > } > > CONFIGFS_ATTR(core_tpg_, rtpi);
On 9/29/22 7:02 PM, Mike Christie wrote: > On 9/6/22 10:45 AM, Dmitry Bogdanov wrote: >> Garantee uniquity of RTPI only for enabled target port groups. >> Allow any RPTI for disabled tpg until it is enabled. >> >> Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> >> --- >> drivers/target/target_core_fabric_configfs.c | 29 +++++++++++++- >> drivers/target/target_core_internal.h | 2 + >> drivers/target/target_core_tpg.c | 40 +++++++++++++------- >> 3 files changed, 56 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c >> index a34b5db4eec5..fc1b8f54fb54 100644 >> --- a/drivers/target/target_core_fabric_configfs.c >> +++ b/drivers/target/target_core_fabric_configfs.c >> @@ -857,6 +857,7 @@ static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item, >> size_t count) >> { >> struct se_portal_group *se_tpg = to_tpg(item); >> + struct se_portal_group *tpg; >> int ret; >> bool op; >> >> @@ -867,11 +868,37 @@ static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item, >> if (se_tpg->enabled == op) >> return count; >> >> + spin_lock(&g_tpg_lock); >> + >> + if (op) { >> + tpg = core_get_tpg_by_rtpi(se_tpg->tpg_rtpi); >> + if (tpg) { >> + spin_unlock(&g_tpg_lock); >> + >> + pr_err("TARGET_CORE[%s]->TPG[%u] - RTPI %#x conflicts with TARGET_CORE[%s]->TPG[%u]\n", >> + se_tpg->se_tpg_tfo->fabric_name, >> + se_tpg->se_tpg_tfo->tpg_get_tag(tpg), >> + se_tpg->tpg_rtpi, >> + tpg->se_tpg_tfo->fabric_name, >> + tpg->se_tpg_tfo->tpg_get_tag(tpg)); >> + return -EINVAL; >> + } >> + } >> + >> + se_tpg->enabled |= 0x10; /* transient state */ > > Just use a mutex and hold it the entire time if I was looking at the configfs code and am now not sure what the transient state is for. It looks like when doing a read or write configfs holds the buffer->mutex, so I don't think userspace would ever see the transient state. Can you just drop it?
On Thu, Sep 29, 2022 at 07:02:12PM -0500, Mike Christie wrote: > > On 9/6/22 10:45 AM, Dmitry Bogdanov wrote: > > Garantee uniquity of RTPI only for enabled target port groups. > > Allow any RPTI for disabled tpg until it is enabled. > > > > Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> > > --- > > drivers/target/target_core_fabric_configfs.c | 29 +++++++++++++- > > drivers/target/target_core_internal.h | 2 + > > drivers/target/target_core_tpg.c | 40 +++++++++++++------- > > 3 files changed, 56 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c > > index a34b5db4eec5..fc1b8f54fb54 100644 > > --- a/drivers/target/target_core_fabric_configfs.c > > +++ b/drivers/target/target_core_fabric_configfs.c > > @@ -857,6 +857,7 @@ static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item, > > size_t count) > > { > > struct se_portal_group *se_tpg = to_tpg(item); > > + struct se_portal_group *tpg; > > int ret; > > bool op; > > > > @@ -867,11 +868,37 @@ static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item, > > if (se_tpg->enabled == op) > > return count; > > > > + spin_lock(&g_tpg_lock); > > + > > + if (op) { > > + tpg = core_get_tpg_by_rtpi(se_tpg->tpg_rtpi); > > + if (tpg) { > > + spin_unlock(&g_tpg_lock); > > + > > + pr_err("TARGET_CORE[%s]->TPG[%u] - RTPI %#x conflicts with TARGET_CORE[%s]->TPG[%u]\n", > > + se_tpg->se_tpg_tfo->fabric_name, > > + se_tpg->se_tpg_tfo->tpg_get_tag(tpg), > > + se_tpg->tpg_rtpi, > > + tpg->se_tpg_tfo->fabric_name, > > + tpg->se_tpg_tfo->tpg_get_tag(tpg)); > > + return -EINVAL; > > + } > > + } > > + > > + se_tpg->enabled |= 0x10; /* transient state */ > > Just use a mutex and hold it the entire time if you can and > drop this. Or add a proper enum/define for the states and make a real > API since this is exported to userspace, and it's going to be difficult > to explain to users that state. Yes, it looks wierd. After rewriting to xarray I decided (according to SAM-5) to not allow to change RTPI on the enabled TPG. And that part will be dropped. 4.6.5.2 Relative Port Identifier attribute The relative port identifier for a SCSI port shall not change once assigned unless reconfiguration of the SCSI target device occurs. > > > + spin_unlock(&g_tpg_lock); > > + > > ret = se_tpg->se_tpg_tfo->fabric_enable_tpg(se_tpg, op); > > - if (ret) > > + > > + spin_lock(&g_tpg_lock); > > + if (ret < 0) { > > + se_tpg->enabled &= ~0x10; /* clear transient state */ > > + spin_unlock(&g_tpg_lock); > > return ret; > > + } > > > > se_tpg->enabled = op; > > + spin_unlock(&g_tpg_lock); > > > > return count; > > } > > diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h > > index d662cdc9a04c..d4ace697edb0 100644 > > --- a/drivers/target/target_core_internal.h > > +++ b/drivers/target/target_core_internal.h > > @@ -116,6 +116,7 @@ int core_tmr_lun_reset(struct se_device *, struct se_tmr_req *, > > struct list_head *, struct se_cmd *); > > > > /* target_core_tpg.c */ > > +extern struct spinlock g_tpg_lock; > > extern struct se_device *g_lun0_dev; > > extern struct configfs_attribute *core_tpg_attrib_attrs[]; > > > > @@ -131,6 +132,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); > > +struct se_portal_group *core_get_tpg_by_rtpi(u16 rtpi); > > > > /* target_core_transport.c */ > > extern struct kmem_cache *se_tmr_req_cache; > > diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c > > index 85c9473a38fd..ef2adbe628e0 100644 > > --- a/drivers/target/target_core_tpg.c > > +++ b/drivers/target/target_core_tpg.c > > @@ -34,7 +34,7 @@ extern struct se_device *g_lun0_dev; > > static u16 g_tpg_count; > > static u16 g_tpg_rtpi_counter = 1; > > static LIST_HEAD(g_tpg_list); > > -static DEFINE_SPINLOCK(g_tpg_lock); > > +DEFINE_SPINLOCK(g_tpg_lock); > > > > /* __core_tpg_get_initiator_node_acl(): > > * > > @@ -476,7 +476,7 @@ static int core_tpg_register_rtpi(struct se_portal_group *se_tpg) > > * Make sure RELATIVE TARGET PORT IDENTIFIER is unique > > * for 16-bit wrap.. > > */ > > - if (se_tpg->tpg_rtpi == tpg->tpg_rtpi) > > + if (tpg->enabled && se_tpg->tpg_rtpi == tpg->tpg_rtpi) > > goto again; > > } > > list_add(&se_tpg->tpg_list, &g_tpg_list); > > @@ -738,12 +738,26 @@ static ssize_t core_tpg_rtpi_show(struct config_item *item, char *page) > > return sprintf(page, "%#x\n", se_tpg->tpg_rtpi); > > } > > > > +struct se_portal_group * > > +core_get_tpg_by_rtpi(u16 rtpi) > > +{ > > + struct se_portal_group *tpg; > > + > > + lockdep_assert_held(&g_tpg_lock); > > + > > + list_for_each_entry(tpg, &g_tpg_list, tpg_list) { > > + if (tpg->enabled && rtpi == tpg->tpg_rtpi) > > + return tpg; > > + } > > + > > + return NULL; > > +} > > + > > 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); > > struct se_portal_group *tpg; > > - bool rtpi_changed = false; > > u16 val; > > int ret; > > > > @@ -753,11 +767,14 @@ static ssize_t core_tpg_rtpi_store(struct config_item *item, > > if (val == 0) > > return -EINVAL; > > > > - /* RTPI shouldn't conflict with values of existing ports */ > > + if (se_tpg->tpg_rtpi == val) > > + return count; > > This test above and the chunk at the end looks like it should have been > in patch 6. > This patch will be completely rewritten. > > + > > spin_lock(&g_tpg_lock); > > > > - list_for_each_entry(tpg, &g_tpg_list, tpg_list) { > > - if (se_tpg != tpg && val == tpg->tpg_rtpi) { > > + if (se_tpg->enabled) { > > + tpg = core_get_tpg_by_rtpi(val); > > + if (tpg) { > > spin_unlock(&g_tpg_lock); > > pr_err("TARGET_CORE[%s]->TPG[%u] - RTPI %#x conflicts with TARGET_CORE[%s]->TPG[%u]\n", > > se_tpg->se_tpg_tfo->fabric_name, > > @@ -769,17 +786,12 @@ static ssize_t core_tpg_rtpi_store(struct config_item *item, > > } > > } > > > > - if (se_tpg->tpg_rtpi != val) { > > - se_tpg->tpg_rtpi = val; > > - rtpi_changed = true; > > - } > > + se_tpg->tpg_rtpi = val; > > spin_unlock(&g_tpg_lock); > > > > - if (rtpi_changed) > > - core_tpg_ua(se_tpg, 0x3f, ASCQ_3FH_INQUIRY_DATA_HAS_CHANGED); > > - ret = count; > > + core_tpg_ua(se_tpg, 0x3f, ASCQ_3FH_INQUIRY_DATA_HAS_CHANGED); > > > > - return ret; > > + return count; > > } > > > > CONFIGFS_ATTR(core_tpg_, rtpi); >
On Sat, Oct 01, 2022 at 11:19:45AM -0500, michael.christie@oracle.com wrote: > > On 9/29/22 7:02 PM, Mike Christie wrote: > > On 9/6/22 10:45 AM, Dmitry Bogdanov wrote: > >> Garantee uniquity of RTPI only for enabled target port groups. > >> Allow any RPTI for disabled tpg until it is enabled. > >> > >> Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> > >> --- > >> drivers/target/target_core_fabric_configfs.c | 29 +++++++++++++- > >> drivers/target/target_core_internal.h | 2 + > >> drivers/target/target_core_tpg.c | 40 +++++++++++++------- > >> 3 files changed, 56 insertions(+), 15 deletions(-) > >> > >> diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c > >> index a34b5db4eec5..fc1b8f54fb54 100644 > >> --- a/drivers/target/target_core_fabric_configfs.c > >> +++ b/drivers/target/target_core_fabric_configfs.c > >> @@ -857,6 +857,7 @@ static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item, > >> size_t count) > >> { > >> struct se_portal_group *se_tpg = to_tpg(item); > >> + struct se_portal_group *tpg; > >> int ret; > >> bool op; > >> > >> @@ -867,11 +868,37 @@ static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item, > >> if (se_tpg->enabled == op) > >> return count; > >> > >> + spin_lock(&g_tpg_lock); > >> + > >> + if (op) { > >> + tpg = core_get_tpg_by_rtpi(se_tpg->tpg_rtpi); > >> + if (tpg) { > >> + spin_unlock(&g_tpg_lock); > >> + > >> + pr_err("TARGET_CORE[%s]->TPG[%u] - RTPI %#x conflicts with TARGET_CORE[%s]->TPG[%u]\n", > >> + se_tpg->se_tpg_tfo->fabric_name, > >> + se_tpg->se_tpg_tfo->tpg_get_tag(tpg), > >> + se_tpg->tpg_rtpi, > >> + tpg->se_tpg_tfo->fabric_name, > >> + tpg->se_tpg_tfo->tpg_get_tag(tpg)); > >> + return -EINVAL; > >> + } > >> + } > >> + > >> + se_tpg->enabled |= 0x10; /* transient state */ > > > > Just use a mutex and hold it the entire time if > I was looking at the configfs code and am now not sure what the transient state > is for. It looks like when doing a read or write configfs holds the buffer->mutex, > so I don't think userspace would ever see the transient state. > > Can you just drop it? sysfs lock is per file, so 'enable' and 'rtpi' files can be written in parallel. But after rewriting of the patchset, this part will be dropped.
diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c index a34b5db4eec5..fc1b8f54fb54 100644 --- a/drivers/target/target_core_fabric_configfs.c +++ b/drivers/target/target_core_fabric_configfs.c @@ -857,6 +857,7 @@ static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item, size_t count) { struct se_portal_group *se_tpg = to_tpg(item); + struct se_portal_group *tpg; int ret; bool op; @@ -867,11 +868,37 @@ static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item, if (se_tpg->enabled == op) return count; + spin_lock(&g_tpg_lock); + + if (op) { + tpg = core_get_tpg_by_rtpi(se_tpg->tpg_rtpi); + if (tpg) { + spin_unlock(&g_tpg_lock); + + pr_err("TARGET_CORE[%s]->TPG[%u] - RTPI %#x conflicts with TARGET_CORE[%s]->TPG[%u]\n", + se_tpg->se_tpg_tfo->fabric_name, + se_tpg->se_tpg_tfo->tpg_get_tag(tpg), + se_tpg->tpg_rtpi, + tpg->se_tpg_tfo->fabric_name, + tpg->se_tpg_tfo->tpg_get_tag(tpg)); + return -EINVAL; + } + } + + se_tpg->enabled |= 0x10; /* transient state */ + spin_unlock(&g_tpg_lock); + ret = se_tpg->se_tpg_tfo->fabric_enable_tpg(se_tpg, op); - if (ret) + + spin_lock(&g_tpg_lock); + if (ret < 0) { + se_tpg->enabled &= ~0x10; /* clear transient state */ + spin_unlock(&g_tpg_lock); return ret; + } se_tpg->enabled = op; + spin_unlock(&g_tpg_lock); return count; } diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h index d662cdc9a04c..d4ace697edb0 100644 --- a/drivers/target/target_core_internal.h +++ b/drivers/target/target_core_internal.h @@ -116,6 +116,7 @@ int core_tmr_lun_reset(struct se_device *, struct se_tmr_req *, struct list_head *, struct se_cmd *); /* target_core_tpg.c */ +extern struct spinlock g_tpg_lock; extern struct se_device *g_lun0_dev; extern struct configfs_attribute *core_tpg_attrib_attrs[]; @@ -131,6 +132,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); +struct se_portal_group *core_get_tpg_by_rtpi(u16 rtpi); /* target_core_transport.c */ extern struct kmem_cache *se_tmr_req_cache; diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c index 85c9473a38fd..ef2adbe628e0 100644 --- a/drivers/target/target_core_tpg.c +++ b/drivers/target/target_core_tpg.c @@ -34,7 +34,7 @@ extern struct se_device *g_lun0_dev; static u16 g_tpg_count; static u16 g_tpg_rtpi_counter = 1; static LIST_HEAD(g_tpg_list); -static DEFINE_SPINLOCK(g_tpg_lock); +DEFINE_SPINLOCK(g_tpg_lock); /* __core_tpg_get_initiator_node_acl(): * @@ -476,7 +476,7 @@ static int core_tpg_register_rtpi(struct se_portal_group *se_tpg) * Make sure RELATIVE TARGET PORT IDENTIFIER is unique * for 16-bit wrap.. */ - if (se_tpg->tpg_rtpi == tpg->tpg_rtpi) + if (tpg->enabled && se_tpg->tpg_rtpi == tpg->tpg_rtpi) goto again; } list_add(&se_tpg->tpg_list, &g_tpg_list); @@ -738,12 +738,26 @@ static ssize_t core_tpg_rtpi_show(struct config_item *item, char *page) return sprintf(page, "%#x\n", se_tpg->tpg_rtpi); } +struct se_portal_group * +core_get_tpg_by_rtpi(u16 rtpi) +{ + struct se_portal_group *tpg; + + lockdep_assert_held(&g_tpg_lock); + + list_for_each_entry(tpg, &g_tpg_list, tpg_list) { + if (tpg->enabled && rtpi == tpg->tpg_rtpi) + return tpg; + } + + return NULL; +} + 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); struct se_portal_group *tpg; - bool rtpi_changed = false; u16 val; int ret; @@ -753,11 +767,14 @@ static ssize_t core_tpg_rtpi_store(struct config_item *item, if (val == 0) return -EINVAL; - /* RTPI shouldn't conflict with values of existing ports */ + if (se_tpg->tpg_rtpi == val) + return count; + spin_lock(&g_tpg_lock); - list_for_each_entry(tpg, &g_tpg_list, tpg_list) { - if (se_tpg != tpg && val == tpg->tpg_rtpi) { + if (se_tpg->enabled) { + tpg = core_get_tpg_by_rtpi(val); + if (tpg) { spin_unlock(&g_tpg_lock); pr_err("TARGET_CORE[%s]->TPG[%u] - RTPI %#x conflicts with TARGET_CORE[%s]->TPG[%u]\n", se_tpg->se_tpg_tfo->fabric_name, @@ -769,17 +786,12 @@ static ssize_t core_tpg_rtpi_store(struct config_item *item, } } - if (se_tpg->tpg_rtpi != val) { - se_tpg->tpg_rtpi = val; - rtpi_changed = true; - } + se_tpg->tpg_rtpi = val; spin_unlock(&g_tpg_lock); - if (rtpi_changed) - core_tpg_ua(se_tpg, 0x3f, ASCQ_3FH_INQUIRY_DATA_HAS_CHANGED); - ret = count; + core_tpg_ua(se_tpg, 0x3f, ASCQ_3FH_INQUIRY_DATA_HAS_CHANGED); - return ret; + return count; } CONFIGFS_ATTR(core_tpg_, rtpi);
Garantee uniquity of RTPI only for enabled target port groups. Allow any RPTI for disabled tpg until it is enabled. Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> --- drivers/target/target_core_fabric_configfs.c | 29 +++++++++++++- drivers/target/target_core_internal.h | 2 + drivers/target/target_core_tpg.c | 40 +++++++++++++------- 3 files changed, 56 insertions(+), 15 deletions(-)