| Message ID | 20191127134553.GC22719@kernel.org |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches | expand |
On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Another fix I'm carrying in my perf/core branch, Why in perf/core? I very much prefer all libbpf patches to go via normal route via bpf/net trees. We had enough conflicts in this merge window. Let's avoid them.
Em Wed, Nov 27, 2019 at 08:39:28AM -0800, Alexei Starovoitov escreveu: > On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo > <acme@kernel.org> wrote: > > > > Another fix I'm carrying in my perf/core branch, > Why in perf/core? > I very much prefer all libbpf patches to go via normal route via bpf/net trees. > We had enough conflicts in this merge window. Let's avoid them. Humm, if we both carry the same patch the merge process can do its magic and nobody gets hurt? Besides these are really minor things, no? - Arnaldo
On Wed, Nov 27, 2019 at 10:45 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Wed, Nov 27, 2019 at 08:39:28AM -0800, Alexei Starovoitov escreveu: > > On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo > > <acme@kernel.org> wrote: > > > > > > Another fix I'm carrying in my perf/core branch, > > > Why in perf/core? > > I very much prefer all libbpf patches to go via normal route via bpf/net trees. > > We had enough conflicts in this merge window. Let's avoid them. > > Humm, if we both carry the same patch the merge process can do its magic > and nobody gets hurt? Besides these are really minor things, no? I thought so too, but learned the hard lesson recently. We should try to avoid that as much as possible. Andrii's is fixing stuff in the same lines: https://patchwork.ozlabs.org/patch/1201344/ these two patches will likely conflict. I'd rather have them both in bpf tree. What is the value for this patch in perf tree? To fix the build on 32-bit arches, right? But how urgent is it? Can you wait few days until this one and other libbpf fixes land via bpf/net trees?
On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Another fix I'm carrying in my perf/core branch, > > Regards, > > - Arnaldo > > commit 98bb09f90a0ae33125fabc8f41529345382f1498 > Author: Arnaldo Carvalho de Melo <acme@redhat.com> > Date: Wed Nov 27 09:26:54 2019 -0300 > > libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches > > The st_value field is a 64-bit value, so use PRIu64 to fix this error on > 32-bit arches: > > In file included from libbpf.c:52: > libbpf.c: In function 'bpf_program__record_reloc': > libbpf_internal.h:59:22: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Addr' {aka 'const long long unsigned int'} [-Werror=format=] > libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \ > ^~~~~~~~~~ > libbpf_internal.h:62:27: note: in expansion of macro '__pr' > #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__) > ^~~~ > libbpf.c:1822:4: note: in expansion of macro 'pr_warn' > pr_warn("bad call relo offset: %lu\n", sym->st_value); > ^~~~~~~ > libbpf.c:1822:37: note: format string is defined here > pr_warn("bad call relo offset: %lu\n", sym->st_value); > ~~^ > %llu > > Fixes: 1f8e2bcb2cd5 ("libbpf: Refactor relocation handling") > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Andrii Nakryiko <andriin@fb.com> > Link: https://lkml.kernel.org/n/tip-iabs1wq19c357bkk84p7blif@git.kernel.org > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index b20f82e58989..6b0eae5c8a94 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -1819,7 +1819,7 @@ static int bpf_program__record_reloc(struct bpf_program *prog, > return -LIBBPF_ERRNO__RELOC; > } > if (sym->st_value % 8) { > - pr_warn("bad call relo offset: %lu\n", sym->st_value); > + pr_warn("bad call relo offset: %" PRIu64 "\n", sym->st_value); Looking at this more... I never liked this PRI stuff. It makes for such unreadable code. How about just typecasting st_value to (long) ?
Em Wed, Nov 27, 2019 at 10:55:31AM -0800, Alexei Starovoitov escreveu: > On Wed, Nov 27, 2019 at 10:45 AM Arnaldo Carvalho de Melo > <acme@kernel.org> wrote: > > > > Em Wed, Nov 27, 2019 at 08:39:28AM -0800, Alexei Starovoitov escreveu: > > > On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo > > > <acme@kernel.org> wrote: > > > > > > > > Another fix I'm carrying in my perf/core branch, > > > > > Why in perf/core? > > > I very much prefer all libbpf patches to go via normal route via bpf/net trees. > > > We had enough conflicts in this merge window. Let's avoid them. > > > > Humm, if we both carry the same patch the merge process can do its magic > > and nobody gets hurt? Besides these are really minor things, no? > > I thought so too, but learned the hard lesson recently. > We should try to avoid that as much as possible. > Andrii's is fixing stuff in the same lines: > https://patchwork.ozlabs.org/patch/1201344/ > these two patches will likely conflict. I'd rather have them both in bpf tree. > What is the value for this patch in perf tree? > To fix the build on 32-bit arches, right? > But how urgent is it? Can you wait few days until this one and other > libbpf fixes > land via bpf/net trees? Ok, I'll add a note to the pull request report about where the perf build is clean in all containers because I added these two patches, but that they'll go via the bpf tree, as soon as that gets merged, the problem will go away. And I wasn't strictly defending that I should carry this in perf/core, just said I was, to fix something minor that I found while doing my usual testing, patch was posted, you got notified and got the patch, I'll remove it from perf/core now since you stated that it'll eventually land upstream. Thanks, - Arnaldo
Hi Arnaldo, FYI, On Wed, 27 Nov 2019 at 19:15, Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > Another fix I'm carrying in my perf/core branch, > > Regards, > > - Arnaldo > > commit 98bb09f90a0ae33125fabc8f41529345382f1498 > Author: Arnaldo Carvalho de Melo <acme@redhat.com> > Date: Wed Nov 27 09:26:54 2019 -0300 > > libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches > > The st_value field is a 64-bit value, so use PRIu64 to fix this error on > 32-bit arches: > > In file included from libbpf.c:52: > libbpf.c: In function 'bpf_program__record_reloc': > libbpf_internal.h:59:22: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Addr' {aka 'const long long unsigned int'} [-Werror=format=] > libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \ > ^~~~~~~~~~ > libbpf_internal.h:62:27: note: in expansion of macro '__pr' > #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__) > ^~~~ > libbpf.c:1822:4: note: in expansion of macro 'pr_warn' > pr_warn("bad call relo offset: %lu\n", sym->st_value); > ^~~~~~~ > libbpf.c:1822:37: note: format string is defined here > pr_warn("bad call relo offset: %lu\n", sym->st_value); > ~~^ > %llu This build error is been noticed on Linux mainline kernel for 32-bit architectures from Nov 26. Full build log, https://ci.linaro.org/job/openembedded-lkft-linux-mainline/DISTRO=lkft,MACHINE=intel-core2-32,label=docker-lkft/2297/consoleText https://ci.linaro.org/job/openembedded-lkft-linux-mainline/ - Naresh
Em Tue, Dec 03, 2019 at 07:20:08PM +0530, Naresh Kamboju escreveu: > Hi Arnaldo, > > FYI, > > On Wed, 27 Nov 2019 at 19:15, Arnaldo Carvalho de Melo > <arnaldo.melo@gmail.com> wrote: > > > > Another fix I'm carrying in my perf/core branch, > > > > Regards, > > > > - Arnaldo > > > > commit 98bb09f90a0ae33125fabc8f41529345382f1498 > > Author: Arnaldo Carvalho de Melo <acme@redhat.com> > > Date: Wed Nov 27 09:26:54 2019 -0300 > > > > libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches > > > > The st_value field is a 64-bit value, so use PRIu64 to fix this error on > > 32-bit arches: > > > > In file included from libbpf.c:52: > > libbpf.c: In function 'bpf_program__record_reloc': > > libbpf_internal.h:59:22: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Addr' {aka 'const long long unsigned int'} [-Werror=format=] > > libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \ > > ^~~~~~~~~~ > > libbpf_internal.h:62:27: note: in expansion of macro '__pr' > > #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__) > > ^~~~ > > libbpf.c:1822:4: note: in expansion of macro 'pr_warn' > > pr_warn("bad call relo offset: %lu\n", sym->st_value); > > ^~~~~~~ > > libbpf.c:1822:37: note: format string is defined here > > pr_warn("bad call relo offset: %lu\n", sym->st_value); > > ~~^ > > %llu > > This build error is been noticed on Linux mainline kernel for 32-bit > architectures from Nov 26. Right, the fix is in the bpf tree: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=7c3977d1e804 Should go upstream soon. - Arnaldo > Full build log, > https://ci.linaro.org/job/openembedded-lkft-linux-mainline/DISTRO=lkft,MACHINE=intel-core2-32,label=docker-lkft/2297/consoleText > https://ci.linaro.org/job/openembedded-lkft-linux-mainline/ > > - Naresh
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index b20f82e58989..6b0eae5c8a94 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -1819,7 +1819,7 @@ static int bpf_program__record_reloc(struct bpf_program *prog, return -LIBBPF_ERRNO__RELOC; } if (sym->st_value % 8) { - pr_warn("bad call relo offset: %lu\n", sym->st_value); + pr_warn("bad call relo offset: %" PRIu64 "\n", sym->st_value); return -LIBBPF_ERRNO__RELOC; } reloc_desc->type = RELO_CALL;
Another fix I'm carrying in my perf/core branch, Regards, - Arnaldo commit 98bb09f90a0ae33125fabc8f41529345382f1498 Author: Arnaldo Carvalho de Melo <acme@redhat.com> Date: Wed Nov 27 09:26:54 2019 -0300 libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches The st_value field is a 64-bit value, so use PRIu64 to fix this error on 32-bit arches: In file included from libbpf.c:52: libbpf.c: In function 'bpf_program__record_reloc': libbpf_internal.h:59:22: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Addr' {aka 'const long long unsigned int'} [-Werror=format=] libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \ ^~~~~~~~~~ libbpf_internal.h:62:27: note: in expansion of macro '__pr' #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__) ^~~~ libbpf.c:1822:4: note: in expansion of macro 'pr_warn' pr_warn("bad call relo offset: %lu\n", sym->st_value); ^~~~~~~ libbpf.c:1822:37: note: format string is defined here pr_warn("bad call relo offset: %lu\n", sym->st_value); ~~^ %llu Fixes: 1f8e2bcb2cd5 ("libbpf: Refactor relocation handling") Cc: Alexei Starovoitov <ast@kernel.org> Cc: Andrii Nakryiko <andriin@fb.com> Link: https://lkml.kernel.org/n/tip-iabs1wq19c357bkk84p7blif@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>