Re: [PATCH] libperf: Add back guard on MAX_NR_CPUS

[Ian Rogers]

On Mon, Jan 6, 2025 at 11:38 AM Christophe Leroy
<christophe.leroy@xxxxxxxxxx> wrote:
>
> Building perf with EXTRA_CFLAGS="-DMAX_NR_CPUS=1" fails:
>
>           CC      /home/chleroy/linux-powerpc/tools/perf/libperf/cpumap.o
>         cpumap.c:16: error: "MAX_NR_CPUS" redefined [-Werror]
>            16 | #define MAX_NR_CPUS 4096
>               |
>         <command-line>: note: this is the location of the previous definition
>
> Commit e8399d34d568 ("libperf cpumap: Hide/reduce scope of MAX_NR_CPUS")
> moved definition of MAX_NR_CPUS from lib/perf/include/internal/cpumap.h
> to lib/perf/cpumap.c but the guard surrounding that definition got lost
> in the move.
>
> See commit 21b8732eb447 ("perf tools: Allow overriding MAX_NR_CPUS at
> compile time") to see why it is needed.
>
> Note that MAX_NR_CPUS was initialy defined in perf/perf.h and a
> redundant definition was added by commit 9c3516d1b850 ("libperf:
> Add perf_cpu_map__new()/perf_cpu_map__read() functions").
>
> A cleaner fix would be to remove that duplicate but for the time
> being fix the problem by bringing back the guard for when MAX_NR_CPUS
> is already defined.
>
> Fixes: e8399d34d568 ("libperf cpumap: Hide/reduce scope of MAX_NR_CPUS")
> Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
> Reviewed-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>

Hello,

I believe this change might be unnecessary. The only use of
MAX_NR_CPUS is in a warning message within perf_cpu_map__new, which
takes a string and produces a perf_cpu_map. Other similar functions
like cpu_map__new_sysconf don't check MAX_NR_CPUS. Therefore,
specifying a -DMAX_NR_CPUS value on the build command line has little
effect—it only impacts a warning message for certain kinds of
perf_cpu_map creation. It's also unclear what the intended outcome is
on the build command line.

Given that specifying the value doesn't seem to have a clear purpose,
allowing the build to break might be the best option. This would alert
the person building perf that they are doing something that doesn't
make sense.

Thanks,
Ian