[PATCH 0/2] code optimizations for btext.c

[PATCH 0/2] code optimizations for btext.c

[Yu Kuai]

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

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

[Yu Kuai]

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 <yukuai3@huawei.com>
---
 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

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

[Christophe Leroy]

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 <yukuai3@huawei.com>
> ---
>   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);
> 

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

[yukuai (C)]

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 <yukuai3@huawei.com>
>> ---
>>   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);
>>
> .
> 

[PATCH 2/2] powerpc: make 'boot_text_mapped' static

[Yu Kuai]

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 <yukuai3@huawei.com>
---
 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

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

[Christophe Leroy]

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 <yukuai3@huawei.com>
> ---
>   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);
> 

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

[yukuai (C)]

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 <yukuai3@huawei.com>
>> ---
>>   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);
>>
> .
> 

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

[Segher Boessenkool]

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