2021-04-08 01:11:50

by Yu Kuai

[permalink] [raw]
Subject: [PATCH 0/2] code optimizations for btext.c

Yu Kuai (2):
powerpc: remove set but not used variable 'force_printk_to_btext'
powerpc: make 'boot_text_mapped' static

arch/powerpc/kernel/btext.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

--
2.25.4


2021-04-08 01:13:26

by Yu Kuai

[permalink] [raw]
Subject: [PATCH 2/2] powerpc: make 'boot_text_mapped' static

The sparse tool complains as follow:

arch/powerpc/kernel/btext.c:48:5: warning:
symbol 'boot_text_mapped' was not declared. Should it be static?

This symbol is not used outside of btext.c, so this commit make
it static.

Signed-off-by: Yu Kuai <[email protected]>
---
arch/powerpc/kernel/btext.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
index 359d0f4ca532..8df9230be6fa 100644
--- a/arch/powerpc/kernel/btext.c
+++ b/arch/powerpc/kernel/btext.c
@@ -45,7 +45,7 @@ unsigned long disp_BAT[2] __initdata = {0, 0};

static unsigned char vga_font[cmapsz];

-int boot_text_mapped __force_data = 0;
+static int boot_text_mapped __force_data;

extern void rmci_on(void);
extern void rmci_off(void);
--
2.25.4

2021-04-08 01:14:48

by Yu Kuai

[permalink] [raw]
Subject: [PATCH 1/2] powerpc: remove set but not used variable 'force_printk_to_btext'

Fixes gcc '-Wunused-but-set-variable' warning:

arch/powerpc/kernel/btext.c:49:12: error: 'force_printk_to_btext'
defined but not used.

It is never used, and so can be removed.

Signed-off-by: Yu Kuai <[email protected]>
---
arch/powerpc/kernel/btext.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
index 803c2a45b22a..359d0f4ca532 100644
--- a/arch/powerpc/kernel/btext.c
+++ b/arch/powerpc/kernel/btext.c
@@ -46,7 +46,6 @@ unsigned long disp_BAT[2] __initdata = {0, 0};
static unsigned char vga_font[cmapsz];

int boot_text_mapped __force_data = 0;
-int force_printk_to_btext = 0;

extern void rmci_on(void);
extern void rmci_off(void);
--
2.25.4

2021-04-08 05:03:32

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 1/2] powerpc: remove set but not used variable 'force_printk_to_btext'



Le 08/04/2021 à 03:18, Yu Kuai a écrit :
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> arch/powerpc/kernel/btext.c:49:12: error: 'force_printk_to_btext'
> defined but not used.

You don't get this error as it is now.
You will get this error only if you make it 'static', which is what you did in your first patch
based on the 'sparse' report.

When removing a non static variable, you should explain that you can remove it after you have
verifier that it is nowhere used, neither in that file nor in any other one.

>
> It is never used, and so can be removed.
>
> Signed-off-by: Yu Kuai <[email protected]>
> ---
> arch/powerpc/kernel/btext.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
> index 803c2a45b22a..359d0f4ca532 100644
> --- a/arch/powerpc/kernel/btext.c
> +++ b/arch/powerpc/kernel/btext.c
> @@ -46,7 +46,6 @@ unsigned long disp_BAT[2] __initdata = {0, 0};
> static unsigned char vga_font[cmapsz];
>
> int boot_text_mapped __force_data = 0;
> -int force_printk_to_btext = 0;
>
> extern void rmci_on(void);
> extern void rmci_off(void);
>

2021-04-08 05:06:16

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 2/2] powerpc: make 'boot_text_mapped' static



Le 08/04/2021 à 03:18, Yu Kuai a écrit :
> The sparse tool complains as follow:
>
> arch/powerpc/kernel/btext.c:48:5: warning:
> symbol 'boot_text_mapped' was not declared. Should it be static?
>
> This symbol is not used outside of btext.c, so this commit make
> it static.
>
> Signed-off-by: Yu Kuai <[email protected]>
> ---
> arch/powerpc/kernel/btext.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
> index 359d0f4ca532..8df9230be6fa 100644
> --- a/arch/powerpc/kernel/btext.c
> +++ b/arch/powerpc/kernel/btext.c
> @@ -45,7 +45,7 @@ unsigned long disp_BAT[2] __initdata = {0, 0};
>
> static unsigned char vga_font[cmapsz];
>
> -int boot_text_mapped __force_data = 0;
> +static int boot_text_mapped __force_data;

Are you sure the initialisation to 0 can be removed ? Usually initialisation to 0 is not needed
because not initialised variables go in the BSS section which is zeroed at startup. But here the
variable is flagged with __force_data so it is not going in the BSS section.


>
> extern void rmci_on(void);
> extern void rmci_off(void);
>

2021-04-08 09:32:19

by Yu Kuai

[permalink] [raw]
Subject: Re: [PATCH 1/2] powerpc: remove set but not used variable 'force_printk_to_btext'

On 2021/04/08 13:01, Christophe Leroy wrote:
>
>
> Le 08/04/2021 à 03:18, Yu Kuai a écrit :
>> Fixes gcc '-Wunused-but-set-variable' warning:
>>
>> arch/powerpc/kernel/btext.c:49:12: error: 'force_printk_to_btext'
>> defined but not used.
>
> You don't get this error as it is now.
> You will get this error only if you make it 'static', which is what you
> did in your first patch based on the 'sparse' report.
>
> When removing a non static variable, you should explain that you can
> remove it after you have verifier that it is nowhere used, neither in
> that file nor in any other one.

Hi,

I do use 'git grep force_printk_to_btext' to confirm that
'force_printk_to_btext' is not used anywhere. Maybe it's better to
metion it in commit message?

Thanks
Yu Kuai
>
>>
>> It is never used, and so can be removed.
>>
>> Signed-off-by: Yu Kuai <[email protected]>
>> ---
>>   arch/powerpc/kernel/btext.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
>> index 803c2a45b22a..359d0f4ca532 100644
>> --- a/arch/powerpc/kernel/btext.c
>> +++ b/arch/powerpc/kernel/btext.c
>> @@ -46,7 +46,6 @@ unsigned long disp_BAT[2] __initdata = {0, 0};
>>   static unsigned char vga_font[cmapsz];
>>   int boot_text_mapped __force_data = 0;
>> -int force_printk_to_btext = 0;
>>   extern void rmci_on(void);
>>   extern void rmci_off(void);
>>
> .
>

2021-04-08 09:35:50

by Yu Kuai

[permalink] [raw]
Subject: Re: [PATCH 2/2] powerpc: make 'boot_text_mapped' static

On 2021/04/08 13:04, Christophe Leroy wrote:
>
>
> Le 08/04/2021 à 03:18, Yu Kuai a écrit :
>> The sparse tool complains as follow:
>>
>> arch/powerpc/kernel/btext.c:48:5: warning:
>>   symbol 'boot_text_mapped' was not declared. Should it be static?
>>
>> This symbol is not used outside of btext.c, so this commit make
>> it static.
>>
>> Signed-off-by: Yu Kuai <[email protected]>
>> ---
>>   arch/powerpc/kernel/btext.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
>> index 359d0f4ca532..8df9230be6fa 100644
>> --- a/arch/powerpc/kernel/btext.c
>> +++ b/arch/powerpc/kernel/btext.c
>> @@ -45,7 +45,7 @@ unsigned long disp_BAT[2] __initdata = {0, 0};
>>   static unsigned char vga_font[cmapsz];
>> -int boot_text_mapped __force_data = 0;
>> +static int boot_text_mapped __force_data;
>
> Are you sure the initialisation to 0 can be removed ? Usually
> initialisation to 0 is not needed because not initialised variables go
> in the BSS section which is zeroed at startup. But here the variable is
> flagged with __force_data so it is not going in the BSS section.

Hi,

I removed the initialisation to 0 because checkpatch complained about
it, I do not familiar with '__force_data', thanks for pointing it out.

Thanks,
Yu Kuai
>
>
>>   extern void rmci_on(void);
>>   extern void rmci_off(void);
>>
> .
>

2021-04-09 12:06:55

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH 2/2] powerpc: make 'boot_text_mapped' static

Hi!

On Thu, Apr 08, 2021 at 07:04:35AM +0200, Christophe Leroy wrote:
> Le 08/04/2021 ? 03:18, Yu Kuai a ?crit?:
> >-int boot_text_mapped __force_data = 0;
> >+static int boot_text_mapped __force_data;
>
> Are you sure the initialisation to 0 can be removed ? Usually
> initialisation to 0 is not needed because not initialised variables go in
> the BSS section which is zeroed at startup. But here the variable is
> flagged with __force_data so it is not going in the BSS section.

Any non-automatic (i.e. function-scope, not static) variable is
initialised to 0. See e.g. C11 6.7.9/10 (this has been like that since
times immemorial, C90 anyway).


Segher