On Fri, Jul 23, 2021 at 02:29:03PM +0200, Christoph Hellwig wrote: > On Fri, Jul 23, 2021 at 09:22:27AM -0300, Jason Gunthorpe wrote: > > > But do we even need the else part? Assingning &mc_dev->dev is > > > equivalent to the default per-device set anyway, isn't it? > > > > Not quite, the default is this: > > > > if (!device->dev_set) > > vfio_assign_device_set(device, device); > > > > Where 'device' is the vfio_device itself, the above is connecting to > > the struct fsl_mc_device. > > Isn't there a 1:1 relation? Yes, but one is a struct device * and the other is a struct vfio_device *. They don't have the same pointer value. The above default code has the goal of creating a singleton dev_set because no other driver can reasonably obtain the 'struct vfio_device *'. FSL is using a 'struct device *' as the key and the interesting case is when two drivers are loaded such that: mc_dev->dev.parent == &mc_dev->dev Then they will have the same dev_set and the locking works out. The vfio_device * can never be == &mc_dev->dev because it is a different allocation. Jason