[PATCH] fix for_each_cpu() again for NR_CPUS=1

* [PATCH] fix for_each_cpu() again for NR_CPUS=1
@ 2021-03-31 14:52 Jan Beulich
  2021-03-31 16:55 ` Dario Faggioli
  2021-04-01  9:00 ` Revert NR_CPUS=1 fix from 4.15 (was: Re: [PATCH] fix for_each_cpu() again for NR_CPUS=1) Roger Pau Monné
  0 siblings, 2 replies; 8+ messages in thread
From: Jan Beulich @ 2021-03-31 14:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, George Dunlap, Ian Jackson, Julien Grall,
	Stefano Stabellini, Wei Liu, Dario Faggioli

Unfortunately aa50f45332f1 ("xen: fix for_each_cpu when NR_CPUS=1") has
caused quite a bit of fallout with gcc10, e.g. (there are at least two
more similar ones, and I didn't bother trying to find them all):

In file included from .../xen/include/xen/config.h:13,
                 from <command-line>:
core_parking.c: In function ‘core_parking_power’:
.../xen/include/asm/percpu.h:12:51: error: array subscript 1 is above array bounds of ‘long unsigned int[1]’ [-Werror=array-bounds]
   12 |     (*RELOC_HIDE(&per_cpu__##var, __per_cpu_offset[cpu]))
.../xen/include/xen/compiler.h:141:29: note: in definition of macro ‘RELOC_HIDE’
  141 |     (typeof(ptr)) (__ptr + (off)); })
      |                             ^~~
core_parking.c:133:39: note: in expansion of macro ‘per_cpu’
  133 |             core_tmp = cpumask_weight(per_cpu(cpu_core_mask, cpu));
      |                                       ^~~~~~~
In file included from .../xen/include/xen/percpu.h:4,
                 from .../xen/include/asm/msr.h:7,
                 from .../xen/include/asm/time.h:5,
                 from .../xen/include/xen/time.h:76,
                 from .../xen/include/xen/spinlock.h:4,
                 from .../xen/include/xen/cpu.h:5,
                 from core_parking.c:19:
.../xen/include/asm/percpu.h:6:22: note: while referencing ‘__per_cpu_offset’
    6 | extern unsigned long __per_cpu_offset[NR_CPUS];
      |                      ^~~~~~~~~~~~~~~~

One of the further errors even went as far as claiming that an array
index (range) of [0, 0] was outside the bounds of a [1] array, so
something fishy is pretty clearly going on there.

The compiler apparently wants to be able to see that the loop isn't
really a loop in order to avoid triggering such warnings, yet what
exactly makes it consider the loop exit condition constant and within
the [0, 1] range isn't obvious - using ((mask)->bits[0] & 1) instead of
cpumask_test_cpu() for example did _not_ help.

Re-instate a special form of for_each_cpu(), experimentally "proven" to
avoid the diagnostics.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/xen/cpumask.h
+++ b/xen/include/xen/cpumask.h
@@ -368,10 +368,15 @@ static inline void free_cpumask_var(cpum
 #define FREE_CPUMASK_VAR(m) free_cpumask_var(m)
 #endif
 
+#if NR_CPUS > 1
 #define for_each_cpu(cpu, mask)			\
 	for ((cpu) = cpumask_first(mask);	\
 	     (cpu) < nr_cpu_ids;		\
 	     (cpu) = cpumask_next(cpu, mask))
+#else /* NR_CPUS == 1 */
+#define for_each_cpu(cpu, mask)			\
+	for ((cpu) = 0; (cpu) < cpumask_test_cpu(0, mask); ++(cpu))
+#endif /* NR_CPUS */
 
 /*
  * The following particular system cpumasks and operations manage


^ permalink raw reply	[flat|nested] 8+ messages in thread