Messages in this thread | | | Subject | Re: [PATCH] arm64/cache: silence -Woverride-init warnings | From | Qian Cai <> | Date | Thu, 8 Aug 2019 18:18:39 -0400 |
| |
> On Aug 8, 2019, at 6:38 AM, Mark Rutland <mark.rutland@arm.com> wrote: > > On Wed, Aug 07, 2019 at 11:29:16PM -0400, Qian Cai wrote: >> The commit 155433cb365e ("arm64: cache: Remove support for ASID-tagged >> VIVT I-caches") introduced some compiation warnings from GCC (and >> Clang) with -Winitializer-overrides), >> >> arch/arm64/kernel/cpuinfo.c:38:26: warning: initialized field >> overwritten [-Woverride-init] >> [ICACHE_POLICY_VIPT] = "VIPT", >> ^~~~~~ >> arch/arm64/kernel/cpuinfo.c:38:26: note: (near initialization for >> 'icache_policy_str[2]') >> arch/arm64/kernel/cpuinfo.c:39:26: warning: initialized field >> overwritten [-Woverride-init] >> [ICACHE_POLICY_PIPT] = "PIPT", >> ^~~~~~ >> arch/arm64/kernel/cpuinfo.c:39:26: note: (near initialization for >> 'icache_policy_str[3]') >> arch/arm64/kernel/cpuinfo.c:40:27: warning: initialized field >> overwritten [-Woverride-init] >> [ICACHE_POLICY_VPIPT] = "VPIPT", >> ^~~~~~~ >> arch/arm64/kernel/cpuinfo.c:40:27: note: (near initialization for >> 'icache_policy_str[0]') >> >> because it initializes icache_policy_str[0 ... 3] twice. Since >> arm64 developers are keen to keep the style of initializing a static >> array with a non-zero pattern first, just disable those warnings for >> both GCC and Clang of this file. >> >> Fixes: 155433cb365e ("arm64: cache: Remove support for ASID-tagged VIVT I-caches") >> Signed-off-by: Qian Cai <cai@lca.pw> > > This is _not_ a fix, and should not require backporting to stable trees.
From my experience, the stable AI will pick up whatever they want to backport not matter if there Is a “Fixes” tag or not unless it is one of those subsystems like Networking that exclusively manually flag for. backporting by the maintainer.
> > What about all the other instances that we have in mainline?
I have not had a chance to review all instances yet. It is not unusually to fix one warning at a time, and then go on fixing some more if time permit.
> > I really don't think that we need to go down this road; we're just going > to end up adding this to every file that happens to include a header > using this scheme…
How about disable them this way in a top level like arch/arm64/Makefile or arch/arm64/kernel/Makefile? Therefore, there is no need to add this to every file, but with a drawback that it could miss a few real issues there in the future which probably not many people are checking for them of the arm64 subsystem nowadays.
> > Please just turn this off by default for clang.
As mentioned before, it is very valuable to run “make W=1” given it found many real developer mistakes which will enable “-Woverride-init” for both compilers. Even “-Woverride-init” itself is useful find real issues as in,
ae5e033d65a (“mfd: rk808: Fix RK818_IRQ_DISCHG_ILIM initializer”) 32df34d875bb (“[media] rc: img-ir: jvc: Remove unused no-leader timings”)
Especially, to find redundant initializations in large structures. e.g.,
e6ea0b917875 (“[media] dvb_frontend: Don't declare values twice at a table”)
It is important to keep the noise-level as low as possible by keeping the amount of false positives under control to be truly benefit from those valuable compiler warnings.
> > If we want to enable this, we need a mechanism to permit overridable > assignments as we use range initializers for.
I am not sure that it is worth filling a RFE for compilers of that feature. I feel like the range initializers just another way to initialize an array, and it is just easier to make mistakes with unintended double-initializations. The compiler developers probably recommend to enforce more of “-Woverride-init” for the range initializers rather than permitting it.
> > Thanks, > Mark. > >> --- >> arch/arm64/kernel/Makefile | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile >> index 478491f07b4f..397ed5f7be1e 100644 >> --- a/arch/arm64/kernel/Makefile >> +++ b/arch/arm64/kernel/Makefile >> @@ -11,6 +11,9 @@ CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE) >> CFLAGS_REMOVE_insn.o = $(CC_FLAGS_FTRACE) >> CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE) >> >> +CFLAGS_cpuinfo.o += $(call cc-disable-warning, override-init) >> +CFLAGS_cpuinfo.o += $(call cc-disable-warning, initializer-overrides) >> + >> # Object file lists. >> obj-y := debug-monitors.o entry.o irq.o fpsimd.o \ >> entry-fpsimd.o process.o ptrace.o setup.o signal.o \ >> -- >> 2.20.1 (Apple Git-117)
| |