[PATCH] scsi: target: alua: Free target port groups with kfree_rcu()

[PATCH] scsi: target: alua: Free target port groups with kfree_rcu()

[Qiliang Yuan]

core_alua_free_tg_pt_gp() frees a target port group when its configfs
group goes away, including the default group of every device when its
backstore is deleted. Readers of lun->lun_tg_pt_gp may still hold the
group, so it waits for an RCU grace period before freeing it.

The wait is synchronous, so deleting a backstore spends most of its time
in synchronize_rcu(), which adds up when tearing down many of them.

Free the group with kfree_rcu() instead. Nothing else in the function
needs the grace period to have elapsed, and kmem_cache_destroy() waits
for pending kfree_rcu() calls on module unload.

Deleting 256 rd_mcp backstores through configfs, five rounds
alternating with and without this patch, takes (median, min-max in
parentheses):

  backstores  before (ms)       after (ms)     change
         256  2108 (2105-2127)  692 (675-700)    -67%

Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
---
 drivers/target/target_core_alua.c | 3 +--
 include/target/target_core_base.h | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index 140154d93c430..4ef243ae843bb 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -1771,8 +1771,7 @@ void core_alua_free_tg_pt_gp(
 	}
 	spin_unlock(&tg_pt_gp->tg_pt_gp_lock);
 
-	synchronize_rcu();
-	kmem_cache_free(t10_alua_tg_pt_gp_cache, tg_pt_gp);
+	kfree_rcu(tg_pt_gp, rcu_head);
 }
 
 static struct t10_alua_tg_pt_gp *core_alua_get_tg_pt_gp_by_name(
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 002b0fc575876..cec29cbec1643 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -331,6 +331,7 @@ struct t10_alua_tg_pt_gp {
 	struct list_head tg_pt_gp_lun_list;
 	struct se_lun *tg_pt_gp_alua_lun;
 	struct se_node_acl *tg_pt_gp_alua_nacl;
+	struct rcu_head rcu_head;
 };
 
 struct t10_vpd {

---
base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9
change-id: 20261110-bug-target-alua-free-gp-1465539a269a

Best regards,
-- 
Qiliang Yuan <qiliang.yuan@linux.dev>

Re: [PATCH] scsi: target: alua: Free target port groups with kfree_rcu()

[Qiliang Yuan]

+Cc Martin, Mike and the patch's other recipients, and Yu Zhang, whose
patch fixes this.

On Wed, Nov 11, 2026 at 01:58:54PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found no new issues
> in this patch, but noticed 1 related pre-existing issue it does not
> introduce:
> - [Medium] scsi: target: race condition in core_alua_free_tg_pt_gp() [1]
> --
>
> commit cbab3de9aa10ecf910c633ef186a6a1775418e09
> Author: Qiliang Yuan <qiliang.yuan@linux.dev>
>
> scsi: target: alua: Free target port groups with kfree_rcu()
>
> This commit replaces synchronize_rcu() and kmem_cache_free() in
> core_alua_free_tg_pt_gp() with kfree_rcu() to avoid synchronous RCU waits
> when deleting target port groups and backstores.
>
> > diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
> > index 140154d93c430..4ef243ae843bb 100644
> > --- a/drivers/target/target_core_alua.c
> > +++ b/drivers/target/target_core_alua.c
> > @@ -1771,8 +1771,7 @@ void core_alua_free_tg_pt_gp(
> >  	}
> >  	spin_unlock(&tg_pt_gp->tg_pt_gp_lock);
> >  
> > -	synchronize_rcu();
> > -	kmem_cache_free(t10_alua_tg_pt_gp_cache, tg_pt_gp);
> > +	kfree_rcu(tg_pt_gp, rcu_head);
> >  }
>
> [Severity: Medium]
> This problem wasn't introduced by this patch, but can the LUN detachment
> loop right above this race with target_detach_tg_pt_gp() [1] and
> leave a freed struct se_lun on default_tg_pt_gp->tg_pt_gp_lun_list?
>
> drivers/target/target_core_alua.c:core_alua_free_tg_pt_gp() {
>     ...
> 	spin_lock(&tg_pt_gp->tg_pt_gp_lock);
> 	list_for_each_entry_safe(lun, next,
> 			&tg_pt_gp->tg_pt_gp_lun_list, lun_tg_pt_gp_link) {
> 		list_del_init(&lun->lun_tg_pt_gp_link);
> 		tg_pt_gp->tg_pt_gp_members--;
>
> 		spin_unlock(&tg_pt_gp->tg_pt_gp_lock);
> 		...
> 		spin_lock(&lun->lun_tg_pt_gp_lock);
> 		if (tg_pt_gp != dev->t10_alua.default_tg_pt_gp) {
> 			__target_attach_tg_pt_gp(lun,
> 					dev->t10_alua.default_tg_pt_gp);
> 		} else
> 			rcu_assign_pointer(lun->lun_tg_pt_gp, NULL);
> 		spin_unlock(&lun->lun_tg_pt_gp_lock);
>
> 		spin_lock(&tg_pt_gp->tg_pt_gp_lock);
> 	}
> 	spin_unlock(&tg_pt_gp->tg_pt_gp_lock);
> }
>
> When a non-default target port group is removed through configfs while a
> fabric LUN in that group is being unlinked, core_alua_free_tg_pt_gp()
> drops tg_pt_gp->tg_pt_gp_lock before taking lun->lun_tg_pt_gp_lock while
> lun->lun_tg_pt_gp still points to tg_pt_gp:
>
> CPU0                                   CPU1
> core_alua_free_tg_pt_gp(tg_pt_gp)      target_fabric_port_unlink()
>   spin_lock(&tg_pt_gp->tg_pt_gp_lock)    core_tpg_remove_lun(lun)
>   list_del_init(&lun->lun_tg_pt_gp_link)   target_detach_tg_pt_gp(lun)
>   tg_pt_gp->tg_pt_gp_members--
>   spin_unlock(&tg_pt_gp->tg_pt_gp_lock)
>                                              spin_lock(&lun->lun_tg_pt_gp_lock)
>                                              __target_detach_tg_pt_gp(...)
>                                              rcu_assign_pointer(
>                                                lun->lun_tg_pt_gp, NULL)
>                                              spin_unlock(&lun->lun_tg_pt_gp_lock)
>   spin_lock(&lun->lun_tg_pt_gp_lock)
>   __target_attach_tg_pt_gp(lun,
>     default_tg_pt_gp)
>   spin_unlock(&lun->lun_tg_pt_gp_lock)
>                                        target_fabric_port_release()
>                                          ...
>                                            kfree(lun)
>                                        target_emulate_report_target_port_groups()
>                                          /* walks default_tg_pt_gp list */
>                                          lun->...  <-- use-after-free
>
> Because core_alua_free_tg_pt_gp() does not recheck lun->lun_tg_pt_gp after
> acquiring lun->lun_tg_pt_gp_lock, it re-attaches the detaching LUN to
> default_tg_pt_gp after target_detach_tg_pt_gp() has already cleared it
> (and decremented tg_pt_gp->tg_pt_gp_members a second time).  When port
> teardown subsequently frees the LUN, a dangling entry remains on
> default_tg_pt_gp->tg_pt_gp_lun_list.
>
> [1] https://sashiko.dev/#/bug/linux-8e4ef2cb-206e-4a76-8c9e-458e67913f8b

The race is real, and it is already being fixed. Yu's patch [2] checks
lun->lun_tg_pt_gp again under lun_tg_pt_gp_lock before reattaching the
LUN, which covers target_detach_tg_pt_gp() as well as the concurrent
group change it was written for. This patch doesn't touch that loop.

[2] https://lore.kernel.org/r/20261019033532.73575-1-yuz08559@gmail.com

Thanks,
Qiliang