diff mbox series

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

Message ID 20230809131024.2039647-1-arnd@kernel.org
State New
Headers
Series [1/2] powerpc: mark more local variables as volatile |

Commit Message

Arnd Bergmann Aug. 9, 2023, 1:10 p.m. UTC 
  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(-)

Comments

Christophe Leroy Aug. 9, 2023, 1:17 p.m. UTC | #1 
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;
Arnd Bergmann Aug. 9, 2023, 1:35 p.m. UTC | #2 
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
Michael Ellerman Aug. 23, 2023, 11:55 a.m. UTC | #3 
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
diff mbox series

Patch

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;