On Wed, Mar 10, 2021 at 4:58 AM Jason Gunthorpe [off-list ref] wrote:

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

quoted

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 :(

quoted

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

It's only worthwhile in my opinion if it makes the diffstat more
favorable for lines removed.

quoted

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.

Sounds good.

You can add:

Reviewed-by: Dan Williams <redacted>