linux-kernel - Re: [PATCH] staging: vt6656: Declare a few variables as __read

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200301150913.GA1470815@kroah.com>
Date:   Sun, 1 Mar 2020 16:09:13 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Oscar Carter <oscar.carter@....com>
Cc:     devel@...verdev.osuosl.org, Malcolm Priestley <tvboxspy@...il.com>,
        linux-kernel@...r.kernel.org,
        Forest Bond <forest@...ttletooquiet.net>,
        Gabriela Bittencourt <gabrielabittencourt00@...il.com>,
        Colin Ian King <colin.king@...onical.com>
Subject: Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly

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@....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