Re: [PATCH v3 1/5] iommu: Lock group->mutex in iommu_deferred_attach

Nicolin Chen
2025-08-15T19:26:39Z

On Fri, Aug 15, 2025 at 08:24:57AM +0000, Tian, Kevin wrote:
> > From: Nicolin Chen <nicolinc@nvidia.com>
> > Sent: Tuesday, August 12, 2025 6:59 AM
> >
> > The iommu_deferred_attach() is a runtime asynchronous function called by
> > iommu-dma function, which could race against other attach functions if it
> > accesses something in the dev->iommu_group.
>
> Is there a real racing scenario being observed or more theoretical?
>
> If the former may need a Fix tag.

Theoretical. I will highlight that in next version.

> > int iommu_deferred_attach(struct device *dev, struct iommu_domain
> > *domain)
> > {
> > - if (dev->iommu && dev->iommu->attach_deferred)
> > - return __iommu_attach_device(domain, dev);
> > + /*
> > + * This is called on the dma mapping fast path so avoid locking. This is
> > + * racy, but we have an expectation that the driver will setup its
> > DMAs
> > + * inside probe while being single threaded to avoid racing.
> > + */
> > + if (!dev->iommu || !dev->iommu->attach_deferred)
> > + return 0;
>
> Is there any way to detect a driver breaking the expectation?

Hmm, I am not sure.. that would sound tricky if we really want to
have a 2nd flag to protect this one..

And this patch doesn't change the race situation, if it does ever
exist..

If we really want to play safe, moving the flag under the lock as
Baolu's suggestion is probably the answer?

Thanks
Nicolin