[RFC PATCH 1/3] iommu: split sysfs link publication out of iommu_group_alloc_device()
From: Pavol Sakac
Date: Fri Sep 11 2026 - 09:13:16 EST
iommu_group_alloc_device() both allocates a group_device and publishes
it in sysfs (the "iommu_group" link and the group "devices/" member
link with its .%d rename loop), while the iommu instance links are
published separately in iommu_init_device() and removed in
iommu_deinit_device().
Gather all per-member publication into one helper,
iommu_group_link_device(), run under group->mutex, and record success
in a new group_device::linked flag. Teardown moves next to the member
link removal in __iommu_group_free_device(), which consults the flag so
an unpublished member skips sysfs.
No functional change intended; this gives publication a single seam so
a later commit can move it out of iommu_probe_device_lock.
Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@xxxxxxxxx>
---
drivers/iommu/iommu.c | 91 +++++++++++++++++++++++++++++++------------
1 file changed, 67 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cd1bca7ede9a..5f92981d9f34 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -77,6 +77,8 @@ struct group_device {
struct list_head list;
struct device *dev;
char *name;
+ /* Membership published in sysfs; set under group->mutex */
+ bool linked;
/*
* Device is blocked for a pending recovery while its group->domain is
* retained. This can happen when:
@@ -166,6 +168,8 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
const char *buf, size_t count);
static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
struct device *dev);
+static int iommu_group_link_device(struct iommu_group *group,
+ struct group_device *device);
static void __iommu_group_free_device(struct iommu_group *group,
struct group_device *grp_dev);
static void iommu_domain_init(struct iommu_domain *domain, unsigned int type,
@@ -515,16 +519,12 @@ static int iommu_init_device(struct device *dev)
}
dev->iommu->iommu_dev = iommu_dev;
- ret = iommu_device_link(iommu_dev, dev);
- if (ret)
- goto err_release;
-
group = ops->device_group(dev);
if (WARN_ON_ONCE(group == NULL))
group = ERR_PTR(-EINVAL);
if (IS_ERR(group)) {
ret = PTR_ERR(group);
- goto err_unlink;
+ goto err_release;
}
dev->iommu_group = group;
@@ -533,8 +533,6 @@ static int iommu_init_device(struct device *dev)
dev->iommu->attach_deferred = ops->is_attach_deferred(dev);
return 0;
-err_unlink:
- iommu_device_unlink(iommu_dev, dev);
err_release:
if (ops->release_device)
ops->release_device(dev);
@@ -553,8 +551,6 @@ static void iommu_deinit_device(struct device *dev)
lockdep_assert_held(&group->mutex);
- iommu_device_unlink(dev->iommu->iommu_dev, dev);
-
/*
* release_device() must stop using any attached domain on the device.
* If there are still other devices in the group, they are not affected
@@ -667,6 +663,11 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
*/
list_add_tail(&gdev->list, &group->devices);
WARN_ON(group->default_domain && !group->domain);
+
+ ret = iommu_group_link_device(group, gdev);
+ if (ret)
+ goto err_remove_gdev;
+
if (group->default_domain)
iommu_create_device_direct_mappings(group->default_domain, dev);
if (group->domain) {
@@ -729,10 +730,14 @@ static void __iommu_group_free_device(struct iommu_group *group,
{
struct device *dev = grp_dev->dev;
- sysfs_remove_link(group->devices_kobj, grp_dev->name);
- sysfs_remove_link(&dev->kobj, "iommu_group");
+ if (grp_dev->linked) {
+ if (dev_has_iommu(dev))
+ iommu_device_unlink(dev->iommu->iommu_dev, dev);
+ sysfs_remove_link(group->devices_kobj, grp_dev->name);
+ sysfs_remove_link(&dev->kobj, "iommu_group");
- trace_remove_device_from_group(group->id, dev);
+ trace_remove_device_from_group(group->id, dev);
+ }
/*
* If the group has become empty then ownership must have been
@@ -1266,7 +1271,6 @@ static int iommu_create_device_direct_mappings(struct iommu_domain *domain,
static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
struct device *dev)
{
- int ret, i = 0;
struct group_device *device;
device = kzalloc_obj(*device);
@@ -1275,11 +1279,40 @@ static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
device->dev = dev;
+ device->name = kasprintf(GFP_KERNEL, "%s", kobject_name(&dev->kobj));
+ if (!device->name) {
+ kfree(device);
+ dev_err(dev, "Failed to add to iommu group %d: %d\n",
+ group->id, -ENOMEM);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ return device;
+}
+
+/*
+ * Publish a member's sysfs links under group->mutex. All-or-nothing:
+ * on failure nothing is left behind and gdev->linked stays false, so
+ * __iommu_group_free_device() removes only what was published.
+ */
+static int iommu_group_link_device(struct iommu_group *group,
+ struct group_device *device)
+{
+ struct device *dev = device->dev;
+ int ret, i = 0;
+
+ lockdep_assert_held(&group->mutex);
+
+ if (dev_has_iommu(dev)) {
+ ret = iommu_device_link(dev->iommu->iommu_dev, dev);
+ if (ret)
+ goto err_out;
+ }
+
ret = sysfs_create_link(&dev->kobj, &group->kobj, "iommu_group");
if (ret)
- goto err_free_device;
+ goto err_unlink_iommu;
- device->name = kasprintf(GFP_KERNEL, "%s", kobject_name(&dev->kobj));
rename:
if (!device->name) {
ret = -ENOMEM;
@@ -1299,23 +1332,25 @@ static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
kobject_name(&dev->kobj), i++);
goto rename;
}
- goto err_free_name;
+ goto err_remove_link;
}
+ device->linked = true;
+
trace_add_device_to_group(group->id, dev);
dev_info(dev, "Adding to iommu group %d\n", group->id);
- return device;
+ return 0;
-err_free_name:
- kfree(device->name);
err_remove_link:
sysfs_remove_link(&dev->kobj, "iommu_group");
-err_free_device:
- kfree(device);
+err_unlink_iommu:
+ if (dev_has_iommu(dev))
+ iommu_device_unlink(dev->iommu->iommu_dev, dev);
+err_out:
dev_err(dev, "Failed to add to iommu group %d: %d\n", group->id, ret);
- return ERR_PTR(ret);
+ return ret;
}
/**
@@ -1329,15 +1364,23 @@ static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
int iommu_group_add_device(struct iommu_group *group, struct device *dev)
{
struct group_device *gdev;
+ int ret;
gdev = iommu_group_alloc_device(group, dev);
if (IS_ERR(gdev))
return PTR_ERR(gdev);
+ mutex_lock(&group->mutex);
+ ret = iommu_group_link_device(group, gdev);
+ if (ret) {
+ mutex_unlock(&group->mutex);
+ kfree(gdev->name);
+ kfree(gdev);
+ return ret;
+ }
+
iommu_group_ref_get(group);
dev->iommu_group = group;
-
- mutex_lock(&group->mutex);
list_add_tail(&gdev->list, &group->devices);
mutex_unlock(&group->mutex);
return 0;
--
2.47.3