Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1570095 > unrolled thread

Re: [PATCHv3 perf/core 1/6] tools lib bpf: Add BPF program pinning APIs.

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCHv3 perf/core 1/6] tools lib bpf: Add BPF program pinning  APIs. Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-01-30 21:30 +0100
    Re: [PATCHv3 perf/core 1/6] tools lib bpf: Add BPF program pinning APIs. Joe Stringer <joe@ovn.org> - 2017-01-30 22:20 +0100
      Re: [PATCHv3 perf/core 1/6] tools lib bpf: Add BPF program pinning  APIs. Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-01-31 02:10 +0100
        Re: [PATCHv3 perf/core 1/6] tools lib bpf: Add BPF program pinning  APIs. Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-01-31 17:10 +0100
          Re: [PATCHv3 perf/core 1/6] tools lib bpf: Add BPF program pinning  APIs. Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-01-31 17:20 +0100
            Re: [PATCHv3 perf/core 1/6] tools lib bpf: Add BPF program pinning  APIs. Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-01-31 17:50 +0100

#1570095 — Re: [PATCHv3 perf/core 1/6] tools lib bpf: Add BPF program pinning APIs.

Em Mon, Jan 30, 2017 at 05:25:06PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jan 26, 2017 at 01:19:56PM -0800, Joe Stringer escreveu:
> > Add new APIs to pin a BPF program (or specific instances) to the filesystem.
> > The user can specify the path full path within a BPF filesystem to pin the
> > program.
> > 
> > bpf_program__pin_instance(prog, path, n) will pin the nth instance of
> > 'prog' to the specified path.
> > bpf_program__pin(prog, path) will create the directory 'path' (if it
> > does not exist) and pin each instance within that directory. For
> > instance, path/0, path/1, path/2.
> > 
> > Signed-off-by: Joe Stringer <joe@ovn.org>
> 
> make: Entering directory '/home/acme/git/linux/tools/perf'
>   BUILD:   Doing 'make -j4' parallel build
>   CC       /tmp/build/perf/builtin-record.o
>   CC       /tmp/build/perf/libbpf.o
>   CC       /tmp/build/perf/util/parse-events.o
>   INSTALL  trace_plugins
> libbpf.c: In function ‘make_dir’:
> libbpf.c:1303:6: error: implicit declaration of function ‘mkdir’ [-Werror=implicit-function-declaration]
>   if (mkdir(path, 0700) && errno != EEXIST)
>       ^~~~~
> libbpf.c:1303:2: error: nested extern declaration of ‘mkdir’ [-Werror=nested-externs]
>   if (mkdir(path, 0700) && errno != EEXIST)
>   ^~
> cc1: all warnings being treated as errors
> mv: cannot stat '/tmp/build/perf/.libbpf.o.tmp': No such file or directory
> /home/acme/git/linux/tools/build/Makefile.build:101: recipe for target '/tmp/build/perf/libbpf.o' failed
> 
> 
> And strdup() is not checked for failure, I'm fixing those,
> 
> +++ b/tools/lib/bpf/libbpf.c
> @@ -36,6 +36,8 @@
>  #include <linux/magic.h>
>  #include <linux/list.h>
>  #include <linux/limits.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
>  #include <sys/vfs.h>

This as well:

@@ -1338,7 +1343,7 @@ int bpf_program__pin(struct bpf_program *prog,
const char *path)
                len = snprintf(buf, PATH_MAX, "%s/%d", path, i);
                if (len < 0)
                        return -EINVAL;
-               else if (len > PATH_MAX)
+               else if (len >= PATH_MAX)
                        return -ENAMETOOLONG;


See 'man snprintf', return value:

---
Thus, a return value of size or more means that the output was
truncated.
---

[toc] | [next] | [standalone]

#1570122 — Re: [PATCHv3 perf/core 1/6] tools lib bpf: Add BPF program pinning APIs.

On 30 January 2017 at 12:28, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> Em Mon, Jan 30, 2017 at 05:25:06PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Thu, Jan 26, 2017 at 01:19:56PM -0800, Joe Stringer escreveu:
>> > Add new APIs to pin a BPF program (or specific instances) to the filesystem.
>> > The user can specify the path full path within a BPF filesystem to pin the
>> > program.
>> >
>> > bpf_program__pin_instance(prog, path, n) will pin the nth instance of
>> > 'prog' to the specified path.
>> > bpf_program__pin(prog, path) will create the directory 'path' (if it
>> > does not exist) and pin each instance within that directory. For
>> > instance, path/0, path/1, path/2.
>> >
>> > Signed-off-by: Joe Stringer <joe@ovn.org>
>>
>> make: Entering directory '/home/acme/git/linux/tools/perf'
>>   BUILD:   Doing 'make -j4' parallel build
>>   CC       /tmp/build/perf/builtin-record.o
>>   CC       /tmp/build/perf/libbpf.o
>>   CC       /tmp/build/perf/util/parse-events.o
>>   INSTALL  trace_plugins
>> libbpf.c: In function ‘make_dir’:
>> libbpf.c:1303:6: error: implicit declaration of function ‘mkdir’ [-Werror=implicit-function-declaration]
>>   if (mkdir(path, 0700) && errno != EEXIST)
>>       ^~~~~
>> libbpf.c:1303:2: error: nested extern declaration of ‘mkdir’ [-Werror=nested-externs]
>>   if (mkdir(path, 0700) && errno != EEXIST)
>>   ^~
>> cc1: all warnings being treated as errors
>> mv: cannot stat '/tmp/build/perf/.libbpf.o.tmp': No such file or directory
>> /home/acme/git/linux/tools/build/Makefile.build:101: recipe for target '/tmp/build/perf/libbpf.o' failed

Not sure why but I didn't see this. Appreciate the fix.

>>
>>
>> And strdup() is not checked for failure, I'm fixing those,
>>
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -36,6 +36,8 @@
>>  #include <linux/magic.h>
>>  #include <linux/list.h>
>>  #include <linux/limits.h>
>> +#include <sys/stat.h>
>> +#include <sys/types.h>
>>  #include <sys/vfs.h>
>
> This as well:
>
> @@ -1338,7 +1343,7 @@ int bpf_program__pin(struct bpf_program *prog,
> const char *path)
>                 len = snprintf(buf, PATH_MAX, "%s/%d", path, i);
>                 if (len < 0)
>                         return -EINVAL;
> -               else if (len > PATH_MAX)
> +               else if (len >= PATH_MAX)
>                         return -ENAMETOOLONG;
>
>
> See 'man snprintf', return value:
>
> ---
> Thus, a return value of size or more means that the output was
> truncated.
> ---

Good spotting, I looked over the committed versions and tested them,
they seem good to me. Thanks!

[toc] | [prev] | [next] | [standalone]

#1570228

Em Mon, Jan 30, 2017 at 01:16:18PM -0800, Joe Stringer escreveu:
> On 30 January 2017 at 12:28, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > ---
> > Thus, a return value of size or more means that the output was
> > truncated.
> > ---
 
> Good spotting, I looked over the committed versions and tested them,
> they seem good to me. Thanks!

Thanks for checking, will push Ingo's way after a battery of extra
tests, tomorrow,

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1570855

Em Mon, Jan 30, 2017 at 09:58:05PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Jan 30, 2017 at 01:16:18PM -0800, Joe Stringer escreveu:
> > On 30 January 2017 at 12:28, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > ---
> > > Thus, a return value of size or more means that the output was
> > > truncated.
> > > ---
>  
> > Good spotting, I looked over the committed versions and tested them,
> > they seem good to me. Thanks!
> 
> Thanks for checking, will push Ingo's way after a battery of extra
> tests, tomorrow,

Which failed for centos:5, centos:6, centos:7, debian:7, debian:8,
debian:experimental and others, I stopped the test at this point,
working on fixing it.

All seems related to:

libbpf.c:1267: error: 'BPF_FS_MAGIC' undeclared (first use in this function)
libbpf.c:1267: error: (Each undeclared identifier is reported only once
libbpf.c:1267: error: for each function it appears in.)

[toc] | [prev] | [next] | [standalone]

#1570870

Em Tue, Jan 31, 2017 at 01:08:27PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Jan 30, 2017 at 09:58:05PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Mon, Jan 30, 2017 at 01:16:18PM -0800, Joe Stringer escreveu:
> > > On 30 January 2017 at 12:28, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > ---
> > > > Thus, a return value of size or more means that the output was
> > > > truncated.
> > > > ---
> >  
> > > Good spotting, I looked over the committed versions and tested them,
> > > they seem good to me. Thanks!
> > 
> > Thanks for checking, will push Ingo's way after a battery of extra
> > tests, tomorrow,
> 
> Which failed for centos:5, centos:6, centos:7, debian:7, debian:8,
> debian:experimental and others, I stopped the test at this point,
> working on fixing it.
> 
> All seems related to:
> 
> libbpf.c:1267: error: 'BPF_FS_MAGIC' undeclared (first use in this function)
> libbpf.c:1267: error: (Each undeclared identifier is reported only once
> libbpf.c:1267: error: for each function it appears in.)

We need to carry a tools/include/uapi/linux/magic.c copy, check if it
drifts, remove the ifdefs for _FS_MAGIC defines from tools/ and use that
instead, etc, till then I'll just add the ifdef to libbpf.c.

[acme@jouet linux]$ grep BPF_FS_MAGIC /usr/include/*/*.h
/usr/include/linux/magic.h:#define BPF_FS_MAGIC		0xcafe4a11
[acme@jouet linux]$ rpm -qf /usr/include/linux/magic.h
kernel-headers-4.9.6-200.fc25.x86_64
[acme@jouet linux]$ cat /etc/fedora-release 
Fedora release 25 (Twenty Five)
[acme@jouet linux]$ 

But those other distros don't have it.

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1570896

Em Tue, Jan 31, 2017 at 01:13:20PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Jan 31, 2017 at 01:08:27PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Mon, Jan 30, 2017 at 09:58:05PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Mon, Jan 30, 2017 at 01:16:18PM -0800, Joe Stringer escreveu:
> > > > On 30 January 2017 at 12:28, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > > ---
> > > > > Thus, a return value of size or more means that the output was
> > > > > truncated.
> > > > > ---
> > >  
> > > > Good spotting, I looked over the committed versions and tested them,
> > > > they seem good to me. Thanks!
> > > 
> > > Thanks for checking, will push Ingo's way after a battery of extra
> > > tests, tomorrow,
> > 
> > Which failed for centos:5, centos:6, centos:7, debian:7, debian:8,
> > debian:experimental and others, I stopped the test at this point,
> > working on fixing it.
> > 
> > All seems related to:
> > 
> > libbpf.c:1267: error: 'BPF_FS_MAGIC' undeclared (first use in this function)
> > libbpf.c:1267: error: (Each undeclared identifier is reported only once
> > libbpf.c:1267: error: for each function it appears in.)
> 
> We need to carry a tools/include/uapi/linux/magic.c copy, check if it
> drifts, remove the ifdefs for _FS_MAGIC defines from tools/ and use that
> instead, etc, till then I'll just add the ifdef to libbpf.c.

After also removing that

#include <linux/magic.h>

line, that is not used anywhere else in tools/{perf,include,lib}/ it is
going further:

[root@jouet ~]# time dm
   1 83.120412349 alpine:3.4: Ok
   2 35.486456929 android-ndk:r12b-arm: Ok
   3 85.384259996 archlinux:latest: Ok
   4 49.518031326 centos:5: Ok
   5 70.417375831 centos:6: Ok
   6 87.033156092 centos:7: Ok

31 more to go

:-)

- Arnaldo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web