On Wed, Feb 24, 2021 at 02:55:05PM -0700, Alex Williamson wrote:
> Ok, but how does this really help us, unless you're also proposing some
> redesign of the memory_lock semaphore? Even if we're zapping all the
> affected devices for a bus reset that doesn't eliminate that we still
> require device level granularity for other events.
Ok, I missed the device level one, forget this remark about the reflk
then, per vfio_pci_device is the right granularity
> > > struct vfio_pci_device {
> > > struct pci_dev *pdev;
> > > + struct vfio_device *device;
> >
> > Ah, I did this too, but I didn't use a pointer :)
>
> vfio_device is embedded in vfio.c, so that worries me.
I'm working on what we talked about in the other thread to show how
VFIO looks if it follows the normal Linux container_of idiom and to
then remove the vfio_mdev.c indirection as an illustration.
I want to directly make the case the VFIO is better off following the
standard kernel designs and driver core norms so we can find an
agreement to get Max's work on vfio_pci going forward.
You were concerned about hand waving, and maintainability so I'm
willing to put in some more work to make it concrete.
I hope you'll keep an open mind
> > All the places trying to call vfio_device_put() when they really want
> > a vfio_pci_device * become simpler now. Eg struct vfio_devices wants
> > to have an array of vfio_pci_device, and get_pf_vdev() only needs to
> > return one pointer.
>
> Sure, that example would be a good simplification. I'm not sure see
> other cases where we're going out of our way to manage the vfio_device
> versus vfio_pci_device objects though.
What I've found is there are lots of little costs all over the
place. The above was just easy to describe in this context.
In my mind the biggest negative cost is the type erasure. Lots of
places are using 'device *', 'void *', 'kobj *' as generic handles to
things that are actually already concretely typed. In the kernel I
would say concretely typing things is considered a virtue.
Having gone through alot of VFIO now, as it relates to the
vfio_device, I think the type erasure directly hurts readability and
maintainability. It just takes too much information away from the
reader and the compiler. The core code is managable, but when you
chuck mdev into it that follows the same, it really starts hurting.
Jason
