| Message ID | 20190806141826.52712-3-parav@mellanox.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Simplify mtty driver and mdev core | expand |
On Tue, 6 Aug 2019 09:18:26 -0500 Parav Pandit <parav@mellanox.com> wrote: > There is no single production driver who is interested in mdev device > uuid. Currently UUID is mainly used to derive a device name. > Additionally mdev device name is already available using core kernel > API dev_name(). Well, the mdev code actually uses the uuid to check for duplicates before registration with the driver core would fail... I'd just drop the two sentences talking about the device name, IMHO they don't really add useful information; but I'll leave that decision to the maintainers. > > Hence removed unused exported symbol. > > Signed-off-by: Parav Pandit <parav@mellanox.com> > --- > Changelog: > v0->v1: > - Updated commit log to address comments from Cornelia > --- > drivers/vfio/mdev/mdev_core.c | 6 ------ > include/linux/mdev.h | 1 - > 2 files changed, 7 deletions(-) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> -----Original Message----- > From: Cornelia Huck <cohuck@redhat.com> > Sent: Wednesday, August 7, 2019 2:58 PM > To: Parav Pandit <parav@mellanox.com> > Cc: kvm@vger.kernel.org; wankhede@nvidia.com; linux- > kernel@vger.kernel.org; alex.williamson@redhat.com; cjia@nvidia.com > Subject: Re: [PATCH v1 2/2] vfio/mdev: Removed unused and redundant API for > mdev UUID > > On Tue, 6 Aug 2019 09:18:26 -0500 > Parav Pandit <parav@mellanox.com> wrote: > > > There is no single production driver who is interested in mdev device > > uuid. Currently UUID is mainly used to derive a device name. > > Additionally mdev device name is already available using core kernel > > API dev_name(). > > Well, the mdev code actually uses the uuid to check for duplicates before > registration with the driver core would fail... I'd just drop the two sentences Yes, it does the check. But its mainly used to derive a device name. And to ensure that there are no two devices with duplicate name, it compares with the uuid. Even this 16 bytes storage is redundant. Subsequently, I will submit a patch to get rid of storing this 16 bytes of UUID too. Because for duplicate name check, device name itself is pretty good enough. Since I ran out of time and rc-4 is going on, I differed the 3rd simplification patch. Commit message actually came from the thoughts of 3rd patch, but I see that without it, its not so intuitive. > talking about the device name, IMHO they don't really add useful information; > but I'll leave that decision to the maintainers. > > > > > Hence removed unused exported symbol. > > > > Signed-off-by: Parav Pandit <parav@mellanox.com> > > --- > > Changelog: > > v0->v1: > > - Updated commit log to address comments from Cornelia > > --- > > drivers/vfio/mdev/mdev_core.c | 6 ------ > > include/linux/mdev.h | 1 - > > 2 files changed, 7 deletions(-) > > Reviewed-by: Cornelia Huck <cohuck@redhat.com> Thanks for the review.
On Wed, 7 Aug 2019 16:33:11 +0000 Parav Pandit <parav@mellanox.com> wrote: > > -----Original Message----- > > From: Cornelia Huck <cohuck@redhat.com> > > Sent: Wednesday, August 7, 2019 2:58 PM > > To: Parav Pandit <parav@mellanox.com> > > Cc: kvm@vger.kernel.org; wankhede@nvidia.com; linux- > > kernel@vger.kernel.org; alex.williamson@redhat.com; cjia@nvidia.com > > Subject: Re: [PATCH v1 2/2] vfio/mdev: Removed unused and redundant API for > > mdev UUID > > > > On Tue, 6 Aug 2019 09:18:26 -0500 > > Parav Pandit <parav@mellanox.com> wrote: > > > > > There is no single production driver who is interested in mdev device > > > uuid. Currently UUID is mainly used to derive a device name. > > > Additionally mdev device name is already available using core kernel > > > API dev_name(). > > > > Well, the mdev code actually uses the uuid to check for duplicates before > > registration with the driver core would fail... I'd just drop the two sentences > Yes, it does the check. But its mainly used to derive a device name. > And to ensure that there are no two devices with duplicate name, it compares with the uuid. > > Even this 16 bytes storage is redundant. > Subsequently, I will submit a patch to get rid of storing this 16 bytes of UUID too. > Because for duplicate name check, device name itself is pretty good enough. > > Since I ran out of time and rc-4 is going on, I differed the 3rd simplification patch. I'm not sure why we'd want to ditch the uuid; it's not like it is taking up huge amounts of space... and I see the device name being derived from the unique identifier that is the uuid, and not as the unique identifier itself. > > Commit message actually came from the thoughts of 3rd patch, but I see that without it, its not so intuitive. > > > talking about the device name, IMHO they don't really add useful information; > > but I'll leave that decision to the maintainers. > > > > > > > > Hence removed unused exported symbol. > > > > > > Signed-off-by: Parav Pandit <parav@mellanox.com> > > > --- > > > Changelog: > > > v0->v1: > > > - Updated commit log to address comments from Cornelia > > > --- > > > drivers/vfio/mdev/mdev_core.c | 6 ------ > > > include/linux/mdev.h | 1 - > > > 2 files changed, 7 deletions(-) > > > > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > Thanks for the review.
> -----Original Message----- > From: Cornelia Huck <cohuck@redhat.com> > Sent: Thursday, August 8, 2019 2:00 PM > To: Parav Pandit <parav@mellanox.com> > Cc: kvm@vger.kernel.org; wankhede@nvidia.com; linux- > kernel@vger.kernel.org; alex.williamson@redhat.com; cjia@nvidia.com > Subject: Re: [PATCH v1 2/2] vfio/mdev: Removed unused and redundant API > for mdev UUID > > On Wed, 7 Aug 2019 16:33:11 +0000 > Parav Pandit <parav@mellanox.com> wrote: > > > > -----Original Message----- > > > From: Cornelia Huck <cohuck@redhat.com> > > > Sent: Wednesday, August 7, 2019 2:58 PM > > > To: Parav Pandit <parav@mellanox.com> > > > Cc: kvm@vger.kernel.org; wankhede@nvidia.com; linux- > > > kernel@vger.kernel.org; alex.williamson@redhat.com; cjia@nvidia.com > > > Subject: Re: [PATCH v1 2/2] vfio/mdev: Removed unused and redundant > > > API for mdev UUID > > > > > > On Tue, 6 Aug 2019 09:18:26 -0500 > > > Parav Pandit <parav@mellanox.com> wrote: > > > > > > > There is no single production driver who is interested in mdev > > > > device uuid. Currently UUID is mainly used to derive a device name. > > > > Additionally mdev device name is already available using core > > > > kernel API dev_name(). > > > > > > Well, the mdev code actually uses the uuid to check for duplicates > > > before registration with the driver core would fail... I'd just drop > > > the two sentences > > Yes, it does the check. But its mainly used to derive a device name. > > And to ensure that there are no two devices with duplicate name, it > compares with the uuid. > > > > Even this 16 bytes storage is redundant. > > Subsequently, I will submit a patch to get rid of storing this 16 bytes of > UUID too. > > Because for duplicate name check, device name itself is pretty good > enough. > > > > Since I ran out of time and rc-4 is going on, I differed the 3rd simplification > patch. > > I'm not sure why we'd want to ditch the uuid; it's not like it is taking up huge > amounts of space... and I see the device name being derived from the > unique identifier that is the uuid, and not as the unique identifier itself. > Its just extra storage where ID is already present in device name. Its redundant. Same functionality can be achieved without its storage, so it's better to simplify. Anyways, will handle it right after this two patches. I realized that I had typo in the email of Kirti. So resending it with corrected email. > > > > Commit message actually came from the thoughts of 3rd patch, but I see > that without it, its not so intuitive. > > > > > talking about the device name, IMHO they don't really add useful > > > information; but I'll leave that decision to the maintainers. > > > > > > > > > > > Hence removed unused exported symbol. > > > > > > > > Signed-off-by: Parav Pandit <parav@mellanox.com> > > > > --- > > > > Changelog: > > > > v0->v1: > > > > - Updated commit log to address comments from Cornelia > > > > --- > > > > drivers/vfio/mdev/mdev_core.c | 6 ------ > > > > include/linux/mdev.h | 1 - > > > > 2 files changed, 7 deletions(-) > > > > > > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > > Thanks for the review.
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index b558d4cfd082..c2b809cbe59f 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -57,12 +57,6 @@ struct mdev_device *mdev_from_dev(struct device *dev) } EXPORT_SYMBOL(mdev_from_dev); -const guid_t *mdev_uuid(struct mdev_device *mdev) -{ - return &mdev->uuid; -} -EXPORT_SYMBOL(mdev_uuid); - /* Should be called holding parent_list_lock */ static struct mdev_parent *__find_parent_device(struct device *dev) { diff --git a/include/linux/mdev.h b/include/linux/mdev.h index 0ce30ca78db0..375a5830c3d8 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -131,7 +131,6 @@ struct mdev_driver { void *mdev_get_drvdata(struct mdev_device *mdev); void mdev_set_drvdata(struct mdev_device *mdev, void *data); -const guid_t *mdev_uuid(struct mdev_device *mdev); extern struct bus_type mdev_bus_type;
There is no single production driver who is interested in mdev device uuid. Currently UUID is mainly used to derive a device name. Additionally mdev device name is already available using core kernel API dev_name(). Hence removed unused exported symbol. Signed-off-by: Parav Pandit <parav@mellanox.com> --- Changelog: v0->v1: - Updated commit log to address comments from Cornelia --- drivers/vfio/mdev/mdev_core.c | 6 ------ include/linux/mdev.h | 1 - 2 files changed, 7 deletions(-)