On Mon, Aug 05, 2019 at 07:47:37AM -0400, Qian Cai wrote: > > > > On Aug 5, 2019, at 5:52 AM, Will Deacon <[email protected]> wrote: > > > > On Fri, Aug 02, 2019 at 11:32:24AM -0400, Qian Cai wrote: > >> The commit 155433cb365e ("arm64: cache: Remove support for ASID-tagged > >> VIVT I-caches") introduced some compiation warnings from GCC, > >> > >> 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. > >> > >> Fixes: 155433cb365e ("arm64: cache: Remove support for ASID-tagged VIVT > >> I-caches") > >> Signed-off-by: Qian Cai <[email protected]> > >> --- > >> arch/arm64/kernel/cpuinfo.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c > >> index 876055e37352..193b38da8d96 100644 > >> --- a/arch/arm64/kernel/cpuinfo.c > >> +++ b/arch/arm64/kernel/cpuinfo.c > >> @@ -34,10 +34,10 @@ > >> static struct cpuinfo_arm64 boot_cpu_data; > >> > >> static char *icache_policy_str[] = { > >> - [0 ... ICACHE_POLICY_PIPT] = "RESERVED/UNKNOWN", > >> + [ICACHE_POLICY_VPIPT] = "VPIPT", > >> + [ICACHE_POLICY_VPIPT + 1] = "RESERVED/UNKNOWN", > >> [ICACHE_POLICY_VIPT] = "VIPT", > >> [ICACHE_POLICY_PIPT] = "PIPT", > >> - [ICACHE_POLICY_VPIPT] = "VPIPT", > > > > I really don't like this patch. Using "[0 ... MAXIDX] = <default>" is a > > useful idiom and I think the code is more error-prone the way you have > > restructured it. > > > > Why are you passing -Woverride-init to the compiler anyway? There's only > > one Makefile that references that option, and it's specific to a pinctrl > > driver. > > Those extra warnings can be enabled by “make W=1”. “-Woverride-init “ seems > to be useful > to catch potential developer mistakes with unintented double-initializations. > It is normal to > start to fix the most of false-positives first before globally enabling the > flag by default just like > “-Wimplicit-fallthrough” mentioned in, > > https://lwn.net/Articles/794944/
I think this case is completely different to the implicit fallthrough stuff. The solution there was simply to add a comment without restructuring the surrounding code. What your patch does here is actively make the code harder to understand. Initialising a static array with a non-zero pattern is a useful idiom and I don't think we should throw that away just to appease a silly compiler warning that appears only with non-default build options. Have a look at the way we use PERF_MAP_ALL_UNSUPPORTED in the Arm PMU code, for example. Will
