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

Jason Gunthorpe
2025-08-18T18:09:25Z

On Mon, Aug 18, 2025 at 10:45:07AM -0700, Nicolin Chen wrote:
> On Mon, Aug 18, 2025 at 11:17:51AM -0300, Jason Gunthorpe wrote:
> > 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?
> >
> > I think the commit message should explain the actual reason this is
> > being done, which AFAICT because the new lockdeps added in following
> > patches will fail on this path otherwise.
>
> Hmm, I can mention that. But I think that's just a part of the
> reason. It still doesn't seem correct to invoke an attach_dev
> function without the lock since iommu_group_mutex_assert() may
> be used in the path?

Last time this was brought up there was a bit of an argument that it
couldn't happen in parallel with anything anyhow so it doesn't
technically need locking. But I think we should not make such
arguments and be strict about our locking. It is too hard to
understand the system correctness otherwise.

Jason