On Thu, 8 Aug 2019 09:12:53 -0500 Parav Pandit <parav@xxxxxxxxxxxx> wrote: > Currently mtty sample driver uses mdev state and UUID in convoluated way to > generate an interrupt. > It uses several translations from mdev_state to mdev_device to mdev uuid. > After which it does linear search of long uuid comparision to > find out mdev_state in mtty_trigger_interrupt(). > mdev_state is already available while generating interrupt from which all > such translations are done to reach back to mdev_state. > > This translations are done during interrupt generation path. > This is unnecessary and reduandant. Is the interrupt handling efficiency of this particular sample driver really relevant, or is its purpose more to illustrate the API and provide a proof of concept? If we go to the trouble to optimize the sample driver and remove this interface from the API, what do we lose? This interface was added via commit: 99e3123e3d72 vfio-mdev: Make mdev_device private and abstract interfaces Where the goal was to create a more formal interface and abstract driver access to the struct mdev_device. In part this served to make out-of-tree mdev vendor drivers more supportable; the object is considered opaque and access is provided via an API rather than through direct structure fields. I believe that the NVIDIA GRID mdev driver does make use of this interface and it's likely included in the sample driver specifically so that there is an in-kernel user for it (ie. specifically to avoid it being removed so casually). An interesting feature of the NVIDIA mdev driver is that I believe it has portions that run in userspace. As we know, mdevs are named with a UUID, so I can imagine there are some efficiencies to be gained in having direct access to the UUID for a device when interacting with userspace, rather than repeatedly parsing it from a device name. Is that really something we want to make more difficult in order to optimize a sample driver? Knowing that an mdev device uses a UUID for it's name, as tools like libvirt and mdevctl expect, is it really worthwhile to remove such a trivial API? > Hence, > Patch-1 simplifies mtty sample driver to directly use mdev_state. > > Patch-2, Since no production driver uses mdev_uuid(), simplifies and > removes redandant mdev_uuid() exported symbol. s/no production driver/no in-kernel production driver/ I'd be interested to hear how the NVIDIA folks make use of this API interface. Thanks, Alex > --- > Changelog: > v1->v2: > - Corrected email of Kirti > - Updated cover letter commit log to address comment from Cornelia > - Added Reviewed-by tag > v0->v1: > - Updated commit log > > Parav Pandit (2): > vfio-mdev/mtty: Simplify interrupt generation > vfio/mdev: Removed unused and redundant API for mdev UUID > > drivers/vfio/mdev/mdev_core.c | 6 ------ > include/linux/mdev.h | 1 - > samples/vfio-mdev/mtty.c | 39 +++++++---------------------------- > 3 files changed, 8 insertions(+), 38 deletions(-) >
