> -----Original Message----- > From: Kirti Wankhede <kwankhede@xxxxxxxxxx> > Sent: Monday, August 12, 2019 5:06 PM > To: Alex Williamson <alex.williamson@xxxxxxxxxx>; Parav Pandit > <parav@xxxxxxxxxxxx> > Cc: kvm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; cohuck@xxxxxxxxxx; > cjia@xxxxxxxxxx > Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core > > > > On 8/9/2019 4:32 AM, Alex Williamson wrote: > > 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. > > That's right. > > > 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, > > > > Yes, NVIDIA mdev driver do use this interface. I don't agree on removing > mdev_uuid() interface. > We need to ask Greg or Linus on the kernel policy on whether an API should exist without in-kernel driver. We don't add such API in netdev, rdma and possibly other subsystem. Where can we find this mdev driver in-tree?
