* [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 2020-03-02 15:43 ` Quentin Deslandes 0 siblings, 2 replies; 9+ 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] 9+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly 2020-03-01 11:26 [PATCH] staging: vt6656: Declare a few variables as __read_mostly Oscar Carter @ 2020-03-01 12:25 ` Greg Kroah-Hartman 2020-03-01 13:17 ` Oscar Carter 2020-03-02 15:43 ` Quentin Deslandes 1 sibling, 1 reply; 9+ 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] 9+ 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 2020-03-01 15:09 ` Greg Kroah-Hartman 0 siblings, 1 reply; 9+ 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] 9+ 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 2020-03-07 8:29 ` Oscar Carter 0 siblings, 1 reply; 9+ 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] 9+ 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 2020-03-09 9:32 ` Quentin Deslandes 0 siblings, 1 reply; 9+ 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] 9+ 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 2020-03-09 10:34 ` Greg Kroah-Hartman 0 siblings, 1 reply; 9+ 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] 9+ 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 2020-03-09 11:09 ` Greg Kroah-Hartman 0 siblings, 1 reply; 9+ 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] 9+ 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 0 siblings, 0 replies; 9+ 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] 9+ messages in thread
* Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly 2020-03-01 11:26 [PATCH] staging: vt6656: Declare a few variables as __read_mostly Oscar Carter 2020-03-01 12:25 ` Greg Kroah-Hartman @ 2020-03-02 15:43 ` Quentin Deslandes 1 sibling, 0 replies; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2020-03-09 11:09 UTC | newest] Thread overview: 9+ 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 12:25 ` Greg Kroah-Hartman 2020-03-01 13:17 ` Oscar Carter 2020-03-01 15:09 ` Greg Kroah-Hartman 2020-03-07 8:29 ` Oscar Carter 2020-03-09 9:32 ` Quentin Deslandes 2020-03-09 10:34 ` Greg Kroah-Hartman 2020-03-09 11:09 ` Greg Kroah-Hartman 2020-03-02 15:43 ` Quentin Deslandes
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox