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

[Nicolin Chen <nicolinc@nvidia.com>]

On Mon, Aug 18, 2025 at 03:09:20PM -0300, Jason Gunthorpe wrote:
> 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.

Yes. I will make the commit message clear that it isn't about a
bug fix, but to align with the locking policy at the attach_dev
callback and (more importantly) to fence gdev for the new flag.

Thanks
Nicolin