On Mon, May 18, 2020 at 9:11 AM Arnaldo Carvalho de Melo <arnaldo.melo@xxxxxxxxx> wrote: > > Em Mon, May 18, 2020 at 01:06:48PM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Mon, May 18, 2020 at 09:03:45AM -0700, Ian Rogers escreveu: > > > On Mon, May 18, 2020 at 8:45 AM Arnaldo Carvalho de Melo wrote: > > > this build issue sounds like this patch is missing: > > > https://lore.kernel.org/lkml/20200515221732.44078-3-irogers@xxxxxxxxxx/ > > > The commit message there could have explicitly said having this > > > #include causes the conflicting definitions between perf's debug.h and > > > libbpf_internal.h's definitions of pr_info, etc. > > > > yeah, understood, but I'm not processing patches for tools/lib/bpf/, > > Daniel is, I'll only get that one later, then we can go back to the way > > you structured it. Just an extra bit of confusion in this process ;-) > > So, thiis is failing on all alpine Linux containers: > > CC /tmp/build/perf/util/metricgroup.o > CC /tmp/build/perf/util/header.o > In file included from util/metricgroup.c:25:0: > /git/linux/tools/lib/api/fs/fs.h:16:0: error: "FS" redefined [-Werror] > #define FS(name) \ > ^ > In file included from /git/linux/tools/perf/util/hashmap.h:16:0, > from util/expr.h:11, > from util/metricgroup.c:14: > /usr/include/bits/reg.h:28:0: note: this is the location of the previous definition > #define FS 25 > ^ > CC /tmp/build/perf/util/callchain.o > CC /tmp/build/perf/util/values.o > CC /tmp/build/perf/util/debug.o > CC /tmp/build/perf/util/fncache.o > cc1: all warnings being treated as errors > mv: can't rename '/tmp/build/perf/util/.metricgroup.o.tmp': No such file or directory > /git/linux/tools/build/Makefile.build:96: recipe for target '/tmp/build/perf/util/metricgroup.o' failed > > > I'll check that soon, > > - Arnaldo I had some issues here too: https://lore.kernel.org/lkml/CAEf4BzYxTTND7T7X0dLr2CbkEvUuKtarOeoJYYROefij+qds0w@xxxxxxxxxxxxxx/ The only reason for the bits/reg.h inclusion is for __WORDSIZE for the hash_bits operation. As shown below: #ifdef __GLIBC__ #include <bits/wordsize.h> #else #include <bits/reg.h> #endif static inline size_t hash_bits(size_t h, int bits) { /* shuffle bits and return requested number of upper bits */ return (h * 11400714819323198485llu) >> (__WORDSIZE - bits); } It'd be possible to change the definition of hash_bits and remove the #includes by: static inline size_t hash_bits(size_t h, int bits) { /* shuffle bits and return requested number of upper bits */ #ifdef __LP64__ int shift = 64 - bits; #else int shift = 32 - bits; #endif return (h * 11400714819323198485llu) >> shift; } Others may have a prefered more portable solution. A separate issue with this same function is undefined behavior getting flagged (unnecessarily) by sanitizers: https://lore.kernel.org/lkml/20200508063954.256593-1-irogers@xxxxxxxxxx/ I was planning to come back to that once we got these changes landed. Thanks! Ian
