| Message ID | 20191107160834.21087-19-parav@mellanox.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Mellanox, mlx5 sub function support | expand |
On Thu, Nov 07, 2019 at 10:08:34AM -0600, Parav Pandit wrote: > Provide a module parameter to set alias length to optionally generate > mdev alias. Why do we need it? > > Example to request mdev alias. > $ modprobe mtty alias_length=12 > > Make use of mtty_alias() API when alias_length module parameter is set. > > Signed-off-by: Parav Pandit <parav@mellanox.com> > --- > samples/vfio-mdev/mtty.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c > index ce84a300a4da..5a69121ed5ec 100644 > --- a/samples/vfio-mdev/mtty.c > +++ b/samples/vfio-mdev/mtty.c > @@ -150,6 +150,10 @@ static const struct file_operations vd_fops = { > .owner = THIS_MODULE, > }; > > +static unsigned int mtty_alias_length; > +module_param_named(alias_length, mtty_alias_length, uint, 0444); > +MODULE_PARM_DESC(alias_length, "mdev alias length; default=0"); > + > /* function prototypes */ > > static int mtty_trigger_interrupt(struct mdev_state *mdev_state); > @@ -755,6 +759,9 @@ static int mtty_create(struct kobject *kobj, struct mdev_device *mdev) > list_add(&mdev_state->next, &mdev_devices_list); > mutex_unlock(&mdev_list_lock); > > + if (mtty_alias_length) > + dev_dbg(mdev_dev(mdev), "alias is %s\n", mdev_alias(mdev)); > + > return 0; > } > > @@ -1387,6 +1394,11 @@ static struct attribute_group *mdev_type_groups[] = { > NULL, > }; > > +static unsigned int mtty_get_alias_length(void) > +{ > + return mtty_alias_length; > +} > + > static const struct mdev_parent_ops mdev_fops = { > .owner = THIS_MODULE, > .dev_attr_groups = mtty_dev_groups, > @@ -1399,6 +1411,7 @@ static const struct mdev_parent_ops mdev_fops = { > .read = mtty_read, > .write = mtty_write, > .ioctl = mtty_ioctl, > + .get_alias_length = mtty_get_alias_length > }; > > static void mtty_device_release(struct device *dev) > -- > 2.19.2 >
Thu, Nov 07, 2019 at 05:08:34PM CET, parav@mellanox.com wrote: >Provide a module parameter to set alias length to optionally generate >mdev alias. > >Example to request mdev alias. >$ modprobe mtty alias_length=12 > >Make use of mtty_alias() API when alias_length module parameter is set. > >Signed-off-by: Parav Pandit <parav@mellanox.com> This patch looks kind of unrelated to the rest of the set. I think that you can either: 1) send this patch as a separate follow-up to this patchset 2) use this patch as a user and push out the mdev alias patches out of this patchset to a separate one (I fear that this was discussed and declined before).
On Thu, 7 Nov 2019 10:08:34 -0600 Parav Pandit <parav@mellanox.com> wrote: > Provide a module parameter to set alias length to optionally generate > mdev alias. > > Example to request mdev alias. > $ modprobe mtty alias_length=12 > > Make use of mtty_alias() API when alias_length module parameter is set. > > Signed-off-by: Parav Pandit <parav@mellanox.com> > --- > samples/vfio-mdev/mtty.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) If you already have code using the alias interface, you probably don't need to add it to the sample driver here. Especially as the alias looks kind of pointless here.
> -----Original Message----- > From: Jiri Pirko <jiri@resnulli.us> > Sent: Friday, November 8, 2019 4:45 AM > To: Parav Pandit <parav@mellanox.com> > Cc: alex.williamson@redhat.com; davem@davemloft.net; > kvm@vger.kernel.org; netdev@vger.kernel.org; Saeed Mahameed > <saeedm@mellanox.com>; kwankhede@nvidia.com; leon@kernel.org; > cohuck@redhat.com; Jiri Pirko <jiri@mellanox.com>; linux- > rdma@vger.kernel.org > Subject: Re: [PATCH net-next 19/19] mtty: Optionally support mtty alias > > Thu, Nov 07, 2019 at 05:08:34PM CET, parav@mellanox.com wrote: > >Provide a module parameter to set alias length to optionally generate > >mdev alias. > > > >Example to request mdev alias. > >$ modprobe mtty alias_length=12 > > > >Make use of mtty_alias() API when alias_length module parameter is set. > > > >Signed-off-by: Parav Pandit <parav@mellanox.com> > > This patch looks kind of unrelated to the rest of the set. > I think that you can either: > 1) send this patch as a separate follow-up to this patchset > 2) use this patch as a user and push out the mdev alias patches out of this > patchset to a separate one (I fear that this was discussed and declined > before). Yes, we already discussed to run mdev 5-6 patches as pre-patch before this series when reviewed on kvm mailing list. Alex was suggesting to package with this series as mlx5_core being the first user. Series will have conflict (not this patch) if Jason Wang's series [9] is merged first. So please let me know how shall we do it. [9] https://patchwork.ozlabs.org/patch/1190425
> -----Original Message----- > From: Cornelia Huck <cohuck@redhat.com> > Sent: Friday, November 8, 2019 7:46 AM > To: Parav Pandit <parav@mellanox.com> > Cc: alex.williamson@redhat.com; davem@davemloft.net; > kvm@vger.kernel.org; netdev@vger.kernel.org; Saeed Mahameed > <saeedm@mellanox.com>; kwankhede@nvidia.com; leon@kernel.org; Jiri > Pirko <jiri@mellanox.com>; linux-rdma@vger.kernel.org > Subject: Re: [PATCH net-next 19/19] mtty: Optionally support mtty alias > > On Thu, 7 Nov 2019 10:08:34 -0600 > Parav Pandit <parav@mellanox.com> wrote: > > > Provide a module parameter to set alias length to optionally generate > > mdev alias. > > > > Example to request mdev alias. > > $ modprobe mtty alias_length=12 > > > > Make use of mtty_alias() API when alias_length module parameter is set. > > > > Signed-off-by: Parav Pandit <parav@mellanox.com> > > --- > > samples/vfio-mdev/mtty.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > If you already have code using the alias interface, you probably don't need > to add it to the sample driver here. Especially as the alias looks kind of > pointless here. It is pointless. Alex point when we ran through the series in August, was, QA should be able to do cover coverage of mdev_core where there is mdev collision and mdev_create() can fail. And QA should be able to set alias length to be short to 1 or 2 letters to trigger it. Hence this patch was added.
Fri, Nov 08, 2019 at 04:08:22PM CET, parav@mellanox.com wrote: > > >> -----Original Message----- >> From: Jiri Pirko <jiri@resnulli.us> >> Sent: Friday, November 8, 2019 4:45 AM >> To: Parav Pandit <parav@mellanox.com> >> Cc: alex.williamson@redhat.com; davem@davemloft.net; >> kvm@vger.kernel.org; netdev@vger.kernel.org; Saeed Mahameed >> <saeedm@mellanox.com>; kwankhede@nvidia.com; leon@kernel.org; >> cohuck@redhat.com; Jiri Pirko <jiri@mellanox.com>; linux- >> rdma@vger.kernel.org >> Subject: Re: [PATCH net-next 19/19] mtty: Optionally support mtty alias >> >> Thu, Nov 07, 2019 at 05:08:34PM CET, parav@mellanox.com wrote: >> >Provide a module parameter to set alias length to optionally generate >> >mdev alias. >> > >> >Example to request mdev alias. >> >$ modprobe mtty alias_length=12 >> > >> >Make use of mtty_alias() API when alias_length module parameter is set. >> > >> >Signed-off-by: Parav Pandit <parav@mellanox.com> >> >> This patch looks kind of unrelated to the rest of the set. >> I think that you can either: >> 1) send this patch as a separate follow-up to this patchset >> 2) use this patch as a user and push out the mdev alias patches out of this >> patchset to a separate one (I fear that this was discussed and declined >> before). >Yes, we already discussed to run mdev 5-6 patches as pre-patch before this series when reviewed on kvm mailing list. >Alex was suggesting to package with this series as mlx5_core being the first user. >Series will have conflict (not this patch) if Jason Wang's series [9] is merged first. >So please let me know how shall we do it. Just remove this patch from the set and push it later on individually (if ever). > >[9] https://patchwork.ozlabs.org/patch/1190425 > >
On Fri, 8 Nov 2019 15:10:42 +0000 Parav Pandit <parav@mellanox.com> wrote: > > -----Original Message----- > > From: Cornelia Huck <cohuck@redhat.com> > > Sent: Friday, November 8, 2019 7:46 AM > > To: Parav Pandit <parav@mellanox.com> > > Cc: alex.williamson@redhat.com; davem@davemloft.net; > > kvm@vger.kernel.org; netdev@vger.kernel.org; Saeed Mahameed > > <saeedm@mellanox.com>; kwankhede@nvidia.com; leon@kernel.org; Jiri > > Pirko <jiri@mellanox.com>; linux-rdma@vger.kernel.org > > Subject: Re: [PATCH net-next 19/19] mtty: Optionally support mtty alias > > > > On Thu, 7 Nov 2019 10:08:34 -0600 > > Parav Pandit <parav@mellanox.com> wrote: > > > > > Provide a module parameter to set alias length to optionally generate > > > mdev alias. > > > > > > Example to request mdev alias. > > > $ modprobe mtty alias_length=12 > > > > > > Make use of mtty_alias() API when alias_length module parameter is set. > > > > > > Signed-off-by: Parav Pandit <parav@mellanox.com> > > > --- > > > samples/vfio-mdev/mtty.c | 13 +++++++++++++ > > > 1 file changed, 13 insertions(+) > > > > If you already have code using the alias interface, you probably don't need > > to add it to the sample driver here. Especially as the alias looks kind of > > pointless here. > > It is pointless. > Alex point when we ran through the series in August, was, QA should be able to do cover coverage of mdev_core where there is mdev collision and mdev_create() can fail. > And QA should be able to set alias length to be short to 1 or 2 letters to trigger it. > Hence this patch was added. If we want this for testing purposes, that should be spelled out explicitly (the above had already dropped from my cache). Even better if we had something in actual test infrastructure.
> -----Original Message----- > From: Cornelia Huck <cohuck@redhat.com> > Sent: Friday, November 8, 2019 9:28 AM > To: Parav Pandit <parav@mellanox.com> > Cc: alex.williamson@redhat.com; davem@davemloft.net; > kvm@vger.kernel.org; netdev@vger.kernel.org; Saeed Mahameed > <saeedm@mellanox.com>; kwankhede@nvidia.com; leon@kernel.org; Jiri > Pirko <jiri@mellanox.com>; linux-rdma@vger.kernel.org > Subject: Re: [PATCH net-next 19/19] mtty: Optionally support mtty alias > > On Fri, 8 Nov 2019 15:10:42 +0000 > Parav Pandit <parav@mellanox.com> wrote: > > > > -----Original Message----- > > > From: Cornelia Huck <cohuck@redhat.com> > > > Sent: Friday, November 8, 2019 7:46 AM > > > To: Parav Pandit <parav@mellanox.com> > > > Cc: alex.williamson@redhat.com; davem@davemloft.net; > > > kvm@vger.kernel.org; netdev@vger.kernel.org; Saeed Mahameed > > > <saeedm@mellanox.com>; kwankhede@nvidia.com; leon@kernel.org; Jiri > > > Pirko <jiri@mellanox.com>; linux-rdma@vger.kernel.org > > > Subject: Re: [PATCH net-next 19/19] mtty: Optionally support mtty > > > alias > > > > > > On Thu, 7 Nov 2019 10:08:34 -0600 > > > Parav Pandit <parav@mellanox.com> wrote: > > > > > > > Provide a module parameter to set alias length to optionally > > > > generate mdev alias. > > > > > > > > Example to request mdev alias. > > > > $ modprobe mtty alias_length=12 > > > > > > > > Make use of mtty_alias() API when alias_length module parameter is > set. > > > > > > > > Signed-off-by: Parav Pandit <parav@mellanox.com> > > > > --- > > > > samples/vfio-mdev/mtty.c | 13 +++++++++++++ > > > > 1 file changed, 13 insertions(+) > > > > > > If you already have code using the alias interface, you probably > > > don't need to add it to the sample driver here. Especially as the > > > alias looks kind of pointless here. > > > > It is pointless. > > Alex point when we ran through the series in August, was, QA should be > able to do cover coverage of mdev_core where there is mdev collision and > mdev_create() can fail. > > And QA should be able to set alias length to be short to 1 or 2 letters to > trigger it. > > Hence this patch was added. > > If we want this for testing purposes, that should be spelled out explicitly (the > above had already dropped from my cache). Even better if we had > something in actual test infrastructure. What else purpose sample driver has other than getting reference on how to use API? :-)
On Fri, 8 Nov 2019 15:30:59 +0000 Parav Pandit <parav@mellanox.com> wrote: > > -----Original Message----- > > From: Cornelia Huck <cohuck@redhat.com> > > Sent: Friday, November 8, 2019 9:28 AM > > To: Parav Pandit <parav@mellanox.com> > > Cc: alex.williamson@redhat.com; davem@davemloft.net; > > kvm@vger.kernel.org; netdev@vger.kernel.org; Saeed Mahameed > > <saeedm@mellanox.com>; kwankhede@nvidia.com; leon@kernel.org; Jiri > > Pirko <jiri@mellanox.com>; linux-rdma@vger.kernel.org > > Subject: Re: [PATCH net-next 19/19] mtty: Optionally support mtty alias > > > > On Fri, 8 Nov 2019 15:10:42 +0000 > > Parav Pandit <parav@mellanox.com> wrote: > > > > > > -----Original Message----- > > > > From: Cornelia Huck <cohuck@redhat.com> > > > > Sent: Friday, November 8, 2019 7:46 AM > > > > To: Parav Pandit <parav@mellanox.com> > > > > Cc: alex.williamson@redhat.com; davem@davemloft.net; > > > > kvm@vger.kernel.org; netdev@vger.kernel.org; Saeed Mahameed > > > > <saeedm@mellanox.com>; kwankhede@nvidia.com; leon@kernel.org; Jiri > > > > Pirko <jiri@mellanox.com>; linux-rdma@vger.kernel.org > > > > Subject: Re: [PATCH net-next 19/19] mtty: Optionally support mtty > > > > alias > > > > > > > > On Thu, 7 Nov 2019 10:08:34 -0600 > > > > Parav Pandit <parav@mellanox.com> wrote: > > > > > > > > > Provide a module parameter to set alias length to optionally > > > > > generate mdev alias. > > > > > > > > > > Example to request mdev alias. > > > > > $ modprobe mtty alias_length=12 > > > > > > > > > > Make use of mtty_alias() API when alias_length module parameter is > > set. > > > > > > > > > > Signed-off-by: Parav Pandit <parav@mellanox.com> > > > > > --- > > > > > samples/vfio-mdev/mtty.c | 13 +++++++++++++ > > > > > 1 file changed, 13 insertions(+) > > > > > > > > If you already have code using the alias interface, you probably > > > > don't need to add it to the sample driver here. Especially as the > > > > alias looks kind of pointless here. > > > > > > It is pointless. > > > Alex point when we ran through the series in August, was, QA should be > > able to do cover coverage of mdev_core where there is mdev collision and > > mdev_create() can fail. > > > And QA should be able to set alias length to be short to 1 or 2 letters to > > trigger it. > > > Hence this patch was added. > > > > If we want this for testing purposes, that should be spelled out explicitly (the > > above had already dropped from my cache). Even better if we had > > something in actual test infrastructure. > > What else purpose sample driver has other than getting reference on how to use API? :-) Yup, personally I still find the ROI for this worthwhile. It gives us a mechanism to test aliases, and particularly alias collisions, without special hardware, as well as providing an example of the API. FWIW, there will be merge conflicts with the alias support in this series versus the mdev parent ops refactoring to allow class specific device ops. As the use of the alias support solidifies we can revisit which branch we want to use to merge it upstream. Thanks, Alex
diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c index ce84a300a4da..5a69121ed5ec 100644 --- a/samples/vfio-mdev/mtty.c +++ b/samples/vfio-mdev/mtty.c @@ -150,6 +150,10 @@ static const struct file_operations vd_fops = { .owner = THIS_MODULE, }; +static unsigned int mtty_alias_length; +module_param_named(alias_length, mtty_alias_length, uint, 0444); +MODULE_PARM_DESC(alias_length, "mdev alias length; default=0"); + /* function prototypes */ static int mtty_trigger_interrupt(struct mdev_state *mdev_state); @@ -755,6 +759,9 @@ static int mtty_create(struct kobject *kobj, struct mdev_device *mdev) list_add(&mdev_state->next, &mdev_devices_list); mutex_unlock(&mdev_list_lock); + if (mtty_alias_length) + dev_dbg(mdev_dev(mdev), "alias is %s\n", mdev_alias(mdev)); + return 0; } @@ -1387,6 +1394,11 @@ static struct attribute_group *mdev_type_groups[] = { NULL, }; +static unsigned int mtty_get_alias_length(void) +{ + return mtty_alias_length; +} + static const struct mdev_parent_ops mdev_fops = { .owner = THIS_MODULE, .dev_attr_groups = mtty_dev_groups, @@ -1399,6 +1411,7 @@ static const struct mdev_parent_ops mdev_fops = { .read = mtty_read, .write = mtty_write, .ioctl = mtty_ioctl, + .get_alias_length = mtty_get_alias_length }; static void mtty_device_release(struct device *dev)
Provide a module parameter to set alias length to optionally generate mdev alias. Example to request mdev alias. $ modprobe mtty alias_length=12 Make use of mtty_alias() API when alias_length module parameter is set. Signed-off-by: Parav Pandit <parav@mellanox.com> --- samples/vfio-mdev/mtty.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)