Re: [PATCH 3/8] vfio/mdev: simplify mdev_type handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 6/7/2022 11:20 AM, Christoph Hellwig wrote:
On Tue, Jun 07, 2022 at 12:52:49AM +0530, Kirti Wankhede wrote:
   	void (*remove)(struct mdev_device *dev);
-	struct attribute_group **supported_type_groups;
   	struct device_driver driver;
   };

mdev_type should be part of mdev_parent, separating it from mdev_parent
could result in more errors while using mdev framework.

Why?

Similarly it should
be added as part of mdev_register_device. Below adding types is separated
from mdev_register_device which is more error prone.

How so?


Jason has already pointed that about udev event.

What if driver
registering to mdev doesn't add mdev_types? - mdev framework is un-usable
in that case.

Yes, so it is if you don't add it to the supported_type_groups field
in the current kernel.  Basic programmer error, and trivially caught.


Current kernel version mandate supported_type_groups, otherwise mdev registration fails. But this behavior is being changed with this patch.


We had kept it together with mdev registration so that
mdev_types should be mandatory to be defined by driver during registration.
How would you mandate mdev_type by such separation?

I would not.  Registering a parent without types is perfectly valid from
the code correctness perspective.  It just isn't very useful.  Just
like say creating a kobject without attributes in the device model.

Creating kobject without kobj_type is not allowed in the kernel, similarly mdev registration should not be allowed without its type.

Instead of exporting mdev_type_add/mdev_type_remove, these functions might be called internally from registration function.

Thanks,
Kirti



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux