Re: stable-rc build: 72 warnings 1 failures (stable-rc/v4.4.30-35-gf821e08)

[Arnd Bergmann <arnd@arndb.de>]

On Tuesday, November 8, 2016 6:45:53 PM CET Eric W. Biederman wrote:
> Arnd Bergmann <arnd@arndb.de> writes:

> > On Tuesday, November 8, 2016 9:16:28 AM CET Olof's autobuilder wrote:
> >> Here are the build results from automated periodic testing.
> > I think this was accidentally fixed by eedf265aa003 ("devpts: Make each mount of
> > devpts an independent filesystem."), which unfortunately is not a
> > candidate for stable
> 
> Well eedf265aa003 ("devpts: Make each mount of devpts an independent
> filesystem.") does contain a somewhat serious bug fix, and it was tested
> to ensure it works everywhere so that might possibly be a canidate for
> stable.
> 
> Certainly that is a change I would aim at vendor trees that care about
> containers.
> 
> >>       1 net/netfilter/xt_owner.c:27:23: warning: self-comparison always evaluates to false [-Wtautological-compare]
> >
> > Apparently also fixed as a side-effect of a larger patch:
> >
> > 9847371a84b0 ("netfilter: Allow xt_owner in any user namespace")
> >
> > This one might be appropriate for a stable backport, Eric Biederman
> > would know for sure.
> 
> Well it is a feature patch.  This sounds like an error message that is
> only generated when user namespace support is disabled.  And we are
> making it go away by making the code more expensive.

Ah, so both of these were exactly the opposite of what I expected ;-)

> I am not a great fan of that warning being on by default, as it seems to
> encourage more expensive code to be generated by macros.   Has that
> warning caught any real bugs yet?

I fixed up all instanced I could find on ARM, see "git log --oneline
--grep=tautological". I'd categorize three of them as bugs:

0a938697d7fb drm/exynos: fix error handling in exynos_drm_subdrv_open
0fb504001192 [media] am437x-vfpe: fix typo in vpfe_get_app_input_index
86d65b7e7a0c nouveau: fix nv40_perfctr_next() cleanup regression

and four others as false-positive:

dd665be0e243 ARM: 8584/1: floppy: avoid gcc-6 warning
f419a08fb329 drivers/memstick/host/r592.c: avoid gcc-6 warning
e3ebd894f084 smc91x: avoid self-comparison warning
0335695dfa4d cred/userns: define current_user_ns() as a function

which is not a bad ratio at all. The last of those is probably
sufficient to address most of the gcc-6 warnings on stable-4.4.

I also did a patch to shut up four more warnings on x86, but never
got around to submitting them properly. These are all false-positive,
see patch below (I folded this into one).

	Arnd

commit 28624e5166a2d33c386b6d5b7f627d84939cabce
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Fri Oct 7 14:15:06 2016 +0200

    x86: treewide: hide -Wtautological-compare warnings
    
    proc/kcore: hide a harmless warning
    
    fs/proc/kcore.c: In function ‘add_modules_range’:
    fs/proc/kcore.c:622:161: error: self-comparison always evaluates to false [-Werror=tautological-compare]
      if (/*MODULES_VADDR != VMALLOC_START && */MODULES_END != VMALLOC_END) {
    
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>
    
    acpi: thermal: fix gcc-6/ccache warning
    
    drivers/acpi/processor_thermal.c: In function ‘cpufreq_set_cur_state’:
    drivers/acpi/processor_thermal.c:137:36: error: self-comparison always evaluates to true [-Werror=tautological-compare]
    
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>
    
    x86: microcode/amd: avoid ccache/gcc-6 warning
    
    /git/arm-soc/arch/x86/kernel/cpu/microcode/amd.c: In function ‘load_microcode_amd’:
    /git/arm-soc/arch/x86/kernel/cpu/microcode/amd.c:878:30: error: self-comparison always evaluates to true [-Werror=tautological-compare]
    
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>
    
    sfi: avoid ccache warning
    
    With ccache in combination with gcc-6, we get a harmless warning for the sfi subsystem,
    as ccache only sees the preprocessed source:
    
    drivers/sfi/sfi_core.c: In function ‘sfi_map_table’:
    drivers/sfi/sfi_core.c:175:53: error: self-comparison always evaluates to true [-Werror=tautological-compare]
    
    Using an inline function to do the comparison tells the compiler what is
    going on even for preprocessed files, and avoids the warning.
    
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 6f353bdb3a25..64a457da4a4c 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -854,6 +854,7 @@ static enum ucode_state
 load_microcode_amd(int cpu, u8 family, const u8 *data, size_t size)
 {
 	enum ucode_state ret;
+	int bootcpu = boot_cpu_data.cpu_index;
 
 	/* free old equiv table */
 	free_equiv_cpu_table();
@@ -865,7 +866,7 @@ load_microcode_amd(int cpu, u8 family, const u8 *data, size_t size)
 
 #ifdef CONFIG_X86_32
 	/* save BSP's matching patch for early load */
-	if (cpu_data(cpu).cpu_index == boot_cpu_data.cpu_index) {
+	if (cpu_data(cpu).cpu_index == bootcpu) {
 		struct ucode_patch *p = find_patch(cpu);
 		if (p) {
 			memset(amd_ucode_patch, 0, PATCH_MAX_SIZE);
diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index 1fed84a092c2..8cde6715537b 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -122,20 +122,22 @@ static int cpufreq_get_cur_state(unsigned int cpu)
 static int cpufreq_set_cur_state(unsigned int cpu, int state)
 {
 	int i;
+	int id;
 
 	if (!cpu_has_cpufreq(cpu))
 		return 0;
 
 	reduction_pctg(cpu) = state;
 
+	id = topology_physical_package_id(cpu);
+
 	/*
 	 * Update all the CPUs in the same package because they all
 	 * contribute to the temperature and often share the same
 	 * frequency.
 	 */
 	for_each_online_cpu(i) {
-		if (topology_physical_package_id(i) ==
-		    topology_physical_package_id(cpu))
+		if (topology_physical_package_id(i) == id)
 			cpufreq_update_policy(i);
 	}
 	return 0;
diff --git a/drivers/sfi/sfi_core.c b/drivers/sfi/sfi_core.c
index 296db7a69c27..a8f2313a2613 100644
--- a/drivers/sfi/sfi_core.c
+++ b/drivers/sfi/sfi_core.c
@@ -71,9 +71,12 @@
 
 #include "sfi_core.h"
 
-#define ON_SAME_PAGE(addr1, addr2) \
-	(((unsigned long)(addr1) & PAGE_MASK) == \
-	((unsigned long)(addr2) & PAGE_MASK))
+static inline bool on_same_page(unsigned long addr1, unsigned long addr2)
+{
+	return (addr1 & PAGE_MASK) == (addr2 & PAGE_MASK);
+}
+
+#define ON_SAME_PAGE(addr1, addr2) on_same_page((unsigned long)addr1, (unsigned long)addr2)
 #define TABLE_ON_PAGE(page, table, size) (ON_SAME_PAGE(page, table) && \
 				ON_SAME_PAGE(page, table + size))
 
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index 5c89a07e3d7f..8ca8de0d13eb 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -616,12 +616,14 @@ static void __init proc_kcore_text_init(void)
 /*
  * MODULES_VADDR has no intersection with VMALLOC_ADDR.
  */
-struct kcore_list kcore_modules;
+static struct kcore_list kcore_modules;
 static void __init add_modules_range(void)
 {
-	if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
-		kclist_add(&kcore_modules, (void *)MODULES_VADDR,
-			MODULES_END - MODULES_VADDR, KCORE_VMALLOC);
+	void *start = (void *)MODULES_VADDR;
+	size_t len = MODULES_END - MODULES_VADDR;
+
+	if (start != (void *)VMALLOC_START && len != VMALLOC_END - VMALLOC_START) {
+		kclist_add(&kcore_modules, start, len, KCORE_VMALLOC);
 	}
 }
 #else