[PATCH RFC v2 1/4] iommu: Lock group->mutex in iommu_deferred_attach

Nicolin Chen posted 4 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH RFC v2 1/4] iommu: Lock group->mutex in iommu_deferred_attach
Posted by Nicolin Chen 3 months, 1 week ago
The iommu_deferred_attach() is a runtime asynchronous function called by
iommu-dma function, which will race against other attach functions if it
accesses something in the dev->iommu_group.

Grab the lock to protect it like others who call __iommu_attach_device()
as it will need to access dev->iommu_group.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/iommu.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index a4b606c591da..08ff7efa8925 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2151,10 +2151,14 @@ EXPORT_SYMBOL_GPL(iommu_attach_device);
 
 int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain)
 {
-	if (dev->iommu && dev->iommu->attach_deferred)
-		return __iommu_attach_device(domain, dev);
+	struct iommu_group *group = dev->iommu_group;
+	int ret = 0;
 
-	return 0;
+	mutex_lock(&group->mutex);
+	if (dev->iommu && dev->iommu->attach_deferred)
+		ret = __iommu_attach_device(domain, dev);
+	mutex_unlock(&group->mutex);
+	return ret;
 }
 
 void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
-- 
2.43.0
Re: [PATCH RFC v2 1/4] iommu: Lock group->mutex in iommu_deferred_attach
Posted by Jason Gunthorpe 3 months ago
On Sat, Jun 28, 2025 at 12:42:39AM -0700, Nicolin Chen wrote:
> The iommu_deferred_attach() is a runtime asynchronous function called by
> iommu-dma function, which will race against other attach functions if it
> accesses something in the dev->iommu_group.
> 
> Grab the lock to protect it like others who call __iommu_attach_device()
> as it will need to access dev->iommu_group.
> 
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>  drivers/iommu/iommu.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

I vaugely recall seeing something like this before.

IIRC it can't actually race but there is no harm in taking the lock so
lockdep works reliably. It isn't fast path.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason