mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention
@ 2026-09-11 12:58 Pavol Sakac
  2026-09-11 12:58 ` [RFC PATCH 1/3] iommu: split sysfs link publication out of iommu_group_alloc_device() Pavol Sakac
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:58 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon
  Cc: Robin Murphy, iommu, linux-kernel, Bjorn Helgaas, linux-pci,
	nh-open-source

iommu_probe_device_lock is a file-scope mutex held across
__iommu_probe_device(): every device's IOMMU probe serializes against
every other. With "PCI/IOV: Initialize virtual functions in
parallel" [1] fanning an SR-IOV enable across CPUs, it collects 94.7%
of all lock wait. Patch 1 gathers per-member sysfs publication into one
all-or-nothing helper (no functional change); patches 2-3 move that
publication and first-device default-domain setup into per-group
finalisation after the global unlock, under group->mutex, mirroring the
removal path and bus_iommu_probe().

Lock statistics and SR-IOV init time for 4x PF (NVMe, 255 VFs each), on
the reproducer from the parallel VF initialization cover letter [1]:

  lock_stat:
  Lock                     wait: Before     After   contentions: Before   After
  iommu_probe_device_lock      25507 ms  10471 ms                  1143     841
  &root->kernfs_rwsem            942 ms   1834 ms                 93208  116316
  &vfio.group_lock               425 ms   3614 ms                   497     736

  avg wait per acquisition  6.3 ms -> 2.6 ms (4080 acq., both arms)

  Stage                 SR-IOV init time:
  S0 (baseline)         3027 ms
  S1                     999 ms
  S2 (this series)       995 ms

Reproducer disclaimer:
I lean primarily on lock_stat numbers to defend the improvements. In
the reproducer, this lock's residual hold dominates the window and
masks the later series' wall-time gains, more in [1].

This is relief, not removal: the lock still has the highest wait
time in the profile after this series. Wait per acquisition drops
from 6.3 ms to 2.6 ms, which is what makes the smaller
serialization points behind it measurable for the later series.

RFC on the direction: The comment in __iommu_probe_device() expects
the lock to narrow to device_lock() once the ACPI/OF replay calls
are cleaned up. I could not make that work for the whole section:
group formation in ops->device_group() is a cross-device decision a
per-device lock cannot order. This series instead moves the work
that needs no global ordering out of the section. Is that an
acceptable step, or is there a scoping or removal plan this should
wait for?

A second question: I have measured where 90% of the residual hold goes:
get_pci_alias_group() walks every PCI device in the system to find
same-bus DMA aliases. It runs once per device probed, so once per VF,
under this lock, and the VFs keep growing the list it walks. A prototype
that skips the walk when no device has a dma_alias_mask and this device
has no pci_real_dma_dev() override cuts this lock's hold time by about
90%, for the same acquisitions and the same groups. I am not proposing
it here, as I do not have the time to get it right this cycle. How can
we optimize this preferably in O(1) time?

The lock_stat and timing figures come from the public reproducer. The
full series has also been tested on current datacenter server hardware
with thousands of VFs.

[1] https://lore.kernel.org/r/20260911-vfopt-s1-v1-0-693271dc0226@amazon.de

Pavol Sakac (3):
  iommu: split sysfs link publication out of iommu_group_alloc_device()
  iommu: create device sysfs links outside iommu_probe_device_lock
  iommu: set up the default domain outside iommu_probe_device_lock

 drivers/iommu/iommu.c | 197 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 160 insertions(+), 37 deletions(-)


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.47.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH 1/3] iommu: split sysfs link publication out of iommu_group_alloc_device()
  2026-09-11 12:58 [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention Pavol Sakac
@ 2026-09-11 12:58 ` Pavol Sakac
  2026-09-11 12:58 ` [RFC PATCH 2/3] iommu: create device sysfs links outside iommu_probe_device_lock Pavol Sakac
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:58 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon
  Cc: Robin Murphy, iommu, linux-kernel, Bjorn Helgaas, linux-pci,
	nh-open-source

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@amazon.de>
---
 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH 2/3] iommu: create device sysfs links outside iommu_probe_device_lock
  2026-09-11 12:58 [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention Pavol Sakac
  2026-09-11 12:58 ` [RFC PATCH 1/3] iommu: split sysfs link publication out of iommu_group_alloc_device() Pavol Sakac
@ 2026-09-11 12:58 ` Pavol Sakac
  2026-09-11 12:58 ` [RFC PATCH 3/3] iommu: set up the default domain " Pavol Sakac
  2026-09-11 17:45 ` [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention Robin Murphy
  3 siblings, 0 replies; 6+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:58 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon
  Cc: Robin Murphy, iommu, linux-kernel, Bjorn Helgaas, linux-pci,
	nh-open-source

Parallel device probes convoy on iommu_probe_device_lock, and parallel VF
initialisation shows it as a top contention source, with per-member sysfs
publication the dominant term under the hold. Publication needs no global
ordering: removal already does the reverse under group->mutex alone in
__iommu_group_free_device().

Pin the group under the global lock, drop it, and publish every member
still lacking links under group->mutex. A device joining a group that
already has a domain is published under the global lock instead, because
no later pass is guaranteed to visit it; members created by
probe_iommu_group() are published by bus_iommu_probe()'s existing drain.

The group reference is needed because not every caller pins the device:
iommu_add_device() on powerpc, the __init sweep in
probe_acpi_namespace_devices() and the of_dma_configure() replay run
neither inside device_add() nor under device_lock().

A sibling whose publication failed stays unpublished, retried by any later
finalisation or drain; in the worst case it waits until a new member joins,
the same terminal shape a failed bus_iommu_probe() drain leaves today. On
the hotplug path the links exist before device_add() emits KOBJ_ADD, so
udev cannot tell the difference.

A failed call also removes the probing device itself from the group:
iommu_probe_device() early-exits on membership, so a member left behind
would turn a retried probe into a no-op success while the group's members
remain unpublished.

Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@amazon.de>
---
 drivers/iommu/iommu.c | 70 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5f92981d9f34..a5e3327aeaca 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -663,11 +663,12 @@ 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->domain || group->default_domain) {
+		/* No later pass is guaranteed to publish this member. */
+		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) {
@@ -710,19 +711,68 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
 int iommu_probe_device(struct device *dev)
 {
 	const struct iommu_ops *ops;
+	struct group_device *gdev, *own;
+	struct iommu_group *group;
 	int ret;
 
 	mutex_lock(&iommu_probe_device_lock);
+	/* Already probed; publication belongs to the creating call. */
+	if (dev->iommu_group) {
+		mutex_unlock(&iommu_probe_device_lock);
+		goto probe_finalize;
+	}
 	ret = __iommu_probe_device(dev, NULL);
+	if (!ret) {
+		/*
+		 * Not every caller pins the device, so pin the group across
+		 * the unlock.
+		 */
+		group = dev->iommu_group;
+		iommu_group_ref_get(group);
+	}
 	mutex_unlock(&iommu_probe_device_lock);
 	if (ret)
 		return ret;
 
+	mutex_lock(&group->mutex);
+	/* iommu_group_link_device() is all-or-nothing. */
+	for_each_group_device(group, gdev) {
+		if (gdev->linked)
+			continue;
+		ret = iommu_group_link_device(group, gdev);
+		if (ret)
+			goto err_remove_device;
+	}
+	mutex_unlock(&group->mutex);
+	iommu_group_put(group);
+
+probe_finalize:
 	ops = dev_iommu_ops(dev);
 	if (ops->probe_finalize)
 		ops->probe_finalize(dev);
 
 	return 0;
+
+err_remove_device:
+	/* Retried probes early-exit on membership; failure must undo it. */
+	own = NULL;
+	for_each_group_device(group, gdev) {
+		if (gdev->dev == dev) {
+			own = gdev;
+			break;
+		}
+	}
+	if (own) {
+		list_del(&own->list);
+		__iommu_group_free_device(group, own);
+		iommu_deinit_device(dev);
+	}
+	mutex_unlock(&group->mutex);
+	if (own)
+		iommu_group_put(group);	/* iommu_init_device()'s reference */
+	iommu_group_put(group);		/* the reference taken above */
+
+	return ret;
 }
 
 static void __iommu_group_free_device(struct iommu_group *group,
@@ -2012,6 +2062,16 @@ static int bus_iommu_probe(const struct bus_type *bus)
 		/* Remove item from the list */
 		list_del_init(&group->entry);
 
+		for_each_group_device(group, gdev) {
+			if (gdev->linked)
+				continue;
+			ret = iommu_group_link_device(group, gdev);
+			if (ret) {
+				mutex_unlock(&group->mutex);
+				return ret;
+			}
+		}
+
 		/*
 		 * We go to the trouble of deferred default domain creation so
 		 * that the cross-group default domain type and the setup of the
-- 
2.47.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH 3/3] iommu: set up the default domain outside iommu_probe_device_lock
  2026-09-11 12:58 [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention Pavol Sakac
  2026-09-11 12:58 ` [RFC PATCH 1/3] iommu: split sysfs link publication out of iommu_group_alloc_device() Pavol Sakac
  2026-09-11 12:58 ` [RFC PATCH 2/3] iommu: create device sysfs links outside iommu_probe_device_lock Pavol Sakac
@ 2026-09-11 12:58 ` Pavol Sakac
  2026-09-11 17:45 ` [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention Robin Murphy
  3 siblings, 0 replies; 6+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:58 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon
  Cc: Robin Murphy, iommu, linux-kernel, Bjorn Helgaas, linux-pci,
	nh-open-source

Default domain allocation, attach and direct-mapping setup for a group's
first device still run inline under iommu_probe_device_lock, though they
need only group->mutex: iommu_group_store_type() does this at runtime
under group->mutex alone, and bus_iommu_probe() already defers it via the
group_list. With publication moved out it is the largest term left under
the hold, and parallel VF initialisation convoys behind it.

Use the group_list deferral for the singleton probe path too:
iommu_probe_device() passes a list and runs the setup after the global
unlock, under group->mutex. The global lock's original purpose - commit
01657bc14a39 ("iommu: Avoid races around device probe") - kept two devices
of one group from double-initialising it; that invariant moves to
group->mutex, where the setup is rechecked: of two racing siblings the
first to find the group unset sets it up for every member, and the rest
skip.

A failed setup unwinds only the calling device, like a failed publication;
siblings wait in -EPROBE_DEFER until a later probe finalises the group, or
until a new member joins if the failing call had claimed a bus-scanned
group's queued entry - the same terminal shape a failed bus_iommu_probe()
drain leaves today. A domain the failed setup published stays attached
(the first attach is IOMMU_SET_DOMAIN_MUST_SUCCEED), so members left
behind get the DMA ops it owed them.

bus_iommu_probe() rechecks group->default_domain, as a hotplug probe can
finalise a queued group before the drain takes the mutex.

Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@amazon.de>
---
 drivers/iommu/iommu.c | 48 ++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index a5e3327aeaca..48fe22bbd1bc 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -657,10 +657,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
 		goto err_put_group;
 	}
 
-	/*
-	 * The gdev must be in the list before calling
-	 * iommu_setup_default_domain()
-	 */
+	/* List the gdev first: the finaliser covers every listed member. */
 	list_add_tail(&gdev->list, &group->devices);
 	WARN_ON(group->default_domain && !group->domain);
 	if (group->domain || group->default_domain) {
@@ -676,15 +673,10 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
 						0);
 		if (ret)
 			goto err_remove_gdev;
-	} else if (!group->default_domain && !group_list) {
-		ret = iommu_setup_default_domain(group, 0);
-		if (ret)
-			goto err_remove_gdev;
 	} else if (!group->default_domain) {
 		/*
-		 * With a group_list argument we defer the default_domain setup
-		 * to the caller by providing a de-duplicated list of groups
-		 * that need further setup.
+		 * Defer setup to the caller draining group_list; both in-tree
+		 * callers pass one.
 		 */
 		if (list_empty(&group->entry))
 			list_add_tail(&group->entry, group_list);
@@ -713,6 +705,7 @@ int iommu_probe_device(struct device *dev)
 	const struct iommu_ops *ops;
 	struct group_device *gdev, *own;
 	struct iommu_group *group;
+	LIST_HEAD(group_list);
 	int ret;
 
 	mutex_lock(&iommu_probe_device_lock);
@@ -721,7 +714,7 @@ int iommu_probe_device(struct device *dev)
 		mutex_unlock(&iommu_probe_device_lock);
 		goto probe_finalize;
 	}
-	ret = __iommu_probe_device(dev, NULL);
+	ret = __iommu_probe_device(dev, &group_list);
 	if (!ret) {
 		/*
 		 * Not every caller pins the device, so pin the group across
@@ -735,6 +728,9 @@ int iommu_probe_device(struct device *dev)
 		return ret;
 
 	mutex_lock(&group->mutex);
+	/* Only the caller that queued the entry may unlink it. */
+	if (!list_empty(&group_list))
+		list_del_init(&group->entry);
 	/* iommu_group_link_device() is all-or-nothing. */
 	for_each_group_device(group, gdev) {
 		if (gdev->linked)
@@ -743,6 +739,20 @@ int iommu_probe_device(struct device *dev)
 		if (ret)
 			goto err_remove_device;
 	}
+	/*
+	 * First thread to find the group unset sets it up for every member;
+	 * binding needs a default domain, so no DMA races the window.
+	 */
+	if (!list_empty(&group->devices) &&
+	    !group->domain && !group->default_domain) {
+		ret = iommu_setup_default_domain(group, 0);
+		if (ret)
+			goto err_remove_device;
+		for_each_group_device(group, gdev)
+			if (dev_has_iommu(gdev->dev))
+				iommu_setup_dma_ops(gdev->dev,
+						    group->default_domain);
+	}
 	mutex_unlock(&group->mutex);
 	iommu_group_put(group);
 
@@ -765,8 +775,15 @@ int iommu_probe_device(struct device *dev)
 	if (own) {
 		list_del(&own->list);
 		__iommu_group_free_device(group, own);
-		iommu_deinit_device(dev);
 	}
+	/* Members left behind stay attached; give them the DMA ops owed. */
+	if (group->default_domain)
+		for_each_group_device(group, gdev)
+			if (dev_has_iommu(gdev->dev))
+				iommu_setup_dma_ops(gdev->dev,
+						    group->default_domain);
+	if (own)
+		iommu_deinit_device(dev);
 	mutex_unlock(&group->mutex);
 	if (own)
 		iommu_group_put(group);	/* iommu_init_device()'s reference */
@@ -2077,7 +2094,10 @@ static int bus_iommu_probe(const struct bus_type *bus)
 		 * that the cross-group default domain type and the setup of the
 		 * IOMMU_RESV_DIRECT will work correctly in non-hotpug scenarios.
 		 */
-		ret = iommu_setup_default_domain(group, 0);
+		ret = 0;
+		/* A hotplug probe may have finalised this group meanwhile. */
+		if (!group->default_domain)
+			ret = iommu_setup_default_domain(group, 0);
 		if (ret) {
 			mutex_unlock(&group->mutex);
 			return ret;
-- 
2.47.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention
  2026-09-11 12:58 [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention Pavol Sakac
                   ` (2 preceding siblings ...)
  2026-09-11 12:58 ` [RFC PATCH 3/3] iommu: set up the default domain " Pavol Sakac
@ 2026-09-11 17:45 ` Robin Murphy
  2026-09-17 20:21   ` Sakac, Pavol
  3 siblings, 1 reply; 6+ messages in thread
From: Robin Murphy @ 2026-09-11 17:45 UTC (permalink / raw)
  To: Pavol Sakac, Joerg Roedel, Will Deacon
  Cc: iommu, linux-kernel, Bjorn Helgaas, linux-pci, nh-open-source

On 11/09/2026 1:58 pm, Pavol Sakac wrote:
> iommu_probe_device_lock is a file-scope mutex held across
> __iommu_probe_device(): every device's IOMMU probe serializes against
> every other. With "PCI/IOV: Initialize virtual functions in
> parallel" [1] fanning an SR-IOV enable across CPUs, it collects 94.7%
> of all lock wait. Patch 1 gathers per-member sysfs publication into one
> all-or-nothing helper (no functional change); patches 2-3 move that
> publication and first-device default-domain setup into per-group
> finalisation after the global unlock, under group->mutex, mirroring the
> removal path and bus_iommu_probe().
> 
> Lock statistics and SR-IOV init time for 4x PF (NVMe, 255 VFs each), on
> the reproducer from the parallel VF initialization cover letter [1]:
> 
>    lock_stat:
>    Lock                     wait: Before     After   contentions: Before   After
>    iommu_probe_device_lock      25507 ms  10471 ms                  1143     841
>    &root->kernfs_rwsem            942 ms   1834 ms                 93208  116316
>    &vfio.group_lock               425 ms   3614 ms                   497     736
> 
>    avg wait per acquisition  6.3 ms -> 2.6 ms (4080 acq., both arms)
> 
>    Stage                 SR-IOV init time:
>    S0 (baseline)         3027 ms
>    S1                     999 ms
>    S2 (this series)       995 ms
> 
> Reproducer disclaimer:
> I lean primarily on lock_stat numbers to defend the improvements. In
> the reproducer, this lock's residual hold dominates the window and
> masks the later series' wall-time gains, more in [1].
> 
> This is relief, not removal: the lock still has the highest wait
> time in the profile after this series. Wait per acquisition drops
> from 6.3 ms to 2.6 ms, which is what makes the smaller
> serialization points behind it measurable for the later series.
> 
> RFC on the direction: The comment in __iommu_probe_device() expects
> the lock to narrow to device_lock() once the ACPI/OF replay calls
> are cleaned up. I could not make that work for the whole section:
> group formation in ops->device_group() is a cross-device decision a
> per-device lock cannot order. This series instead moves the work
> that needs no global ordering out of the section. Is that an
> acceptable step, or is there a scoping or removal plan this should
> wait for?

The point of probe_device_lock is to prevent multiple threads trying to
probe the *same* device concurrently; it protects the per-device state
of dev->iommu and dev->iommu_group until the latter is assigned or the
former is cleaned up (depending on how the probe goes). The replay calls
are mostly gone, but the main reason device_lock() still won't work is
that the same problem exists for driver-model-based IOMMU drivers
themselves, since we don't have a good way to avoid bus_iommu_probe()
deadlocking on IOMMU devices that are in the middle of registering
during their own driver bind (not least the caller itself).

> A second question: I have measured where 90% of the residual hold goes:
> get_pci_alias_group() walks every PCI device in the system to find
> same-bus DMA aliases. It runs once per device probed, so once per VF,
> under this lock, and the VFs keep growing the list it walks. A prototype
> that skips the walk when no device has a dma_alias_mask and this device
> has no pci_real_dma_dev() override cuts this lock's hold time by about
> 90%, for the same acquisitions and the same groups. I am not proposing
> it here, as I do not have the time to get it right this cycle. How can
> we optimize this preferably in O(1) time?

TBH that makes it sound like optimising pci_device_group() is the better
thing to do. We were never really meant to have a global lock here - it
was just an acceptable compromise for simplicity at the time - so I'm
still not keen on adding yet more complexity to the probe flow to work
around it as if global serialisation was necessary when it isn't.

Heck, even if you do just want a quick bodge to ease contention then I'd
still lean more towards something more self-contained like this
hometime-on-a-Friday fun I couldn't resist sketching out...

Thanks,
Robin.

----->8-----

From: Robin Murphy <robin.murphy@arm.com>
Subject: [PATCH] UNTESTED: iommu: Reduce iommu_probe_device_lock contention

The purpose of iommu_probe_device_lock was to prevent multiple threads
trying to probe the same device concurrently, it's only global for the
sake of simplicity, as there are still reasons why we can't use
device_lock(), and adding a whole other lock to struct device itself
just for this would be unreasonable.

However, we're now getting sufficiently large systems with enough
devices to start seeing significant contention on this lock, so let's
scale it to a lock table to reduce contention between unrelated devices.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
  drivers/acpi/scan.c      |  6 +++---
  drivers/iommu/iommu.c    | 37 +++++++++++++++++++++++++------------
  drivers/iommu/of_iommu.c |  6 +++---
  include/linux/iommu.h    |  2 +-
  4 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index f48715ed827c..33d6a758042a 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1620,10 +1620,10 @@ static int acpi_iommu_configure_id(struct device *dev, const u32 *id_in)
  	int err;
  
  	/* Serialise to make dev->iommu stable under our potential fwspec */
-	mutex_lock(&iommu_probe_device_lock);
+	mutex_lock(iommu_probe_device_lock(dev));
  	/* If we already translated the fwspec there is nothing left to do */
  	if (dev_iommu_fwspec_get(dev)) {
-		mutex_unlock(&iommu_probe_device_lock);
+		mutex_unlock(iommu_probe_device_lock(dev));
  		return 0;
  	}
  
@@ -1633,7 +1633,7 @@ static int acpi_iommu_configure_id(struct device *dev, const u32 *id_in)
  	if (err && err != -EPROBE_DEFER)
  		err = viot_iommu_configure(dev);
  
-	mutex_unlock(&iommu_probe_device_lock);
+	mutex_unlock(iommu_probe_device_lock(dev));
  
  	return err;
  }
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cd1bca7ede9a..11519e195aaf 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -381,9 +381,9 @@ int iommu_mock_device_add(struct device *dev, struct iommu_device *iommu)
  {
  	int rc;
  
-	mutex_lock(&iommu_probe_device_lock);
+	mutex_lock(iommu_probe_device_lock(dev));
  	rc = iommu_fwspec_init(dev, iommu->fwnode);
-	mutex_unlock(&iommu_probe_device_lock);
+	mutex_unlock(iommu_probe_device_lock(dev));
  
  	if (rc)
  		return rc;
@@ -400,7 +400,7 @@ static struct dev_iommu *dev_iommu_get(struct device *dev)
  {
  	struct dev_iommu *param = dev->iommu;
  
-	lockdep_assert_held(&iommu_probe_device_lock);
+	lockdep_assert_held(iommu_probe_device_lock(dev));
  
  	if (param)
  		return param;
@@ -457,7 +457,7 @@ void dev_iommu_priv_set(struct device *dev, void *priv)
  {
  	/* FSL_PAMU does something weird */
  	if (!IS_ENABLED(CONFIG_FSL_PAMU))
-		lockdep_assert_held(&iommu_probe_device_lock);
+		lockdep_assert_held(iommu_probe_device_lock(dev));
  	dev->iommu->priv = priv;
  }
  EXPORT_SYMBOL_GPL(dev_iommu_priv_set);
@@ -483,9 +483,9 @@ static int iommu_init_device(struct device *dev)
  	 * found no IOMMU to wait for, so there's no point calling it again.
  	 */
  	if (!dev->iommu->fwspec && !dev->driver && dev->bus->dma_configure) {
-		mutex_unlock(&iommu_probe_device_lock);
+		mutex_unlock(iommu_probe_device_lock(dev));
  		dev->bus->dma_configure(dev);
-		mutex_lock(&iommu_probe_device_lock);
+		mutex_lock(iommu_probe_device_lock(dev));
  		/* If another instance finished the job for us, skip it */
  		if (!dev->iommu || dev->iommu_group)
  			return -ENODEV;
@@ -622,7 +622,20 @@ static struct iommu_domain *pasid_array_entry_to_domain(void *entry)
  	return ((struct iommu_attach_handle *)xa_untag_pointer(entry))->domain;
  }
  
-DEFINE_MUTEX(iommu_probe_device_lock);
+static struct mutex __iommu_probe_device_lock[4] = {
+	__MUTEX_INITIALIZER(iommu_probe_device_lock),
+	__MUTEX_INITIALIZER(iommu_probe_device_lock),
+	__MUTEX_INITIALIZER(iommu_probe_device_lock),
+	__MUTEX_INITIALIZER(iommu_probe_device_lock),
+};
+
+struct mutex *iommu_probe_device_lock(const struct device *dev)
+{
+	int hash = ((uintptr_t)dev / roundup_pow_of_two(sizeof(*dev))) %
+		    ARRAY_SIZE(__iommu_probe_device_lock);
+
+	return __iommu_probe_device_lock + hash;
+}
  
  static int __iommu_probe_device(struct device *dev, struct list_head *group_list)
  {
@@ -637,7 +650,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
  	 * probably be able to use device_lock() here to minimise the scope,
  	 * but for now enforcing a simple global ordering is fine.
  	 */
-	lockdep_assert_held(&iommu_probe_device_lock);
+	lockdep_assert_held(iommu_probe_device_lock(dev));
  
  	/* Device is probed already if in a group */
  	if (dev->iommu_group)
@@ -711,9 +724,9 @@ int iommu_probe_device(struct device *dev)
  	const struct iommu_ops *ops;
  	int ret;
  
-	mutex_lock(&iommu_probe_device_lock);
+	mutex_lock(iommu_probe_device_lock(dev));
  	ret = __iommu_probe_device(dev, NULL);
-	mutex_unlock(&iommu_probe_device_lock);
+	mutex_unlock(iommu_probe_device_lock(dev));
  	if (ret)
  		return ret;
  
@@ -1803,9 +1816,9 @@ static int probe_iommu_group(struct device *dev, void *data)
  	struct list_head *group_list = data;
  	int ret;
  
-	mutex_lock(&iommu_probe_device_lock);
+	mutex_lock(iommu_probe_device_lock(dev));
  	ret = __iommu_probe_device(dev, group_list);
-	mutex_unlock(&iommu_probe_device_lock);
+	mutex_unlock(iommu_probe_device_lock(dev));
  	if (ret == -ENODEV)
  		ret = 0;
  
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index a18bb60f6f3d..b5e3a425ca2d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -121,9 +121,9 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np,
  		return -ENODEV;
  
  	/* Serialise to make dev->iommu stable under our potential fwspec */
-	mutex_lock(&iommu_probe_device_lock);
+	mutex_lock(iommu_probe_device_lock(dev));
  	if (dev_iommu_fwspec_get(dev)) {
-		mutex_unlock(&iommu_probe_device_lock);
+		mutex_unlock(iommu_probe_device_lock(dev));
  		return 0;
  	}
  	dev_iommu_present = dev->iommu;
@@ -151,7 +151,7 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np,
  		iommu_fwspec_free(dev);
  	else if (err && dev->iommu)
  		dev_iommu_free(dev);
-	mutex_unlock(&iommu_probe_device_lock);
+	mutex_unlock(iommu_probe_device_lock(dev));
  
  	/*
  	 * If we're not on the iommu_probe_device() path (as indicated by the
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index ac43b8b93f14..3b876cb285e0 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1201,7 +1201,7 @@ static inline void *dev_iommu_priv_get(struct device *dev)
  
  void dev_iommu_priv_set(struct device *dev, void *priv);
  
-extern struct mutex iommu_probe_device_lock;
+struct mutex *iommu_probe_device_lock(const struct device *dev);
  int iommu_probe_device(struct device *dev);
  
  int iommu_device_use_default_domain(struct device *dev);
-- 
2.54.0.dirty


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention
  2026-09-11 17:45 ` [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention Robin Murphy
@ 2026-09-17 20:21   ` Sakac, Pavol
  0 siblings, 0 replies; 6+ messages in thread
From: Sakac, Pavol @ 2026-09-17 20:21 UTC (permalink / raw)
  To: Robin Murphy, Joerg Roedel, Will Deacon
  Cc: iommu, linux-kernel, Bjorn Helgaas, linux-pci, nh-open-source

On 9/11/26 19:45, Robin Murphy wrote:
> On 11/09/2026 1:58 pm, Pavol Sakac wrote:
>> iommu_probe_device_lock is a file-scope mutex held across
>> __iommu_probe_device(): every device's IOMMU probe serializes against
>> every other. With "PCI/IOV: Initialize virtual functions in
>> parallel" [1] fanning an SR-IOV enable across CPUs, it collects 94.7%
>> of all lock wait. Patch 1 gathers per-member sysfs publication into one
>> all-or-nothing helper (no functional change); patches 2-3 move that
>> publication and first-device default-domain setup into per-group
>> finalisation after the global unlock, under group->mutex, mirroring the
>> removal path and bus_iommu_probe().
>>
>> Lock statistics and SR-IOV init time for 4x PF (NVMe, 255 VFs each), on
>> the reproducer from the parallel VF initialization cover letter [1]:
>>
>>    lock_stat:
>>    Lock                     wait: Before     After   contentions: Before   After
>>    iommu_probe_device_lock      25507 ms  10471 ms                  1143     841
>>    &root->kernfs_rwsem            942 ms   1834 ms                 93208  116316
>>    &vfio.group_lock               425 ms   3614 ms                   497     736
>>
>>    avg wait per acquisition  6.3 ms -> 2.6 ms (4080 acq., both arms)
>>
>>    Stage                 SR-IOV init time:
>>    S0 (baseline)         3027 ms
>>    S1                     999 ms
>>    S2 (this series)       995 ms
>>
>> Reproducer disclaimer:
>> I lean primarily on lock_stat numbers to defend the improvements. In
>> the reproducer, this lock's residual hold dominates the window and
>> masks the later series' wall-time gains, more in [1].
>>
>> This is relief, not removal: the lock still has the highest wait
>> time in the profile after this series. Wait per acquisition drops
>> from 6.3 ms to 2.6 ms, which is what makes the smaller
>> serialization points behind it measurable for the later series.
>>
>> RFC on the direction: The comment in __iommu_probe_device() expects
>> the lock to narrow to device_lock() once the ACPI/OF replay calls
>> are cleaned up. I could not make that work for the whole section:
>> group formation in ops->device_group() is a cross-device decision a
>> per-device lock cannot order. This series instead moves the work
>> that needs no global ordering out of the section. Is that an
>> acceptable step, or is there a scoping or removal plan this should
>> wait for?
> 
> The point of probe_device_lock is to prevent multiple threads trying to
> probe the *same* device concurrently; it protects the per-device state
> of dev->iommu and dev->iommu_group until the latter is assigned or the
> former is cleaned up (depending on how the probe goes). The replay calls
> are mostly gone, but the main reason device_lock() still won't work is
> that the same problem exists for driver-model-based IOMMU drivers
> themselves, since we don't have a good way to avoid bus_iommu_probe()
> deadlocking on IOMMU devices that are in the middle of registering
> during their own driver bind (not least the caller itself).
> 

Thanks, that makes sense.

>> A second question: I have measured where 90% of the residual hold goes:
>> get_pci_alias_group() walks every PCI device in the system to find
>> same-bus DMA aliases. It runs once per device probed, so once per VF,
>> under this lock, and the VFs keep growing the list it walks. A prototype
>> that skips the walk when no device has a dma_alias_mask and this device
>> has no pci_real_dma_dev() override cuts this lock's hold time by about
>> 90%, for the same acquisitions and the same groups. I am not proposing
>> it here, as I do not have the time to get it right this cycle. How can
>> we optimize this preferably in O(1) time?
> 
> TBH that makes it sound like optimising pci_device_group() is the better
> thing to do. We were never really meant to have a global lock here - it
> was just an acceptable compromise for simplicity at the time - so I'm
> still not keen on adding yet more complexity to the probe flow to work
> around it as if global serialisation was necessary when it isn't.
> 
> Heck, even if you do just want a quick bodge to ease contention then I'd
> still lean more towards something more self-contained like this
> hometime-on-a-Friday fun I couldn't resist sketching out...
> 
> Thanks,
> Robin.
> 

I appreciate the sketch :)

I've tested it with table size of 16, but it's not enough and needs also
get_pci_alias_group() walking scope reduction fix so it does not iterate
all devices but just those on the bus - as it itself claims to only need.
I left get_pci_function_alias_group() as is - VFs skip the walk there. 

When both are combined (in below table as arm C), they work the best and
finally put kernfs_rwsem to the top. Here are numbers from the reproducer
running with all of my patches from S1-S5 series:

Arm:
- A  the 3 iommu patches as posted
- B  purely your lock table with 16 entries replacing my 3 
- C  B + the alias-walk fix
- D  just alias-walk fix alone

                                         A         B         C         D
  iommu_probe_device_lock wait    10391 ms  19538 ms   1198 ms   8237 ms
  iommu_probe_device_lock hold     3338 ms  15074 ms   1002 ms    754 ms
  iommu_probe_device_lock cont         836       731       384      1134
  &root->kernfs_rwsem wait          660 ms    949 ms   2129 ms    835 ms
  gdp_mutex wait                    162 ms    189 ms   1139 ms    154 ms
  &k->k_lock acquisitions          1195808   1201016     86824     87242
  &k->k_lock contentions              2378    997420      4567      2261
  all classes, wait               11249 ms  26730 ms   4497 ms   9237 ms
  SR-IOV init time                 1152 ms   1600 ms    923 ms   1012 ms
  runs (min over)                        5         3         3         3

Also ran different table sizes sweep of arm C:

                                        4x       16x       64x
  iommu_probe_device_lock wait     4638 ms   1198 ms    447 ms
  iommu_probe_device_lock hold      854 ms   1002 ms   1423 ms
  iommu_probe_device_lock cont         957       384       121
  &root->kernfs_rwsem wait         1764 ms   2129 ms   3522 ms
  gdp_mutex wait                    665 ms   1139 ms   1713 ms
  &k->k_lock acquisitions            86975     86824     86876
  all classes, wait                7081 ms   4497 ms   5709 ms
  SR-IOV init time                 1000 ms    923 ms   1007 ms
  runs (min over)                        3         3         3

Would you like to take the first one? I can post v2 with just the alias
walk fix.

> ----->8-----
> 
> From: Robin Murphy <robin.murphy@arm.com>
> Subject: [PATCH] UNTESTED: iommu: Reduce iommu_probe_device_lock contention
> 
> The purpose of iommu_probe_device_lock was to prevent multiple threads
> trying to probe the same device concurrently, it's only global for the
> sake of simplicity, as there are still reasons why we can't use
> device_lock(), and adding a whole other lock to struct device itself
> just for this would be unreasonable.
> 
> However, we're now getting sufficiently large systems with enough
> devices to start seeing significant contention on this lock, so let's
> scale it to a lock table to reduce contention between unrelated devices.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  drivers/acpi/scan.c      |  6 +++---
>  drivers/iommu/iommu.c    | 37 +++++++++++++++++++++++++------------
>  drivers/iommu/of_iommu.c |  6 +++---
>  include/linux/iommu.h    |  2 +-
>  4 files changed, 32 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index f48715ed827c..33d6a758042a 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1620,10 +1620,10 @@ static int acpi_iommu_configure_id(struct device *dev, const u32 *id_in)
>        int err;
> 
>        /* Serialise to make dev->iommu stable under our potential fwspec */
> -       mutex_lock(&iommu_probe_device_lock);
> +       mutex_lock(iommu_probe_device_lock(dev));
>        /* If we already translated the fwspec there is nothing left to do */
>        if (dev_iommu_fwspec_get(dev)) {
> -               mutex_unlock(&iommu_probe_device_lock);
> +               mutex_unlock(iommu_probe_device_lock(dev));
>                return 0;
>        }
> 
> @@ -1633,7 +1633,7 @@ static int acpi_iommu_configure_id(struct device *dev, const u32 *id_in)
>        if (err && err != -EPROBE_DEFER)
>                err = viot_iommu_configure(dev);
> 
> -       mutex_unlock(&iommu_probe_device_lock);
> +       mutex_unlock(iommu_probe_device_lock(dev));
> 
>        return err;
>  }
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index cd1bca7ede9a..11519e195aaf 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -381,9 +381,9 @@ int iommu_mock_device_add(struct device *dev, struct iommu_device *iommu)
>  {
>        int rc;
> 
> -       mutex_lock(&iommu_probe_device_lock);
> +       mutex_lock(iommu_probe_device_lock(dev));
>        rc = iommu_fwspec_init(dev, iommu->fwnode);
> -       mutex_unlock(&iommu_probe_device_lock);
> +       mutex_unlock(iommu_probe_device_lock(dev));
> 
>        if (rc)
>                return rc;
> @@ -400,7 +400,7 @@ static struct dev_iommu *dev_iommu_get(struct device *dev)
>  {
>        struct dev_iommu *param = dev->iommu;
> 
> -       lockdep_assert_held(&iommu_probe_device_lock);
> +       lockdep_assert_held(iommu_probe_device_lock(dev));
> 
>        if (param)
>                return param;
> @@ -457,7 +457,7 @@ void dev_iommu_priv_set(struct device *dev, void *priv)
>  {
>        /* FSL_PAMU does something weird */
>        if (!IS_ENABLED(CONFIG_FSL_PAMU))
> -               lockdep_assert_held(&iommu_probe_device_lock);
> +               lockdep_assert_held(iommu_probe_device_lock(dev));
>        dev->iommu->priv = priv;
>  }
>  EXPORT_SYMBOL_GPL(dev_iommu_priv_set);
> @@ -483,9 +483,9 @@ static int iommu_init_device(struct device *dev)
>         * found no IOMMU to wait for, so there's no point calling it again.
>         */
>        if (!dev->iommu->fwspec && !dev->driver && dev->bus->dma_configure) {
> -               mutex_unlock(&iommu_probe_device_lock);
> +               mutex_unlock(iommu_probe_device_lock(dev));
>                dev->bus->dma_configure(dev);
> -               mutex_lock(&iommu_probe_device_lock);
> +               mutex_lock(iommu_probe_device_lock(dev));
>                /* If another instance finished the job for us, skip it */
>                if (!dev->iommu || dev->iommu_group)
>                        return -ENODEV;
> @@ -622,7 +622,20 @@ static struct iommu_domain *pasid_array_entry_to_domain(void *entry)
>        return ((struct iommu_attach_handle *)xa_untag_pointer(entry))->domain;
>  }
> 
> -DEFINE_MUTEX(iommu_probe_device_lock);
> +static struct mutex __iommu_probe_device_lock[4] = {
> +       __MUTEX_INITIALIZER(iommu_probe_device_lock),
> +       __MUTEX_INITIALIZER(iommu_probe_device_lock),
> +       __MUTEX_INITIALIZER(iommu_probe_device_lock),
> +       __MUTEX_INITIALIZER(iommu_probe_device_lock),
> +};
> +
> +struct mutex *iommu_probe_device_lock(const struct device *dev)
> +{
> +       int hash = ((uintptr_t)dev / roundup_pow_of_two(sizeof(*dev))) %
> +                   ARRAY_SIZE(__iommu_probe_device_lock);
> +
> +       return __iommu_probe_device_lock + hash;
> +}
> 
>  static int __iommu_probe_device(struct device *dev, struct list_head *group_list)
>  {
> @@ -637,7 +650,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
>         * probably be able to use device_lock() here to minimise the scope,
>         * but for now enforcing a simple global ordering is fine.
>         */
> -       lockdep_assert_held(&iommu_probe_device_lock);
> +       lockdep_assert_held(iommu_probe_device_lock(dev));
> 
>        /* Device is probed already if in a group */
>        if (dev->iommu_group)
> @@ -711,9 +724,9 @@ int iommu_probe_device(struct device *dev)
>        const struct iommu_ops *ops;
>        int ret;
> 
> -       mutex_lock(&iommu_probe_device_lock);
> +       mutex_lock(iommu_probe_device_lock(dev));
>        ret = __iommu_probe_device(dev, NULL);
> -       mutex_unlock(&iommu_probe_device_lock);
> +       mutex_unlock(iommu_probe_device_lock(dev));
>        if (ret)
>                return ret;
> 
> @@ -1803,9 +1816,9 @@ static int probe_iommu_group(struct device *dev, void *data)
>        struct list_head *group_list = data;
>        int ret;
> 
> -       mutex_lock(&iommu_probe_device_lock);
> +       mutex_lock(iommu_probe_device_lock(dev));
>        ret = __iommu_probe_device(dev, group_list);
> -       mutex_unlock(&iommu_probe_device_lock);
> +       mutex_unlock(iommu_probe_device_lock(dev));
>        if (ret == -ENODEV)
>                ret = 0;
> 
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index a18bb60f6f3d..b5e3a425ca2d 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -121,9 +121,9 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np,
>                return -ENODEV;
> 
>        /* Serialise to make dev->iommu stable under our potential fwspec */
> -       mutex_lock(&iommu_probe_device_lock);
> +       mutex_lock(iommu_probe_device_lock(dev));
>        if (dev_iommu_fwspec_get(dev)) {
> -               mutex_unlock(&iommu_probe_device_lock);
> +               mutex_unlock(iommu_probe_device_lock(dev));
>                return 0;
>        }
>        dev_iommu_present = dev->iommu;
> @@ -151,7 +151,7 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np,
>                iommu_fwspec_free(dev);
>        else if (err && dev->iommu)
>                dev_iommu_free(dev);
> -       mutex_unlock(&iommu_probe_device_lock);
> +       mutex_unlock(iommu_probe_device_lock(dev));
> 
>        /*
>         * If we're not on the iommu_probe_device() path (as indicated by the
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index ac43b8b93f14..3b876cb285e0 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -1201,7 +1201,7 @@ static inline void *dev_iommu_priv_get(struct device *dev)
> 
>  void dev_iommu_priv_set(struct device *dev, void *priv);
> 
> -extern struct mutex iommu_probe_device_lock;
> +struct mutex *iommu_probe_device_lock(const struct device *dev);
>  int iommu_probe_device(struct device *dev);
> 
>  int iommu_device_use_default_domain(struct device *dev);
> -- 
> 2.54.0.dirty
> 

Here's the alias walk fix prototype (only tested on VFs though):

-----

From: Pavol Sakac <sakacpav@amazon.de>
Date: Thu, 17 Sep 2026 13:00:00 +0200
Subject: [PATCH] iommu: Find PCI DMA aliases on the device's own bus

get_pci_alias_group() walks every PCI device in the system with
for_each_pci_dev() to find the aliases of one device. It runs once per
probed device under iommu_probe_device_lock and takes the PCI bus
klist lock twice per step.

A DMA alias is a devfn on the same bus, so walk only that bus, with
pci_walk_bus(). The recursion walks the bus itself and cannot nest
inside pci_walk_bus(), so fetch one alias per pass and let the visited
bit the recursion sets move the next pass on; a visited alias is no
longer re-entered only to return NULL.

No functional change intended.

Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@amazon.de>
---
 drivers/iommu/iommu.c | 55 ++++++++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 18 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cd1bca7..3bf1020 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1521,6 +1521,35 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev *pdev,
 	return NULL;
 }
 
+struct pci_alias_search {
+	struct pci_dev *pdev;
+	unsigned long *devfns;
+	struct pci_dev *alias;
+};
+
+static int find_pci_dma_alias(struct pci_dev *tmp, void *data)
+{
+	struct pci_alias_search *s = data;
+
+	/* pci_walk_bus() descends below the bus; aliases are same-bus only */
+	if (tmp->bus != s->pdev->bus ||
+	    test_bit(tmp->devfn & 0xff, s->devfns) ||
+	    !pci_devs_are_dma_aliases(s->pdev, tmp))
+		return 0;
+
+	s->alias = pci_dev_get(tmp);
+	return 1;
+}
+
+static struct pci_dev *get_pci_dma_alias(struct pci_dev *pdev,
+					 unsigned long *devfns)
+{
+	struct pci_alias_search s = { .pdev = pdev, .devfns = devfns };
+
+	pci_walk_bus(pdev->bus, find_pci_dma_alias, &s);
+	return s.alias;
+}
+
 /*
  * Look for aliases to or from the given device for existing groups. DMA
  * aliases are only supported on the same bus, therefore the search
@@ -1533,7 +1562,7 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev *pdev,
 static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
 					       unsigned long *devfns)
 {
-	struct pci_dev *tmp = NULL;
+	struct pci_dev *tmp;
 	struct iommu_group *group;
 
 	if (test_and_set_bit(pdev->devfn & 0xff, devfns))
@@ -1543,24 +1572,14 @@ static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
 	if (group)
 		return group;
 
-	for_each_pci_dev(tmp) {
-		if (tmp == pdev || tmp->bus != pdev->bus)
-			continue;
-
-		/* We alias them or they alias us */
-		if (pci_devs_are_dma_aliases(pdev, tmp)) {
-			group = get_pci_alias_group(tmp, devfns);
-			if (group) {
-				pci_dev_put(tmp);
-				return group;
-			}
-
+	/* One alias per walk: the recursion cannot nest in pci_walk_bus() */
+	while ((tmp = get_pci_dma_alias(pdev, devfns))) {
+		group = get_pci_alias_group(tmp, devfns);
+		if (!group)
 			group = get_pci_function_alias_group(tmp, devfns);
-			if (group) {
-				pci_dev_put(tmp);
-				return group;
-			}
-		}
+		pci_dev_put(tmp);
+		if (group)
+			return group;
 	}
 
 	return NULL;
-- 
2.47.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-17 20:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-11 12:58 [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention Pavol Sakac
2026-09-11 12:58 ` [RFC PATCH 1/3] iommu: split sysfs link publication out of iommu_group_alloc_device() Pavol Sakac
2026-09-11 12:58 ` [RFC PATCH 2/3] iommu: create device sysfs links outside iommu_probe_device_lock Pavol Sakac
2026-09-11 12:58 ` [RFC PATCH 3/3] iommu: set up the default domain " Pavol Sakac
2026-09-11 17:45 ` [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention Robin Murphy
2026-09-17 20:21   ` Sakac, Pavol

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®