Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Pavol Sakac <sakacpav@amazon.de>
To: Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>, <iommu@lists.linux.dev>,
	<linux-kernel@vger.kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>, <linux-pci@vger.kernel.org>,
	<nh-open-source@amazon.com>
Subject: [RFC PATCH 2/3] iommu: create device sysfs links outside iommu_probe_device_lock
Date: Fri, 11 Sep 2026 14:58:33 +0200	[thread overview]
Message-ID: <20260911125907.67105-2-sakacpav@amazon.de> (raw)
In-Reply-To: <20260911-vfopt-s2-v1-0-fff3db7e01c2@amazon.de>

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


  parent reply	other threads:[~2026-09-11 12:59 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 13:14   ` sashiko-bot
2026-09-11 12:58 ` Pavol Sakac [this message]
2026-09-11 13:13   ` [RFC PATCH 2/3] iommu: create device sysfs links outside iommu_probe_device_lock sashiko-bot
2026-09-11 12:58 ` [RFC PATCH 3/3] iommu: set up the default domain " Pavol Sakac
2026-09-11 13:15   ` sashiko-bot
2026-09-11 17:45 ` [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention Robin Murphy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260911125907.67105-2-sakacpav@amazon.de \
    --to=sakacpav@amazon.de \
    --cc=bhelgaas@google.com \
    --cc=iommu@lists.linux.dev \
    --cc=joro@8bytes.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=nh-open-source@amazon.com \
    --cc=robin.murphy@arm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox