On Tue, Mar 09, 2021 at 09:52:30PM -0800, Dan Williams wrote:

On Tue, Mar 9, 2021 at 1:39 PM Jason Gunthorpe [off-list ref] wrote:

quoted

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