[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

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 <redacted>
---
 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

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.

On 2021/04/08 13:01, Christophe Leroy wrote:

Le 08/04/2021 à 03:18, Yu Kuai a écrit :

quoted

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

quoted

It is never used, and so can be removed.

Signed-off-by: Yu Kuai <redacted>
---
  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);

.

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 <redacted>
---
 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

Le 08/04/2021 à 03:18, Yu Kuai a écrit :

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);

On 2021/04/08 13:04, Christophe Leroy wrote:

Le 08/04/2021 à 03:18, Yu Kuai a écrit :

quoted

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 <redacted>
---
  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

quoted

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

.

Hi!

On Thu, Apr 08, 2021 at 07:04:35AM +0200, Christophe Leroy wrote:

Le 08/04/2021 à 03:18, Yu Kuai a écrit :

quoted

-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