linux-kernel - Re: [PATCH] leds: lp55xx: make various arrays static const

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 29 Jun 2017 21:40:58 +0200
From:   Jacek Anaszewski <jacek.anaszewski@...il.com>
To:     Colin King <colin.king@...onical.com>,
        Richard Purdie <rpurdie@...ys.net>,
        Pavel Machek <pavel@....cz>, linux-leds@...r.kernel.org
Cc:     kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] leds: lp55xx: make various arrays static const

Hi Colin,

Thanks for the patch.

Do you have some profiling results showing the benefit of these changes?
It seems that these functions are called only on driver initialization.
Making the local arrays static will prevent releasing this memory even
though it will be no longer needed.

Anyway, the size of arrays is 3 bytes, so the impact of these changes
seems to be virtually negligible. I am not taking this patch unless
you are able to provide the numbers showing the gain.

Best regards,
Jacek Anaszewski

On 06/29/2017 07:57 PM, Colin King wrote:
> From: Colin Ian King <colin.king@...onical.com>
> 
> Several arrays are currently on-stack and instead should be made
> static const.
> 
> Signed-off-by: Colin Ian King <colin.king@...onical.com>
> ---
>  drivers/leds/leds-lp5523.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
> index e9ba8cd32d66..924e50aefb00 100644
> --- a/drivers/leds/leds-lp5523.c
> +++ b/drivers/leds/leds-lp5523.c
> @@ -168,13 +168,13 @@ static int lp5523_post_init_device(struct lp55xx_chip *chip)
>  static void lp5523_load_engine(struct lp55xx_chip *chip)
>  {
>  	enum lp55xx_engine_index idx = chip->engine_idx;
> -	u8 mask[] = {
> +	static const u8 mask[] = {
>  		[LP55XX_ENGINE_1] = LP5523_MODE_ENG1_M,
>  		[LP55XX_ENGINE_2] = LP5523_MODE_ENG2_M,
>  		[LP55XX_ENGINE_3] = LP5523_MODE_ENG3_M,
>  	};
>  
> -	u8 val[] = {
> +	static const u8 val[] = {
>  		[LP55XX_ENGINE_1] = LP5523_LOAD_ENG1,
>  		[LP55XX_ENGINE_2] = LP5523_LOAD_ENG2,
>  		[LP55XX_ENGINE_3] = LP5523_LOAD_ENG3,
> @@ -188,7 +188,7 @@ static void lp5523_load_engine(struct lp55xx_chip *chip)
>  static void lp5523_load_engine_and_select_page(struct lp55xx_chip *chip)
>  {
>  	enum lp55xx_engine_index idx = chip->engine_idx;
> -	u8 page_sel[] = {
> +	static const u8 page_sel[] = {
>  		[LP55XX_ENGINE_1] = LP5523_PAGE_ENG1,
>  		[LP55XX_ENGINE_2] = LP5523_PAGE_ENG2,
>  		[LP55XX_ENGINE_3] = LP5523_PAGE_ENG3,
> @@ -208,7 +208,7 @@ static void lp5523_stop_all_engines(struct lp55xx_chip *chip)
>  static void lp5523_stop_engine(struct lp55xx_chip *chip)
>  {
>  	enum lp55xx_engine_index idx = chip->engine_idx;
> -	u8 mask[] = {
> +	static const u8 mask[] = {
>  		[LP55XX_ENGINE_1] = LP5523_MODE_ENG1_M,
>  		[LP55XX_ENGINE_2] = LP5523_MODE_ENG2_M,
>  		[LP55XX_ENGINE_3] = LP5523_MODE_ENG3_M,
> @@ -505,7 +505,7 @@ static int lp5523_load_mux(struct lp55xx_chip *chip, u16 mux, int nr)
>  {
>  	struct lp55xx_engine *engine = &chip->engines[nr - 1];
>  	int ret;
> -	u8 mux_page[] = {
> +	static const u8 mux_page[] = {
>  		[LP55XX_ENGINE_1] = LP5523_PAGE_MUX1,
>  		[LP55XX_ENGINE_2] = LP5523_PAGE_MUX2,
>  		[LP55XX_ENGINE_3] = LP5523_PAGE_MUX3,
>