On Wed, Nov 29, 2023 at 05:58:08PM +0000, Robin Murphy wrote: > On 29/11/2023 12:48 am, Jason Gunthorpe wrote: > > The iommu_device_lock protects the iommu_device_list which is only read by > > iommu_ops_from_fwnode(). > > > > This is now always called under the iommu_probe_device_lock, so we don't > > need to double lock the linked list. Use the iommu_probe_device_lock on > > the write side too. > > Please no, iommu_probe_device_lock() is a hack and we need to remove the > *reason* it exists at all. Yes, I agree that goal is good However, it is doing a lot of things, removing it is not so easy. One thing it is quietly doing is keeping the ops and iommu_device pointers alive during the entire probe process against(deeply broken, but whatever) concurrent iommu driver removal. It is also protecting access to dev->iommu_group during the group formation process. So, it is a little more complex. My specific interest was to make it not a spinlock. > And IMO just because iommu_present() is > deprecated doesn't justify making it look utterly nonsensical - in no way > does that have any relationship with probe_device, much less need to > serialise against it! The naming is poor now, I agree, but it is not nonsensical since it still holds the correct lock for the data it is accessing. Thanks, Jason
