On Tue, Mar 09, 2021 at 09:52:30PM -0800, Dan Williams wrote: > On Tue, Mar 9, 2021 at 1:39 PM Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > > > > This is the standard kernel pattern, the ops associated with a struct get > > the struct pointer in for typesafety. The expected design is to use > > container_of to cleanly go from the subsystem level type to the driver > > level type without having any type erasure in a void *. > > This patch alone is worth the price of admission. Right, this is how I got here as I was going to do the mdev stuff and we end up doing transformations like this: -static long mtty_ioctl(struct mdev_device *mdev, unsigned int cmd, +static long mtty_ioctl(struct vfio_device *vdev, unsigned int cmd, unsigned long arg) And if the 'struct vfio_device *vdev' was left a 'void *' then the compiler doesn't get to help any more :( > Seems like it would be worth adding > to_vfio_{pci,platform,fsl_mc}_device() helpers in this patch as well. I have mixed feelings on these one-liners. If people feel they are worthwhile I'll add them > I've sometimes added runtime type safety to to_* helpers for early > warning of mistakes that happen when refactoring... > > static inline struct vfio_pci_device * > to_vfio_pci_device(struct vfio_device *core_dev) > { > if (dev_WARN_ONCE(core_dev->dev, core_dev->ops != &vfio_pci_ops, > "not a vfio_pci_device!\n")) > return NULL; > return container_of(core_vdev, struct vfio_pci_device, vdev); In this case I don't think we need to worry as everything is tidy in a single module such that the compilation units can't see other container_of options anyhow and vfio core isn't going to accidently call an ops with the wrong type. Jason