[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()

[sashiko-bot]

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-bug-target-alua-free-gp-v1-1-d1e692ae8694@linux.dev?part=1