Re: [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile

[Quentin Monnet <quentin@isovalent.com>]

2020-03-04 21:39 UTC+0000 ~ Song Liu <songliubraving@fb.com>
> 
> 
>> On Mar 4, 2020, at 1:29 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>>
>> On Wed, Mar 04, 2020 at 09:16:29PM +0000, Song Liu wrote:
>>>
>>>
>>>> On Mar 4, 2020, at 12:41 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>>>>
>>>> On Wed, Mar 04, 2020 at 08:08:07PM +0100, Jiri Olsa wrote:
>>>>> On Wed, Mar 04, 2020 at 10:07:06AM -0800, Song Liu wrote:
>>>>>> This set introduces bpftool prog profile command, which uses hardware
>>>>>> counters to profile BPF programs.
>>>>>>
>>>>>> This command attaches fentry/fexit programs to a target program. These two
>>>>>> programs read hardware counters before and after the target program and
>>>>>> calculate the difference.
>>>>>>
>>>>>> Changes v3 => v4:
>>>>>> 1. Simplify err handling in profile_open_perf_events() (Quentin);
>>>>>> 2. Remove redundant p_err() (Quentin);
>>>>>> 3. Replace tab with space in bash-completion; (Quentin);
>>>>>> 4. Fix typo _bpftool_get_map_names => _bpftool_get_prog_names (Quentin).
>>>>>
>>>>> hum, I'm getting:
>>>>>
>>>>> 	[jolsa@dell-r440-01 bpftool]$ pwd
>>>>> 	/home/jolsa/linux-perf/tools/bpf/bpftool
>>>>> 	[jolsa@dell-r440-01 bpftool]$ make
>>>>> 	...
>>>>> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
>>>>> 	  LINK     _bpftool
>>>>> 	make: *** No rule to make target 'skeleton/profiler.bpf.c', needed by 'skeleton/profiler.bpf.o'.  Stop.
>>>>
>>>> ok, I had to apply your patches by hand, because 'git am' refused to
>>>> due to fuzz.. so some of you new files did not make it to my tree ;-)
>>>>
>>>> anyway I hit another error now:
>>>>
>>>> 	  CC       prog.o
>>>> 	In file included from prog.c:1553:
>>>> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
>>>> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>>> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
>>>> 	      |                                   ^~
>>>> 	prog.c: In function ‘profile_read_values’:
>>>> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>>> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>>>>
>>>> I'll try to figure it out.. might be error on my end
>>>>
>>>> do you have git repo with these changes?
>>>
>>> I pushed it to 
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/song/linux.git/tree/?h=bpf-per-prog-stats
>>
>> still the same:
>>
>> 	[jolsa@dell-r440-01 bpftool]$ git show --oneline HEAD | head -1
>> 	7bbda5cca00a bpftool: fix typo in bash-completion
>> 	[jolsa@dell-r440-01 bpftool]$ make 
>> 	make[1]: Entering directory '/home/jolsa/linux-perf/tools/lib/bpf'
>> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
>> 	  CC       prog.o
>> 	In file included from prog.c:1553:
>> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
>> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
>> 	      |                                   ^~
>> 	prog.c: In function ‘profile_read_values’:
>> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>> 	      |                             ^~
>> 	prog.c: In function ‘profile_open_perf_events’:
>> 	prog.c:1810:19: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
>> 	      |                   ^~
>> 	prog.c:1810:42: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
>> 	      |                                          ^~
>> 	prog.c:1825:26: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1825 |   for (cpu = 0; cpu < obj->rodata->num_cpu; cpu++) {
>> 	      |                          ^~
>> 	prog.c: In function ‘do_profile’:
>> 	prog.c:1904:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1904 |  profile_obj->rodata->num_cpu = num_cpu;
>> 	      |             ^~
>> 	prog.c:1905:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1905 |  profile_obj->rodata->num_metric = num_metric;
>> 	      |             ^~
>> 	make: *** [Makefile:129: prog.o] Error 1
> 
> I guess you need a newer version of clang that supports global data in BPF programs. 
> 
> Thanks,
> Song
> 

Thinking about this requirement again... Do you think it would be worth
adding (as a follow-up) a feature check on the availability of clang
with global data support to bpftool's Makefile? So that we could compile
out program profiling if clang is not present or does not support it.
Just like libbfd support is optional already.

I'm asking mostly because a number of distributions now package bpftool,
and e.g. Ubuntu builds it from kernel source when creating its
linux-images and linux-tools-* packages. And I am pretty sure the build
environment does not have latest clang/LLVM, but it would be great to
remain able to build bpftool.

Best regards,
Quentin