[PATCH 1/2] powerpc: mark more local variables as volatile

[PATCH 1/2] powerpc: mark more local variables as volatile

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

A while ago I created a2305e3de8193 ("powerpc: mark local variables
around longjmp as volatile") in order to allow building powerpc with
-Wextra enabled on gcc-11.

I tried this again with gcc-13 and found two more of the same issues,
presumably based on slightly different optimization paths being taken
here:

arch/powerpc/xmon/xmon.c:3306:27: error: variable 'mm' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/kexec/crash.c:353:22: error: variable 'i' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]

I checked a bunch of randconfigs and found only these two, so just
address them the same way as the others.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/powerpc/kexec/crash.c | 2 +-
 arch/powerpc/xmon/xmon.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
index 252724ed666a3..ef5c2d25ec397 100644
--- a/arch/powerpc/kexec/crash.c
+++ b/arch/powerpc/kexec/crash.c
@@ -350,7 +350,7 @@ EXPORT_SYMBOL(crash_shutdown_unregister);
 
 void default_machine_crash_shutdown(struct pt_regs *regs)
 {
-	unsigned int i;
+	volatile unsigned int i;
 	int (*old_handler)(struct pt_regs *regs);
 
 	if (TRAP(regs) == INTERRUPT_SYSTEM_RESET)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 3b6f524c790e3..9e12b75850d75 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3303,7 +3303,7 @@ static void show_pte(unsigned long addr)
 {
 	unsigned long tskv = 0;
 	struct task_struct *volatile tsk = NULL;
-	struct mm_struct *mm;
+	struct mm_struct *volatile mm;
 	pgd_t *pgdp;
 	p4d_t *p4dp;
 	pud_t *pudp;
-- 
2.39.2

[PATCH 2/2] powerpc: xmon: remove unused variables

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Randconfig testing with W=1 showed up these warnings that I'd like to enable
by default:

arch/powerpc/xmon/xmon.c: In function 'dump_tlb_book3e':
arch/powerpc/xmon/xmon.c:3833:42: error: variable 'lrat' set but not used [-Werror=unused-but-set-variable]
 3833 |  int i, tlb, ntlbs, pidsz, lpidsz, rasz, lrat = 0;
      |                                          ^~~~
arch/powerpc/xmon/xmon.c:3831:23: error: variable 'lpidmask' set but not used [-Werror=unused-but-set-variable]
 3831 |  u32 mmucfg, pidmask, lpidmask;
      |                       ^~~~~~~~
arch/powerpc/xmon/xmon.c:3831:14: error: variable 'pidmask' set but not used [-Werror=unused-but-set-variable]
 3831 |  u32 mmucfg, pidmask, lpidmask;
      |              ^~~~~~~

Just remove these as they have been unused since the code was added in 2010.

Fixes: 03247157f7391 ("powerpc/book3e: Add TLB dump in xmon for Book3E")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/powerpc/xmon/xmon.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 9e12b75850d75..e35f13842ce15 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3828,9 +3828,9 @@ static void dump_tlb_44x(void)
 #ifdef CONFIG_PPC_BOOK3E_64
 static void dump_tlb_book3e(void)
 {
-	u32 mmucfg, pidmask, lpidmask;
+	u32 mmucfg;
 	u64 ramask;
-	int i, tlb, ntlbs, pidsz, lpidsz, rasz, lrat = 0;
+	int i, tlb, ntlbs, pidsz, lpidsz, rasz;
 	int mmu_version;
 	static const char *pgsz_names[] = {
 		"  1K",
@@ -3874,12 +3874,8 @@ static void dump_tlb_book3e(void)
 	pidsz = ((mmucfg >> 6) & 0x1f) + 1;
 	lpidsz = (mmucfg >> 24) & 0xf;
 	rasz = (mmucfg >> 16) & 0x7f;
-	if ((mmu_version > 1) && (mmucfg & 0x10000))
-		lrat = 1;
 	printf("Book3E MMU MAV=%d.0,%d TLBs,%d-bit PID,%d-bit LPID,%d-bit RA\n",
 	       mmu_version, ntlbs, pidsz, lpidsz, rasz);
-	pidmask = (1ul << pidsz) - 1;
-	lpidmask = (1ul << lpidsz) - 1;
 	ramask = (1ull << rasz) - 1;
 
 	for (tlb = 0; tlb < ntlbs; tlb++) {
-- 
2.39.2

Re: [PATCH 1/2] powerpc: mark more local variables as volatile

[Christophe Leroy]

Le 09/08/2023 à 15:10, Arnd Bergmann a écrit :
> From: Arnd Bergmann <arnd@arndb.de>
> 
> A while ago I created a2305e3de8193 ("powerpc: mark local variables
> around longjmp as volatile") in order to allow building powerpc with
> -Wextra enabled on gcc-11.

Should this be explained in 
https://docs.kernel.org/process/volatile-considered-harmful.html ?

Christophe


> 
> I tried this again with gcc-13 and found two more of the same issues,
> presumably based on slightly different optimization paths being taken
> here:
> 
> arch/powerpc/xmon/xmon.c:3306:27: error: variable 'mm' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/kexec/crash.c:353:22: error: variable 'i' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> 
> I checked a bunch of randconfigs and found only these two, so just
> address them the same way as the others.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   arch/powerpc/kexec/crash.c | 2 +-
>   arch/powerpc/xmon/xmon.c   | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
> index 252724ed666a3..ef5c2d25ec397 100644
> --- a/arch/powerpc/kexec/crash.c
> +++ b/arch/powerpc/kexec/crash.c
> @@ -350,7 +350,7 @@ EXPORT_SYMBOL(crash_shutdown_unregister);
>   
>   void default_machine_crash_shutdown(struct pt_regs *regs)
>   {
> -	unsigned int i;
> +	volatile unsigned int i;
>   	int (*old_handler)(struct pt_regs *regs);
>   
>   	if (TRAP(regs) == INTERRUPT_SYSTEM_RESET)
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 3b6f524c790e3..9e12b75850d75 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -3303,7 +3303,7 @@ static void show_pte(unsigned long addr)
>   {
>   	unsigned long tskv = 0;
>   	struct task_struct *volatile tsk = NULL;
> -	struct mm_struct *mm;
> +	struct mm_struct *volatile mm;
>   	pgd_t *pgdp;
>   	p4d_t *p4dp;
>   	pud_t *pudp;

Re: [PATCH 1/2] powerpc: mark more local variables as volatile

[Arnd Bergmann]

On Wed, Aug 9, 2023, at 15:17, Christophe Leroy wrote:
> Le 09/08/2023 à 15:10, Arnd Bergmann a écrit :
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> A while ago I created a2305e3de8193 ("powerpc: mark local variables
>> around longjmp as volatile") in order to allow building powerpc with
>> -Wextra enabled on gcc-11.
>
> Should this be explained in 
> https://docs.kernel.org/process/volatile-considered-harmful.html ?
>

My feeling is that these two files are special enough that we
don't have to worry about it in general, there is only one other
caller of setjmp in the kernel, and the setjmp() man page
explicitly mentions this problem and the workaround.

     Arnd

Re: [PATCH 1/2] powerpc: mark more local variables as volatile

[Michael Ellerman]

On Wed, 09 Aug 2023 15:10:08 +0200, Arnd Bergmann wrote:
> A while ago I created a2305e3de8193 ("powerpc: mark local variables
> around longjmp as volatile") in order to allow building powerpc with
> -Wextra enabled on gcc-11.
> 
> I tried this again with gcc-13 and found two more of the same issues,
> presumably based on slightly different optimization paths being taken
> here:
> 
> [...]

Applied to powerpc/next.

[1/2] powerpc: mark more local variables as volatile
      https://git.kernel.org/powerpc/c/0f7ce21ab5209426b00636636a5f2d9008738654
[2/2] powerpc: xmon: remove unused variables
      https://git.kernel.org/powerpc/c/ef73dcaa31217c79adc548bf9045afb40ac75b82

cheers