[net-next,19/19] mtty: Optionally support mtty alias
diff mbox series

Message ID 20191107160834.21087-19-parav@mellanox.com
State New
Headers show
Series
  • Mellanox, mlx5 sub function support
Related show

Commit Message

Parav Pandit Nov. 7, 2019, 4:08 p.m. UTC
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(+)

Comments

Leon Romanovsky Nov. 8, 2019, 6:26 a.m. UTC | #1
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
>
Jiri Pirko Nov. 8, 2019, 10:45 a.m. UTC | #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).
Cornelia Huck Nov. 8, 2019, 1:46 p.m. UTC | #3
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.
Parav Pandit Nov. 8, 2019, 3:08 p.m. UTC | #4
> -----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
Parav Pandit Nov. 8, 2019, 3:10 p.m. UTC | #5
> -----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.
Jiri Pirko Nov. 8, 2019, 3:15 p.m. UTC | #6
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
>
>
Cornelia Huck Nov. 8, 2019, 3:28 p.m. UTC | #7
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.
Parav Pandit Nov. 8, 2019, 3:30 p.m. UTC | #8
> -----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? :-)
Alex Williamson Nov. 8, 2019, 5:54 p.m. UTC | #9
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

Patch
diff mbox series

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)