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@xxxxxxxxx> >> --- >> 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?