Re: arm64: kernel/sys.c - silence initialization warnings.

2021-03-15 Thread Valdis Klētnieks
On Mon, 15 Mar 2021 19:23:00 -, Christoph Hellwig said:
> On Mon, Mar 15, 2021 at 11:14:34AM +, Catalin Marinas wrote:
> > We do similar initialisation in arch/arm64/kernel/sys32.c and
> > arch/arm64/kernel/traps.c for example. It's a pretty common pattern
> > throughout the kernel.
> > 
> > So we either treat W=1 output as diff against the vanilla kernel when
> > checking new patches or we remove override-init altogether from W=1.
> > Mark Rutland pointed me to an older thread:
>
> Please just remove the override-init warning, it is not helpful at all.

The tl;dr: Christoph is *probably* correct that it's not flagging any actual
bugs. And since *my* interest is "get the kernel tree to a point where W=1
or sparse throwing a warning is something worth looking at",  I'm not opposed
to a patch to remove it from W=1 tree-wide if it has essentially zero chance of
flagging an actual bug.

The longer version:

So I did a quick analysis...

For an x86_64 allmodconfig, there's not that many left:

 16 drivers/ata/ahci.h
  1 drivers/ata/pata_atiixp.c
  1 drivers/ata/pata_cs5520.c
  1 drivers/ata/pata_cs5530.c
  1 drivers/ata/pata_sc1200.c
  1 drivers/ata/pata_serverworks.c
  1 drivers/ata/sata_mv.c
  4 drivers/ata/sata_nv.c
  1 drivers/ata/sata_sil24.c
  1 drivers/block/drbd/drbd_main.c
  6 drivers/gpu/drm/amd/amdgpu/../include/asic_reg/dce/dce_6_0_d.h
  2 drivers/gpu/drm/amd/amdgpu/../include/asic_reg/dce/dce_6_0_sh_mask.h
  1 drivers/input/serio/i8042-x86ia64io.h
  1 include/linux/blkdev.h
  1 kernel/bpf/btf.c
  4 kernel/time/hrtimer.c
  1 lib/errname.c

The drivers/ata *.c warnings all appear to be the same type of thing:

static struct scsi_host_template serverworks_osb4_sht = {
ATA_BMDMA_SHT(DRV_NAME),
.sg_tablesize   = LIBATA_DUMB_MAX_PRD,
};

The preprocessor macro defining the struct contents, and then
overriding one predefined value. So that's half of x64_64 done right there.

There's a few corners still need looking at, like why drivers/ata/ahci.h
throws 16 warnings on x64, but 30 on arm and 28 on arm64, and why
there's 4 warnings on include/linux/stddef.h on arm64 but not arm or x86.

But the number is certainly small enough that it's only a day or two's work
at most to check every single one.  If I go through the rest of x86 and arm
and they're all legit, I'll send a patch to nuke it kernel-wide rather than 
piecemeal.


Re: arm64: kernel/sys.c - silence initialization warnings.

2021-03-15 Thread Christoph Hellwig
On Mon, Mar 15, 2021 at 11:14:34AM +, Catalin Marinas wrote:
> We do similar initialisation in arch/arm64/kernel/sys32.c and
> arch/arm64/kernel/traps.c for example. It's a pretty common pattern
> throughout the kernel.
> 
> So we either treat W=1 output as diff against the vanilla kernel when
> checking new patches or we remove override-init altogether from W=1.
> Mark Rutland pointed me to an older thread:

Please just remove the override-init warning, it is not helpful at all.


Re: arm64: kernel/sys.c - silence initialization warnings.

2021-03-15 Thread Catalin Marinas
On Fri, Mar 12, 2021 at 04:55:46AM -0500, Valdis Klētnieks wrote:
> Building arch/arm64/kernel/sys.o with W=1 throws over 300 warnings:
> 
> /usr/src/linux-next/arch/arm64/kernel/sys.c:56:40: warning: initialized field 
> overwritten [-Woverride-init]
>56 | #define __SYSCALL(nr, sym)  [nr] = __arm64_##sym,
>   |^~~~
> /usr/src/linux-next/include/uapi/asm-generic/unistd.h:29:37: note: in 
> expansion of macro '__SYSCALL'
>29 | #define __SC_COMP(_nr, _sys, _comp) __SYSCALL(_nr, _sys)
>   | ^
> /usr/src/linux-next/include/uapi/asm-generic/unistd.h:34:1: note: in 
> expansion of macro '__SC_COMP'
>34 | __SC_COMP(__NR_io_setup, sys_io_setup, compat_sys_io_setup)
>   | ^
> 
> We know that's pretty much the file's purpose in life, so tell the
> build system to not remind us.  This makes the 1 other warning a
> lot more noticeable. 
> 
> Signed-off-by: Valdis Kletnieks 
> 
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index ed65576ce710..916b21d2b35b 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -8,6 +8,7 @@ CFLAGS_armv8_deprecated.o := -I$(src)
>  CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_insn.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_sys.o += $(call cc-disable-warning, override-init)

We do similar initialisation in arch/arm64/kernel/sys32.c and
arch/arm64/kernel/traps.c for example. It's a pretty common pattern
throughout the kernel.

So we either treat W=1 output as diff against the vanilla kernel when
checking new patches or we remove override-init altogether from W=1.
Mark Rutland pointed me to an older thread:

https://lore.kernel.org/linux-arm-kernel/20190809083251.ga48...@lakrids.cambridge.arm.com/

-- 
Catalin


arm64: kernel/sys.c - silence initialization warnings.

2021-03-12 Thread Valdis Klētnieks
Building arch/arm64/kernel/sys.o with W=1 throws over 300 warnings:

/usr/src/linux-next/arch/arm64/kernel/sys.c:56:40: warning: initialized field 
overwritten [-Woverride-init]
   56 | #define __SYSCALL(nr, sym)  [nr] = __arm64_##sym,
  |^~~~
/usr/src/linux-next/include/uapi/asm-generic/unistd.h:29:37: note: in expansion 
of macro '__SYSCALL'
   29 | #define __SC_COMP(_nr, _sys, _comp) __SYSCALL(_nr, _sys)
  | ^
/usr/src/linux-next/include/uapi/asm-generic/unistd.h:34:1: note: in expansion 
of macro '__SC_COMP'
   34 | __SC_COMP(__NR_io_setup, sys_io_setup, compat_sys_io_setup)
  | ^

We know that's pretty much the file's purpose in life, so tell the
build system to not remind us.  This makes the 1 other warning a
lot more noticeable. 

Signed-off-by: Valdis Kletnieks 

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index ed65576ce710..916b21d2b35b 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -8,6 +8,7 @@ CFLAGS_armv8_deprecated.o := -I$(src)
 CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_insn.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE)
+CFLAGS_sys.o += $(call cc-disable-warning, override-init)
 
 # Object file lists.
 obj-y  := debug-monitors.o entry.o irq.o fpsimd.o  
\