All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: vt6656: Declare a few variables as __read_mostly
@ 2020-03-01 11:26 ` Oscar Carter
  0 siblings, 0 replies; 18+ messages in thread
From: Oscar Carter @ 2020-03-01 11:26 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman
  Cc: Quentin Deslandes, Malcolm Priestley, Gabriela Bittencourt,
	Colin Ian King, Oscar Carter, devel, linux-kernel

These include module parameters.

Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
---
 drivers/staging/vt6656/main_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
index 5e48b3ddb94c..701300202b21 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);

 #define RX_DESC_DEF0 64
-static int vnt_rx_buffers = RX_DESC_DEF0;
+static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
 module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
 MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");

 #define TX_DESC_DEF0 64
-static int vnt_tx_buffers = TX_DESC_DEF0;
+static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
 module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
 MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");

--
2.20.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH] staging: vt6656: Declare a few variables as __read_mostly
@ 2020-03-01 11:26 ` Oscar Carter
  0 siblings, 0 replies; 18+ messages in thread
From: Oscar Carter @ 2020-03-01 11:26 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman
  Cc: devel, Oscar Carter, Malcolm Priestley, linux-kernel,
	Gabriela Bittencourt, Colin Ian King

These include module parameters.

Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
---
 drivers/staging/vt6656/main_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
index 5e48b3ddb94c..701300202b21 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);

 #define RX_DESC_DEF0 64
-static int vnt_rx_buffers = RX_DESC_DEF0;
+static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
 module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
 MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");

 #define TX_DESC_DEF0 64
-static int vnt_tx_buffers = TX_DESC_DEF0;
+static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
 module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
 MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");

--
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
  2020-03-01 11:26 ` Oscar Carter
@ 2020-03-01 12:25   ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2020-03-01 12:25 UTC (permalink / raw)
  To: Oscar Carter
  Cc: Forest Bond, devel, Malcolm Priestley, linux-kernel,
	Gabriela Bittencourt, Colin Ian King

On Sun, Mar 01, 2020 at 12:26:20PM +0100, Oscar Carter wrote:
> These include module parameters.
> 
> Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> ---
>  drivers/staging/vt6656/main_usb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
> index 5e48b3ddb94c..701300202b21 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);
> 
>  #define RX_DESC_DEF0 64
> -static int vnt_rx_buffers = RX_DESC_DEF0;
> +static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
>  module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
>  MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");
> 
>  #define TX_DESC_DEF0 64
> -static int vnt_tx_buffers = TX_DESC_DEF0;
> +static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
>  module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
>  MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");
> 

Why?  What does this help with?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
@ 2020-03-01 12:25   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2020-03-01 12:25 UTC (permalink / raw)
  To: Oscar Carter
  Cc: devel, Malcolm Priestley, linux-kernel, Forest Bond,
	Gabriela Bittencourt, Colin Ian King

On Sun, Mar 01, 2020 at 12:26:20PM +0100, Oscar Carter wrote:
> These include module parameters.
> 
> Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> ---
>  drivers/staging/vt6656/main_usb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
> index 5e48b3ddb94c..701300202b21 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);
> 
>  #define RX_DESC_DEF0 64
> -static int vnt_rx_buffers = RX_DESC_DEF0;
> +static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
>  module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
>  MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");
> 
>  #define TX_DESC_DEF0 64
> -static int vnt_tx_buffers = TX_DESC_DEF0;
> +static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
>  module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
>  MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");
> 

Why?  What does this help with?

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
  2020-03-01 12:25   ` Greg Kroah-Hartman
@ 2020-03-01 13:17     ` Oscar Carter
  -1 siblings, 0 replies; 18+ messages in thread
From: Oscar Carter @ 2020-03-01 13:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Forest Bond, devel, Malcolm Priestley, linux-kernel,
	Gabriela Bittencourt, Colin Ian King, Oscar Carter

On Sun, Mar 01, 2020 at 01:25:14PM +0100, Greg Kroah-Hartman wrote:
> On Sun, Mar 01, 2020 at 12:26:20PM +0100, Oscar Carter wrote:
> > These include module parameters.
> >
> > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > ---
> >  drivers/staging/vt6656/main_usb.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
> > index 5e48b3ddb94c..701300202b21 100644
> > --- a/drivers/staging/vt6656/main_usb.c
> > +++ b/drivers/staging/vt6656/main_usb.c
> > @@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
> >  MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);
> >
> >  #define RX_DESC_DEF0 64
> > -static int vnt_rx_buffers = RX_DESC_DEF0;
> > +static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
> >  module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
> >  MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");
> >
> >  #define TX_DESC_DEF0 64
> > -static int vnt_tx_buffers = TX_DESC_DEF0;
> > +static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
> >  module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
> >  MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");
> >
>
> Why?  What does this help with?

If we declare these variables __read_mostly we can improve the performance. If
these variables are read many more times than written, each core of a multicore
system can maintain a copy in a local cache and the time to access is less than
if they use the shared-cache.

>
> thanks,
>
> greg k-h

thanks,

oscar carter

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
@ 2020-03-01 13:17     ` Oscar Carter
  0 siblings, 0 replies; 18+ messages in thread
From: Oscar Carter @ 2020-03-01 13:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: devel, Oscar Carter, Malcolm Priestley, linux-kernel,
	Forest Bond, Gabriela Bittencourt, Colin Ian King

On Sun, Mar 01, 2020 at 01:25:14PM +0100, Greg Kroah-Hartman wrote:
> On Sun, Mar 01, 2020 at 12:26:20PM +0100, Oscar Carter wrote:
> > These include module parameters.
> >
> > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > ---
> >  drivers/staging/vt6656/main_usb.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
> > index 5e48b3ddb94c..701300202b21 100644
> > --- a/drivers/staging/vt6656/main_usb.c
> > +++ b/drivers/staging/vt6656/main_usb.c
> > @@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
> >  MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);
> >
> >  #define RX_DESC_DEF0 64
> > -static int vnt_rx_buffers = RX_DESC_DEF0;
> > +static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
> >  module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
> >  MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");
> >
> >  #define TX_DESC_DEF0 64
> > -static int vnt_tx_buffers = TX_DESC_DEF0;
> > +static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
> >  module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
> >  MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");
> >
>
> Why?  What does this help with?

If we declare these variables __read_mostly we can improve the performance. If
these variables are read many more times than written, each core of a multicore
system can maintain a copy in a local cache and the time to access is less than
if they use the shared-cache.

>
> thanks,
>
> greg k-h

thanks,

oscar carter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
  2020-03-01 13:17     ` Oscar Carter
@ 2020-03-01 15:09       ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2020-03-01 15:09 UTC (permalink / raw)
  To: Oscar Carter
  Cc: devel, Malcolm Priestley, linux-kernel, Forest Bond,
	Gabriela Bittencourt, Colin Ian King

On Sun, Mar 01, 2020 at 02:17:01PM +0100, Oscar Carter wrote:
> On Sun, Mar 01, 2020 at 01:25:14PM +0100, Greg Kroah-Hartman wrote:
> > On Sun, Mar 01, 2020 at 12:26:20PM +0100, Oscar Carter wrote:
> > > These include module parameters.
> > >
> > > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > > ---
> > >  drivers/staging/vt6656/main_usb.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
> > > index 5e48b3ddb94c..701300202b21 100644
> > > --- a/drivers/staging/vt6656/main_usb.c
> > > +++ b/drivers/staging/vt6656/main_usb.c
> > > @@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
> > >  MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);
> > >
> > >  #define RX_DESC_DEF0 64
> > > -static int vnt_rx_buffers = RX_DESC_DEF0;
> > > +static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
> > >  module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
> > >  MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");
> > >
> > >  #define TX_DESC_DEF0 64
> > > -static int vnt_tx_buffers = TX_DESC_DEF0;
> > > +static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
> > >  module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
> > >  MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");
> > >
> >
> > Why?  What does this help with?
> 
> If we declare these variables __read_mostly we can improve the performance. If
> these variables are read many more times than written, each core of a multicore
> system can maintain a copy in a local cache and the time to access is less than
> if they use the shared-cache.

This is a USB driver, performance is always limited to the hardware, not
the CPU location of variables.

Please always benchmark things to see if it actually makes sense to make
changes like this, before proposing them.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
@ 2020-03-01 15:09       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2020-03-01 15:09 UTC (permalink / raw)
  To: Oscar Carter
  Cc: devel, Malcolm Priestley, linux-kernel, Forest Bond,
	Gabriela Bittencourt, Colin Ian King

On Sun, Mar 01, 2020 at 02:17:01PM +0100, Oscar Carter wrote:
> On Sun, Mar 01, 2020 at 01:25:14PM +0100, Greg Kroah-Hartman wrote:
> > On Sun, Mar 01, 2020 at 12:26:20PM +0100, Oscar Carter wrote:
> > > These include module parameters.
> > >
> > > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > > ---
> > >  drivers/staging/vt6656/main_usb.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
> > > index 5e48b3ddb94c..701300202b21 100644
> > > --- a/drivers/staging/vt6656/main_usb.c
> > > +++ b/drivers/staging/vt6656/main_usb.c
> > > @@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
> > >  MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);
> > >
> > >  #define RX_DESC_DEF0 64
> > > -static int vnt_rx_buffers = RX_DESC_DEF0;
> > > +static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
> > >  module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
> > >  MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");
> > >
> > >  #define TX_DESC_DEF0 64
> > > -static int vnt_tx_buffers = TX_DESC_DEF0;
> > > +static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
> > >  module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
> > >  MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");
> > >
> >
> > Why?  What does this help with?
> 
> If we declare these variables __read_mostly we can improve the performance. If
> these variables are read many more times than written, each core of a multicore
> system can maintain a copy in a local cache and the time to access is less than
> if they use the shared-cache.

This is a USB driver, performance is always limited to the hardware, not
the CPU location of variables.

Please always benchmark things to see if it actually makes sense to make
changes like this, before proposing them.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
  2020-03-01 11:26 ` Oscar Carter
@ 2020-03-02 15:43   ` Quentin Deslandes
  -1 siblings, 0 replies; 18+ messages in thread
From: Quentin Deslandes @ 2020-03-02 15:43 UTC (permalink / raw)
  To: Oscar Carter
  Cc: Forest Bond, Greg Kroah-Hartman, Malcolm Priestley,
	Gabriela Bittencourt, Colin Ian King, devel, linux-kernel

On Sun, Mar 01, 2020 at 12:26:20PM +0100, Oscar Carter wrote:
> These include module parameters.
> 
> Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> ---
>  drivers/staging/vt6656/main_usb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
> index 5e48b3ddb94c..701300202b21 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);
> 
>  #define RX_DESC_DEF0 64
> -static int vnt_rx_buffers = RX_DESC_DEF0;
> +static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
>  module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
>  MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");
> 
>  #define TX_DESC_DEF0 64
> -static int vnt_tx_buffers = TX_DESC_DEF0;
> +static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
>  module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
>  MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");
> 
> --
> 2.20.1
> 

Looks good to me.

Reviewed-by: Quentin Deslandes <quentin.deslandes@itdev.co.uk>


^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
@ 2020-03-02 15:43   ` Quentin Deslandes
  0 siblings, 0 replies; 18+ messages in thread
From: Quentin Deslandes @ 2020-03-02 15:43 UTC (permalink / raw)
  To: Oscar Carter
  Cc: devel, Malcolm Priestley, Greg Kroah-Hartman, linux-kernel,
	Forest Bond, Gabriela Bittencourt, Colin Ian King

On Sun, Mar 01, 2020 at 12:26:20PM +0100, Oscar Carter wrote:
> These include module parameters.
> 
> Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> ---
>  drivers/staging/vt6656/main_usb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
> index 5e48b3ddb94c..701300202b21 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);
> 
>  #define RX_DESC_DEF0 64
> -static int vnt_rx_buffers = RX_DESC_DEF0;
> +static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
>  module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
>  MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");
> 
>  #define TX_DESC_DEF0 64
> -static int vnt_tx_buffers = TX_DESC_DEF0;
> +static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
>  module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
>  MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");
> 
> --
> 2.20.1
> 

Looks good to me.

Reviewed-by: Quentin Deslandes <quentin.deslandes@itdev.co.uk>

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
  2020-03-01 15:09       ` Greg Kroah-Hartman
@ 2020-03-07  8:29         ` Oscar Carter
  -1 siblings, 0 replies; 18+ messages in thread
From: Oscar Carter @ 2020-03-07  8:29 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Forest Bond, Quentin Deslandes, Malcolm Priestley,
	Gabriela Bittencourt, Colin Ian King, Oscar Carter, devel,
	linux-kernel

On Sun, Mar 01, 2020 at 04:09:13PM +0100, Greg Kroah-Hartman wrote:
> On Sun, Mar 01, 2020 at 02:17:01PM +0100, Oscar Carter wrote:
> > On Sun, Mar 01, 2020 at 01:25:14PM +0100, Greg Kroah-Hartman wrote:
> > > On Sun, Mar 01, 2020 at 12:26:20PM +0100, Oscar Carter wrote:
> > > > These include module parameters.
> > > >
> > > > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > > > ---
> > > >  drivers/staging/vt6656/main_usb.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
> > > > index 5e48b3ddb94c..701300202b21 100644
> > > > --- a/drivers/staging/vt6656/main_usb.c
> > > > +++ b/drivers/staging/vt6656/main_usb.c
> > > > @@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
> > > >  MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);
> > > >
> > > >  #define RX_DESC_DEF0 64
> > > > -static int vnt_rx_buffers = RX_DESC_DEF0;
> > > > +static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
> > > >  module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
> > > >  MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");
> > > >
> > > >  #define TX_DESC_DEF0 64
> > > > -static int vnt_tx_buffers = TX_DESC_DEF0;
> > > > +static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
> > > >  module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
> > > >  MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");
> > > >
> > >
> > > Why?  What does this help with?
> >
> > If we declare these variables __read_mostly we can improve the performance. If
> > these variables are read many more times than written, each core of a multicore
> > system can maintain a copy in a local cache and the time to access is less than
> > if they use the shared-cache.
>
> This is a USB driver, performance is always limited to the hardware, not
> the CPU location of variables.

Thank you for the explanation.

>
> Please always benchmark things to see if it actually makes sense to make
> changes like this, before proposing them.

I'm sorry.

>
> thanks,
>
> greg k-h

thanks,

Oscar

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
@ 2020-03-07  8:29         ` Oscar Carter
  0 siblings, 0 replies; 18+ messages in thread
From: Oscar Carter @ 2020-03-07  8:29 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: devel, Oscar Carter, Malcolm Priestley, linux-kernel,
	Forest Bond, Gabriela Bittencourt, Colin Ian King

On Sun, Mar 01, 2020 at 04:09:13PM +0100, Greg Kroah-Hartman wrote:
> On Sun, Mar 01, 2020 at 02:17:01PM +0100, Oscar Carter wrote:
> > On Sun, Mar 01, 2020 at 01:25:14PM +0100, Greg Kroah-Hartman wrote:
> > > On Sun, Mar 01, 2020 at 12:26:20PM +0100, Oscar Carter wrote:
> > > > These include module parameters.
> > > >
> > > > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > > > ---
> > > >  drivers/staging/vt6656/main_usb.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
> > > > index 5e48b3ddb94c..701300202b21 100644
> > > > --- a/drivers/staging/vt6656/main_usb.c
> > > > +++ b/drivers/staging/vt6656/main_usb.c
> > > > @@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
> > > >  MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);
> > > >
> > > >  #define RX_DESC_DEF0 64
> > > > -static int vnt_rx_buffers = RX_DESC_DEF0;
> > > > +static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
> > > >  module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
> > > >  MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");
> > > >
> > > >  #define TX_DESC_DEF0 64
> > > > -static int vnt_tx_buffers = TX_DESC_DEF0;
> > > > +static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
> > > >  module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
> > > >  MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");
> > > >
> > >
> > > Why?  What does this help with?
> >
> > If we declare these variables __read_mostly we can improve the performance. If
> > these variables are read many more times than written, each core of a multicore
> > system can maintain a copy in a local cache and the time to access is less than
> > if they use the shared-cache.
>
> This is a USB driver, performance is always limited to the hardware, not
> the CPU location of variables.

Thank you for the explanation.

>
> Please always benchmark things to see if it actually makes sense to make
> changes like this, before proposing them.

I'm sorry.

>
> thanks,
>
> greg k-h

thanks,

Oscar
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
  2020-03-07  8:29         ` Oscar Carter
@ 2020-03-09  9:32           ` Quentin Deslandes
  -1 siblings, 0 replies; 18+ messages in thread
From: Quentin Deslandes @ 2020-03-09  9:32 UTC (permalink / raw)
  To: Oscar Carter
  Cc: Greg Kroah-Hartman, Forest Bond, Malcolm Priestley,
	Gabriela Bittencourt, Colin Ian King, devel, linux-kernel

On Sat, Mar 07, 2020 at 09:29:06AM +0100, Oscar Carter wrote:
> On Sun, Mar 01, 2020 at 04:09:13PM +0100, Greg Kroah-Hartman wrote:
> > On Sun, Mar 01, 2020 at 02:17:01PM +0100, Oscar Carter wrote:
> > This is a USB driver, performance is always limited to the hardware, not
> > the CPU location of variables.
> 
> Thank you for the explanation.
> 
> >
> > Please always benchmark things to see if it actually makes sense to make
> > changes like this, before proposing them.
> 
> I'm sorry.
> 

I've been removed from CC list on Greg's answer, so I haven't seen the
explanation earlier and reviewed the patch the next day. I should have
know better, won't happen again.

Thank you,
Quentin

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
@ 2020-03-09  9:32           ` Quentin Deslandes
  0 siblings, 0 replies; 18+ messages in thread
From: Quentin Deslandes @ 2020-03-09  9:32 UTC (permalink / raw)
  To: Oscar Carter
  Cc: devel, Malcolm Priestley, Greg Kroah-Hartman, linux-kernel,
	Forest Bond, Gabriela Bittencourt, Colin Ian King

On Sat, Mar 07, 2020 at 09:29:06AM +0100, Oscar Carter wrote:
> On Sun, Mar 01, 2020 at 04:09:13PM +0100, Greg Kroah-Hartman wrote:
> > On Sun, Mar 01, 2020 at 02:17:01PM +0100, Oscar Carter wrote:
> > This is a USB driver, performance is always limited to the hardware, not
> > the CPU location of variables.
> 
> Thank you for the explanation.
> 
> >
> > Please always benchmark things to see if it actually makes sense to make
> > changes like this, before proposing them.
> 
> I'm sorry.
> 

I've been removed from CC list on Greg's answer, so I haven't seen the
explanation earlier and reviewed the patch the next day. I should have
know better, won't happen again.

Thank you,
Quentin
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
  2020-03-09  9:32           ` Quentin Deslandes
@ 2020-03-09 10:34             ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2020-03-09 10:34 UTC (permalink / raw)
  To: Quentin Deslandes
  Cc: Oscar Carter, Forest Bond, Malcolm Priestley,
	Gabriela Bittencourt, Colin Ian King, devel, linux-kernel

On Mon, Mar 09, 2020 at 09:32:10AM +0000, Quentin Deslandes wrote:
> On Sat, Mar 07, 2020 at 09:29:06AM +0100, Oscar Carter wrote:
> > On Sun, Mar 01, 2020 at 04:09:13PM +0100, Greg Kroah-Hartman wrote:
> > > On Sun, Mar 01, 2020 at 02:17:01PM +0100, Oscar Carter wrote:
> > > This is a USB driver, performance is always limited to the hardware, not
> > > the CPU location of variables.
> > 
> > Thank you for the explanation.
> > 
> > >
> > > Please always benchmark things to see if it actually makes sense to make
> > > changes like this, before proposing them.
> > 
> > I'm sorry.
> > 
> 
> I've been removed from CC list on Greg's answer, so I haven't seen the
> explanation earlier and reviewed the patch the next day. I should have
> know better, won't happen again.

You weren't on the original list of people on the patch, so I didn't
remove anything here that I can tell.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
@ 2020-03-09 10:34             ` Greg Kroah-Hartman
  0 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2020-03-09 10:34 UTC (permalink / raw)
  To: Quentin Deslandes
  Cc: devel, Oscar Carter, Malcolm Priestley, linux-kernel,
	Forest Bond, Gabriela Bittencourt, Colin Ian King

On Mon, Mar 09, 2020 at 09:32:10AM +0000, Quentin Deslandes wrote:
> On Sat, Mar 07, 2020 at 09:29:06AM +0100, Oscar Carter wrote:
> > On Sun, Mar 01, 2020 at 04:09:13PM +0100, Greg Kroah-Hartman wrote:
> > > On Sun, Mar 01, 2020 at 02:17:01PM +0100, Oscar Carter wrote:
> > > This is a USB driver, performance is always limited to the hardware, not
> > > the CPU location of variables.
> > 
> > Thank you for the explanation.
> > 
> > >
> > > Please always benchmark things to see if it actually makes sense to make
> > > changes like this, before proposing them.
> > 
> > I'm sorry.
> > 
> 
> I've been removed from CC list on Greg's answer, so I haven't seen the
> explanation earlier and reviewed the patch the next day. I should have
> know better, won't happen again.

You weren't on the original list of people on the patch, so I didn't
remove anything here that I can tell.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
  2020-03-09 10:34             ` Greg Kroah-Hartman
@ 2020-03-09 11:09               ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2020-03-09 11:09 UTC (permalink / raw)
  To: Quentin Deslandes
  Cc: devel, Oscar Carter, Malcolm Priestley, linux-kernel,
	Forest Bond, Gabriela Bittencourt, Colin Ian King

On Mon, Mar 09, 2020 at 11:34:07AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Mar 09, 2020 at 09:32:10AM +0000, Quentin Deslandes wrote:
> > On Sat, Mar 07, 2020 at 09:29:06AM +0100, Oscar Carter wrote:
> > > On Sun, Mar 01, 2020 at 04:09:13PM +0100, Greg Kroah-Hartman wrote:
> > > > On Sun, Mar 01, 2020 at 02:17:01PM +0100, Oscar Carter wrote:
> > > > This is a USB driver, performance is always limited to the hardware, not
> > > > the CPU location of variables.
> > > 
> > > Thank you for the explanation.
> > > 
> > > >
> > > > Please always benchmark things to see if it actually makes sense to make
> > > > changes like this, before proposing them.
> > > 
> > > I'm sorry.
> > > 
> > 
> > I've been removed from CC list on Greg's answer, so I haven't seen the
> > explanation earlier and reviewed the patch the next day. I should have
> > know better, won't happen again.
> 
> You weren't on the original list of people on the patch, so I didn't
> remove anything here that I can tell.

Turns out you were, on the lkml one, but not the one that went through
the driver-devel list.  odd...


^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly
@ 2020-03-09 11:09               ` Greg Kroah-Hartman
  0 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2020-03-09 11:09 UTC (permalink / raw)
  To: Quentin Deslandes
  Cc: devel, Oscar Carter, Malcolm Priestley, linux-kernel,
	Forest Bond, Gabriela Bittencourt, Colin Ian King

On Mon, Mar 09, 2020 at 11:34:07AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Mar 09, 2020 at 09:32:10AM +0000, Quentin Deslandes wrote:
> > On Sat, Mar 07, 2020 at 09:29:06AM +0100, Oscar Carter wrote:
> > > On Sun, Mar 01, 2020 at 04:09:13PM +0100, Greg Kroah-Hartman wrote:
> > > > On Sun, Mar 01, 2020 at 02:17:01PM +0100, Oscar Carter wrote:
> > > > This is a USB driver, performance is always limited to the hardware, not
> > > > the CPU location of variables.
> > > 
> > > Thank you for the explanation.
> > > 
> > > >
> > > > Please always benchmark things to see if it actually makes sense to make
> > > > changes like this, before proposing them.
> > > 
> > > I'm sorry.
> > > 
> > 
> > I've been removed from CC list on Greg's answer, so I haven't seen the
> > explanation earlier and reviewed the patch the next day. I should have
> > know better, won't happen again.
> 
> You weren't on the original list of people on the patch, so I didn't
> remove anything here that I can tell.

Turns out you were, on the lkml one, but not the one that went through
the driver-devel list.  odd...

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-03-09 12:06 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-01 11:26 [PATCH] staging: vt6656: Declare a few variables as __read_mostly Oscar Carter
2020-03-01 11:26 ` Oscar Carter
2020-03-01 12:25 ` Greg Kroah-Hartman
2020-03-01 12:25   ` Greg Kroah-Hartman
2020-03-01 13:17   ` Oscar Carter
2020-03-01 13:17     ` Oscar Carter
2020-03-01 15:09     ` Greg Kroah-Hartman
2020-03-01 15:09       ` Greg Kroah-Hartman
2020-03-07  8:29       ` Oscar Carter
2020-03-07  8:29         ` Oscar Carter
2020-03-09  9:32         ` Quentin Deslandes
2020-03-09  9:32           ` Quentin Deslandes
2020-03-09 10:34           ` Greg Kroah-Hartman
2020-03-09 10:34             ` Greg Kroah-Hartman
2020-03-09 11:09             ` Greg Kroah-Hartman
2020-03-09 11:09               ` Greg Kroah-Hartman
2020-03-02 15:43 ` Quentin Deslandes
2020-03-02 15:43   ` Quentin Deslandes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.