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@xxxxxxxxxx> > > > > > 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
