diff mbox series

[bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create_map

Message ID 20211206230811.4131230-1-song@kernel.org (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series [bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create_map | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 12 maintainers not CCed: linux-perf-users@vger.kernel.org peterz@infradead.org kafai@fb.com alexander.shishkin@linux.intel.com songliubraving@fb.com john.fastabend@gmail.com namhyung@kernel.org mingo@redhat.com kpsingh@kernel.org mark.rutland@arm.com yhs@fb.com jolsa@redhat.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 12 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next fail VM_Test
bpf/vmtest-bpf-next-PR fail PR summary

Commit Message

Song Liu Dec. 6, 2021, 11:08 p.m. UTC
bpf_create_map is deprecated. Replace it with bpf_map_create.

Fixes: 992c4225419a ("libbpf: Unify low-level map creation APIs w/ new bpf_map_create()")
Signed-off-by: Song Liu <song@kernel.org>
---
 tools/perf/util/bpf_counter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andrii Nakryiko Dec. 7, 2021, 2:37 a.m. UTC | #1
On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@kernel.org> wrote:
>
> bpf_create_map is deprecated. Replace it with bpf_map_create.
>
> Fixes: 992c4225419a ("libbpf: Unify low-level map creation APIs w/ new bpf_map_create()")

This is not a bug fix, it's an improvement. So I don't think "Fixes: "
is warranted here, tbh.

> Signed-off-by: Song Liu <song@kernel.org>
> ---
>  tools/perf/util/bpf_counter.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
> index c17d4a43ce065..ed150a9b3a0c0 100644
> --- a/tools/perf/util/bpf_counter.c
> +++ b/tools/perf/util/bpf_counter.c
> @@ -320,10 +320,10 @@ static int bperf_lock_attr_map(struct target *target)
>         }
>
>         if (access(path, F_OK)) {
> -               map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
> +               map_fd = bpf_map_create(BPF_MAP_TYPE_HASH, NULL,

I think perf is trying to be linkable with libbpf as a shared library,
so on some older versions of libbpf bpf_map_create() won't be (yet)
available. So to make this work, I think you'll need to define your
own weak bpf_map_create function that will use bpf_create_map().

>                                         sizeof(struct perf_event_attr),
>                                         sizeof(struct perf_event_attr_map_entry),
> -                                       ATTR_MAP_SIZE, 0);
> +                                       ATTR_MAP_SIZE, NULL);
>                 if (map_fd < 0)
>                         return -1;
>
> --
> 2.30.2
>
Song Liu Dec. 7, 2021, 4:32 a.m. UTC | #2
> On Dec 6, 2021, at 6:37 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> 
> On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@kernel.org> wrote:
>> 
>> bpf_create_map is deprecated. Replace it with bpf_map_create.
>> 
>> Fixes: 992c4225419a ("libbpf: Unify low-level map creation APIs w/ new bpf_map_create()")
> 
> This is not a bug fix, it's an improvement. So I don't think "Fixes: "
> is warranted here, tbh.

I got compilation errors before this change, like

util/bpf_counter.c: In function ‘bperf_lock_attr_map’:
util/bpf_counter.c:323:3: error: ‘bpf_create_map’ is deprecated: libbpf v0.7+: use bpf_map_create() instead [-Werror=deprecated-declarations]
   map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
   ^~~~~~
In file included from util/bpf_counter.h:7,
                 from util/bpf_counter.c:15:
/data/users/songliubraving/kernel/linux-git/tools/lib/bpf/bpf.h:91:16: note: declared here
 LIBBPF_API int bpf_create_map(enum bpf_map_type map_type, int key_size,
                ^~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[4]: *** [/data/users/songliubraving/kernel/linux-git/tools/build/Makefile.build:96: util/bpf_counter.o] Error 1
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [/data/users/songliubraving/kernel/linux-git/tools/build/Makefile.build:139: util] Error 2
make[2]: *** [Makefile.perf:665: perf-in.o] Error 2
make[1]: *** [Makefile.perf:240: sub-make] Error 2
make: *** [Makefile:70: all] Error 2

Do we plan to remove bpf_create_map in the future? If not, we can probably just
add '#pragma GCC diagnostic ignored "-Wdeprecated-declarations"' can call it done? 

> 
>> Signed-off-by: Song Liu <song@kernel.org>
>> ---
>> tools/perf/util/bpf_counter.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
>> index c17d4a43ce065..ed150a9b3a0c0 100644
>> --- a/tools/perf/util/bpf_counter.c
>> +++ b/tools/perf/util/bpf_counter.c
>> @@ -320,10 +320,10 @@ static int bperf_lock_attr_map(struct target *target)
>>        }
>> 
>>        if (access(path, F_OK)) {
>> -               map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
>> +               map_fd = bpf_map_create(BPF_MAP_TYPE_HASH, NULL,
> 
> I think perf is trying to be linkable with libbpf as a shared library,
> so on some older versions of libbpf bpf_map_create() won't be (yet)
> available. So to make this work, I think you'll need to define your
> own weak bpf_map_create function that will use bpf_create_map().

Hmm... I didn't know the plan to link libbpf as shared library. In this case, 
maybe the #pragma solution is preferred? 

Thanks,
Song
Andrii Nakryiko Dec. 7, 2021, 5:13 a.m. UTC | #3
On Mon, Dec 6, 2021 at 8:32 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Dec 6, 2021, at 6:37 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@kernel.org> wrote:
> >>
> >> bpf_create_map is deprecated. Replace it with bpf_map_create.
> >>
> >> Fixes: 992c4225419a ("libbpf: Unify low-level map creation APIs w/ new bpf_map_create()")
> >
> > This is not a bug fix, it's an improvement. So I don't think "Fixes: "
> > is warranted here, tbh.
>
> I got compilation errors before this change, like
>
> util/bpf_counter.c: In function ‘bperf_lock_attr_map’:
> util/bpf_counter.c:323:3: error: ‘bpf_create_map’ is deprecated: libbpf v0.7+: use bpf_map_create() instead [-Werror=deprecated-declarations]
>    map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
>    ^~~~~~
> In file included from util/bpf_counter.h:7,
>                  from util/bpf_counter.c:15:
> /data/users/songliubraving/kernel/linux-git/tools/lib/bpf/bpf.h:91:16: note: declared here
>  LIBBPF_API int bpf_create_map(enum bpf_map_type map_type, int key_size,
>                 ^~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> make[4]: *** [/data/users/songliubraving/kernel/linux-git/tools/build/Makefile.build:96: util/bpf_counter.o] Error 1
> make[4]: *** Waiting for unfinished jobs....
> make[3]: *** [/data/users/songliubraving/kernel/linux-git/tools/build/Makefile.build:139: util] Error 2
> make[2]: *** [Makefile.perf:665: perf-in.o] Error 2
> make[1]: *** [Makefile.perf:240: sub-make] Error 2
> make: *** [Makefile:70: all] Error 2
>

Hmm.. is util/bpf_counter.h guarded behind some Makefile arguments?
I've sent #pragma temporary workarounds just a few days ago ([0]), but
this one didn't come up during the build.

  [0] https://patchwork.kernel.org/project/netdevbpf/patch/20211203004640.2455717-1-andrii@kernel.org/

> Do we plan to remove bpf_create_map in the future? If not, we can probably just
> add '#pragma GCC diagnostic ignored "-Wdeprecated-declarations"' can call it done?

Yes, it will be removed in a few libbpf releases when we switch to the
1.0 version. So suppressing a warning is a temporary work-around.

>
> >
> >> Signed-off-by: Song Liu <song@kernel.org>
> >> ---
> >> tools/perf/util/bpf_counter.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
> >> index c17d4a43ce065..ed150a9b3a0c0 100644
> >> --- a/tools/perf/util/bpf_counter.c
> >> +++ b/tools/perf/util/bpf_counter.c
> >> @@ -320,10 +320,10 @@ static int bperf_lock_attr_map(struct target *target)
> >>        }
> >>
> >>        if (access(path, F_OK)) {
> >> -               map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
> >> +               map_fd = bpf_map_create(BPF_MAP_TYPE_HASH, NULL,
> >
> > I think perf is trying to be linkable with libbpf as a shared library,
> > so on some older versions of libbpf bpf_map_create() won't be (yet)
> > available. So to make this work, I think you'll need to define your
> > own weak bpf_map_create function that will use bpf_create_map().
>
> Hmm... I didn't know the plan to link libbpf as shared library. In this case,
> maybe the #pragma solution is preferred?

See "perf tools: Add more weak libbpf functions" sent by Jiri not so
long ago about what they did with some other used APIs that are now
marked deprecated.

>
> Thanks,
> Song
>
Song Liu Dec. 7, 2021, 6:30 a.m. UTC | #4
> On Dec 6, 2021, at 9:13 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> 
> On Mon, Dec 6, 2021 at 8:32 PM Song Liu <songliubraving@fb.com> wrote:
>> 
>> 
>> 
>>> On Dec 6, 2021, at 6:37 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>>> 
>>> On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@kernel.org> wrote:
>>>> 
>>>> bpf_create_map is deprecated. Replace it with bpf_map_create.
>>>> 
>>>> Fixes: 992c4225419a ("libbpf: Unify low-level map creation APIs w/ new bpf_map_create()")
>>> 
>>> This is not a bug fix, it's an improvement. So I don't think "Fixes: "
>>> is warranted here, tbh.
>> 
>> I got compilation errors before this change, like
>> 
>> util/bpf_counter.c: In function ‘bperf_lock_attr_map’:
>> util/bpf_counter.c:323:3: error: ‘bpf_create_map’ is deprecated: libbpf v0.7+: use bpf_map_create() instead [-Werror=deprecated-declarations]
>>   map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
>>   ^~~~~~
>> In file included from util/bpf_counter.h:7,
>>                 from util/bpf_counter.c:15:
>> /data/users/songliubraving/kernel/linux-git/tools/lib/bpf/bpf.h:91:16: note: declared here
>> LIBBPF_API int bpf_create_map(enum bpf_map_type map_type, int key_size,
>>                ^~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>> make[4]: *** [/data/users/songliubraving/kernel/linux-git/tools/build/Makefile.build:96: util/bpf_counter.o] Error 1
>> make[4]: *** Waiting for unfinished jobs....
>> make[3]: *** [/data/users/songliubraving/kernel/linux-git/tools/build/Makefile.build:139: util] Error 2
>> make[2]: *** [Makefile.perf:665: perf-in.o] Error 2
>> make[1]: *** [Makefile.perf:240: sub-make] Error 2
>> make: *** [Makefile:70: all] Error 2
>> 
> 
> Hmm.. is util/bpf_counter.h guarded behind some Makefile arguments?
> I've sent #pragma temporary workarounds just a few days ago ([0]), but
> this one didn't come up during the build.
> 
>  [0] https://patchwork.kernel.org/project/netdevbpf/patch/20211203004640.2455717-1-andrii@kernel.org/

I guess the default build test doesn't enable BUILD_BPF_SKEL? 

> 
>> Do we plan to remove bpf_create_map in the future? If not, we can probably just
>> add '#pragma GCC diagnostic ignored "-Wdeprecated-declarations"' can call it done?
> 
> Yes, it will be removed in a few libbpf releases when we switch to the
> 1.0 version. So suppressing a warning is a temporary work-around.
> 
>> 
>>> 
>>>> Signed-off-by: Song Liu <song@kernel.org>
>>>> ---
>>>> tools/perf/util/bpf_counter.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
>>>> index c17d4a43ce065..ed150a9b3a0c0 100644
>>>> --- a/tools/perf/util/bpf_counter.c
>>>> +++ b/tools/perf/util/bpf_counter.c
>>>> @@ -320,10 +320,10 @@ static int bperf_lock_attr_map(struct target *target)
>>>>       }
>>>> 
>>>>       if (access(path, F_OK)) {
>>>> -               map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
>>>> +               map_fd = bpf_map_create(BPF_MAP_TYPE_HASH, NULL,
>>> 
>>> I think perf is trying to be linkable with libbpf as a shared library,
>>> so on some older versions of libbpf bpf_map_create() won't be (yet)
>>> available. So to make this work, I think you'll need to define your
>>> own weak bpf_map_create function that will use bpf_create_map().
>> 
>> Hmm... I didn't know the plan to link libbpf as shared library. In this case,
>> maybe the #pragma solution is preferred?
> 
> See "perf tools: Add more weak libbpf functions" sent by Jiri not so
> long ago about what they did with some other used APIs that are now
> marked deprecated.

Do you mean something like this?

int __weak
bpf_map_create(enum bpf_map_type map_type,
               const char *map_name __maybe_unused,
               __u32 key_size,
               __u32 value_size,
               __u32 max_entries,
               const struct bpf_map_create_opts *opts __maybe_unused)
{
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
        return bpf_create_map(map_type, key_size, value_size, max_entries, 0);
#pragma GCC diagnostic pop
}

I guess this won't work when bpf_create_map() is eventually removed, as 
__weak function are still compiled, no?

Thanks,
Song
Andrii Nakryiko Dec. 7, 2021, 11:02 p.m. UTC | #5
On Mon, Dec 6, 2021 at 10:30 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Dec 6, 2021, at 9:13 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, Dec 6, 2021 at 8:32 PM Song Liu <songliubraving@fb.com> wrote:
> >>
> >>
> >>
> >>> On Dec 6, 2021, at 6:37 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >>>
> >>> On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@kernel.org> wrote:
> >>>>
> >>>> bpf_create_map is deprecated. Replace it with bpf_map_create.
> >>>>
> >>>> Fixes: 992c4225419a ("libbpf: Unify low-level map creation APIs w/ new bpf_map_create()")
> >>>
> >>> This is not a bug fix, it's an improvement. So I don't think "Fixes: "
> >>> is warranted here, tbh.
> >>
> >> I got compilation errors before this change, like
> >>
> >> util/bpf_counter.c: In function ‘bperf_lock_attr_map’:
> >> util/bpf_counter.c:323:3: error: ‘bpf_create_map’ is deprecated: libbpf v0.7+: use bpf_map_create() instead [-Werror=deprecated-declarations]
> >>   map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
> >>   ^~~~~~
> >> In file included from util/bpf_counter.h:7,
> >>                 from util/bpf_counter.c:15:
> >> /data/users/songliubraving/kernel/linux-git/tools/lib/bpf/bpf.h:91:16: note: declared here
> >> LIBBPF_API int bpf_create_map(enum bpf_map_type map_type, int key_size,
> >>                ^~~~~~~~~~~~~~
> >> cc1: all warnings being treated as errors
> >> make[4]: *** [/data/users/songliubraving/kernel/linux-git/tools/build/Makefile.build:96: util/bpf_counter.o] Error 1
> >> make[4]: *** Waiting for unfinished jobs....
> >> make[3]: *** [/data/users/songliubraving/kernel/linux-git/tools/build/Makefile.build:139: util] Error 2
> >> make[2]: *** [Makefile.perf:665: perf-in.o] Error 2
> >> make[1]: *** [Makefile.perf:240: sub-make] Error 2
> >> make: *** [Makefile:70: all] Error 2
> >>
> >
> > Hmm.. is util/bpf_counter.h guarded behind some Makefile arguments?
> > I've sent #pragma temporary workarounds just a few days ago ([0]), but
> > this one didn't come up during the build.
> >
> >  [0] https://patchwork.kernel.org/project/netdevbpf/patch/20211203004640.2455717-1-andrii@kernel.org/
>
> I guess the default build test doesn't enable BUILD_BPF_SKEL?

I see, right, I think I already asked about that before :( Is it
possible to set Makefile such that it will do BUILD_BPF_SKEL=1 if
Clang version is recent enough and other conditions are satisfied
(unless overridden or something)?

>
> >
> >> Do we plan to remove bpf_create_map in the future? If not, we can probably just
> >> add '#pragma GCC diagnostic ignored "-Wdeprecated-declarations"' can call it done?
> >
> > Yes, it will be removed in a few libbpf releases when we switch to the
> > 1.0 version. So suppressing a warning is a temporary work-around.
> >
> >>
> >>>
> >>>> Signed-off-by: Song Liu <song@kernel.org>
> >>>> ---
> >>>> tools/perf/util/bpf_counter.c | 4 ++--
> >>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
> >>>> index c17d4a43ce065..ed150a9b3a0c0 100644
> >>>> --- a/tools/perf/util/bpf_counter.c
> >>>> +++ b/tools/perf/util/bpf_counter.c
> >>>> @@ -320,10 +320,10 @@ static int bperf_lock_attr_map(struct target *target)
> >>>>       }
> >>>>
> >>>>       if (access(path, F_OK)) {
> >>>> -               map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
> >>>> +               map_fd = bpf_map_create(BPF_MAP_TYPE_HASH, NULL,
> >>>
> >>> I think perf is trying to be linkable with libbpf as a shared library,
> >>> so on some older versions of libbpf bpf_map_create() won't be (yet)
> >>> available. So to make this work, I think you'll need to define your
> >>> own weak bpf_map_create function that will use bpf_create_map().
> >>
> >> Hmm... I didn't know the plan to link libbpf as shared library. In this case,
> >> maybe the #pragma solution is preferred?
> >
> > See "perf tools: Add more weak libbpf functions" sent by Jiri not so
> > long ago about what they did with some other used APIs that are now
> > marked deprecated.
>
> Do you mean something like this?
>
> int __weak
> bpf_map_create(enum bpf_map_type map_type,
>                const char *map_name __maybe_unused,
>                __u32 key_size,
>                __u32 value_size,
>                __u32 max_entries,
>                const struct bpf_map_create_opts *opts __maybe_unused)
> {
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
>         return bpf_create_map(map_type, key_size, value_size, max_entries, 0);
> #pragma GCC diagnostic pop
> }
>
> I guess this won't work when bpf_create_map() is eventually removed, as
> __weak function are still compiled, no?

Yes and yes. I'm not sure what would be perf's plan w.r.t. libbpf 1.0,
we'll need to work together to figure this out. At some point perf
will need to say that the minimum version of supported libbpf is v0.6
or something and just assume all those newer APIs are there (no need
to bump it all the way to libbpf 1.0, btw).

>
> Thanks,
> Song
Song Liu Dec. 7, 2021, 11:10 p.m. UTC | #6
> On Dec 7, 2021, at 3:02 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> 
> On Mon, Dec 6, 2021 at 10:30 PM Song Liu <songliubraving@fb.com> wrote:
>> 
>> 
>> 
>>> On Dec 6, 2021, at 9:13 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>>> 
>>> On Mon, Dec 6, 2021 at 8:32 PM Song Liu <songliubraving@fb.com> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On Dec 6, 2021, at 6:37 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>>>>> 
>>>>> On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@kernel.org> wrote:
>>>>>> 
>>>>>> bpf_create_map is deprecated. Replace it with bpf_map_create.
>>>>>> 
>>>>>> Fixes: 992c4225419a ("libbpf: Unify low-level map creation APIs w/ new bpf_map_create()")
>>>>> 
>>>>> This is not a bug fix, it's an improvement. So I don't think "Fixes: "
>>>>> is warranted here, tbh.
>>>> 
>>>> I got compilation errors before this change, like
>>>> 
>>>> util/bpf_counter.c: In function ‘bperf_lock_attr_map’:
>>>> util/bpf_counter.c:323:3: error: ‘bpf_create_map’ is deprecated: libbpf v0.7+: use bpf_map_create() instead [-Werror=deprecated-declarations]
>>>>  map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
>>>>  ^~~~~~
>>>> In file included from util/bpf_counter.h:7,
>>>>                from util/bpf_counter.c:15:
>>>> /data/users/songliubraving/kernel/linux-git/tools/lib/bpf/bpf.h:91:16: note: declared here
>>>> LIBBPF_API int bpf_create_map(enum bpf_map_type map_type, int key_size,
>>>>               ^~~~~~~~~~~~~~
>>>> cc1: all warnings being treated as errors
>>>> make[4]: *** [/data/users/songliubraving/kernel/linux-git/tools/build/Makefile.build:96: util/bpf_counter.o] Error 1
>>>> make[4]: *** Waiting for unfinished jobs....
>>>> make[3]: *** [/data/users/songliubraving/kernel/linux-git/tools/build/Makefile.build:139: util] Error 2
>>>> make[2]: *** [Makefile.perf:665: perf-in.o] Error 2
>>>> make[1]: *** [Makefile.perf:240: sub-make] Error 2
>>>> make: *** [Makefile:70: all] Error 2
>>>> 
>>> 
>>> Hmm.. is util/bpf_counter.h guarded behind some Makefile arguments?
>>> I've sent #pragma temporary workarounds just a few days ago ([0]), but
>>> this one didn't come up during the build.
>>> 
>>> [0] https://patchwork.kernel.org/project/netdevbpf/patch/20211203004640.2455717-1-andrii@kernel.org/
>> 
>> I guess the default build test doesn't enable BUILD_BPF_SKEL?
> 
> I see, right, I think I already asked about that before :( Is it
> possible to set Makefile such that it will do BUILD_BPF_SKEL=1 if
> Clang version is recent enough and other conditions are satisfied
> (unless overridden or something)?

Arnaldo is working on this. I guess we can flip the default soon. 

> 
>> 
>>> 
>>>> Do we plan to remove bpf_create_map in the future? If not, we can probably just
>>>> add '#pragma GCC diagnostic ignored "-Wdeprecated-declarations"' can call it done?
>>> 
>>> Yes, it will be removed in a few libbpf releases when we switch to the
>>> 1.0 version. So suppressing a warning is a temporary work-around.
>>> 
>>>> 
>>>>> 
>>>>>> Signed-off-by: Song Liu <song@kernel.org>
>>>>>> ---
>>>>>> tools/perf/util/bpf_counter.c | 4 ++--
>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>> 
>>>>>> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
>>>>>> index c17d4a43ce065..ed150a9b3a0c0 100644
>>>>>> --- a/tools/perf/util/bpf_counter.c
>>>>>> +++ b/tools/perf/util/bpf_counter.c
>>>>>> @@ -320,10 +320,10 @@ static int bperf_lock_attr_map(struct target *target)
>>>>>>      }
>>>>>> 
>>>>>>      if (access(path, F_OK)) {
>>>>>> -               map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
>>>>>> +               map_fd = bpf_map_create(BPF_MAP_TYPE_HASH, NULL,
>>>>> 
>>>>> I think perf is trying to be linkable with libbpf as a shared library,
>>>>> so on some older versions of libbpf bpf_map_create() won't be (yet)
>>>>> available. So to make this work, I think you'll need to define your
>>>>> own weak bpf_map_create function that will use bpf_create_map().
>>>> 
>>>> Hmm... I didn't know the plan to link libbpf as shared library. In this case,
>>>> maybe the #pragma solution is preferred?
>>> 
>>> See "perf tools: Add more weak libbpf functions" sent by Jiri not so
>>> long ago about what they did with some other used APIs that are now
>>> marked deprecated.
>> 
>> Do you mean something like this?
>> 
>> int __weak
>> bpf_map_create(enum bpf_map_type map_type,
>>               const char *map_name __maybe_unused,
>>               __u32 key_size,
>>               __u32 value_size,
>>               __u32 max_entries,
>>               const struct bpf_map_create_opts *opts __maybe_unused)
>> {
>> #pragma GCC diagnostic push
>> #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
>>        return bpf_create_map(map_type, key_size, value_size, max_entries, 0);
>> #pragma GCC diagnostic pop
>> }
>> 
>> I guess this won't work when bpf_create_map() is eventually removed, as
>> __weak function are still compiled, no?
> 
> Yes and yes. I'm not sure what would be perf's plan w.r.t. libbpf 1.0,
> we'll need to work together to figure this out. At some point perf
> will need to say that the minimum version of supported libbpf is v0.6
> or something and just assume all those newer APIs are there (no need
> to bump it all the way to libbpf 1.0, btw).

OK. I will send this version. And we can decide the next step when we
remove bpf_create_map(). 

Thanks,
Song
diff mbox series

Patch

diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
index c17d4a43ce065..ed150a9b3a0c0 100644
--- a/tools/perf/util/bpf_counter.c
+++ b/tools/perf/util/bpf_counter.c
@@ -320,10 +320,10 @@  static int bperf_lock_attr_map(struct target *target)
 	}
 
 	if (access(path, F_OK)) {
-		map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
+		map_fd = bpf_map_create(BPF_MAP_TYPE_HASH, NULL,
 					sizeof(struct perf_event_attr),
 					sizeof(struct perf_event_attr_map_entry),
-					ATTR_MAP_SIZE, 0);
+					ATTR_MAP_SIZE, NULL);
 		if (map_fd < 0)
 			return -1;