From: Yuze Chi <[email protected]>
Move the correct definition from linker.c into libbpf_internal.h.
Reported-by: Yuze Chi <[email protected]>
Signed-off-by: Yuze Chi <[email protected]>
Signed-off-by: Ian Rogers <[email protected]>
---
tools/lib/bpf/libbpf.c | 5 -----
tools/lib/bpf/libbpf_internal.h | 5 +++++
tools/lib/bpf/linker.c | 5 -----
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 3f4f18684bd3..346f941bb995 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4954,11 +4954,6 @@ bpf_object__populate_internal_map(struct bpf_object *obj, struct bpf_map *map)
static void bpf_map__destroy(struct bpf_map *map);
-static bool is_pow_of_2(size_t x)
-{
- return x && (x & (x - 1));
-}
-
static size_t adjust_ringbuf_sz(size_t sz)
{
__u32 page_sz = sysconf(_SC_PAGE_SIZE);
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 4abdbe2fea9d..ef5d975078e5 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -580,4 +580,9 @@ struct bpf_link * usdt_manager_attach_usdt(struct usdt_manager *man,
const char *usdt_provider, const char *usdt_name,
__u64 usdt_cookie);
+static inline bool is_pow_of_2(size_t x)
+{
+ return x && (x & (x - 1)) == 0;
+}
+
#endif /* __LIBBPF_LIBBPF_INTERNAL_H */
diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index 9aa016fb55aa..85c0fddf55d1 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -697,11 +697,6 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
return err;
}
-static bool is_pow_of_2(size_t x)
-{
- return x && (x & (x - 1)) == 0;
-}
-
static int linker_sanity_check_elf(struct src_obj *obj)
{
struct src_sec *sec;
--
2.36.1.255.ge46751e96f-goog
On Fri, Jun 3, 2022 at 9:58 AM Joe Perches <[email protected]> wrote:
>
> On Thu, 2022-06-02 at 22:57 -0700, Ian Rogers wrote:
> > On Thu, Jun 2, 2022 at 10:52 PM Ian Rogers <[email protected]> wrote:
> > > From: Yuze Chi <[email protected]>
> []
> > > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> []
> > > @@ -580,4 +580,9 @@ struct bpf_link * usdt_manager_attach_usdt(struct usdt_manager *man,
> > > const char *usdt_provider, const char *usdt_name,
> > > __u64 usdt_cookie);
> > >
> > > +static inline bool is_pow_of_2(size_t x)
> > > +{
> > > + return x && (x & (x - 1)) == 0;
> > > +}
> > > +
> > > #endif /* __LIBBPF_LIBBPF_INTERNAL_H */
>
> If speed of execution is a potential issue, maybe:
It's not, as it's not in any sort of high-frequency hot path. But even
if it was, we should be careful with __builtin_popcount() because
depending on target CPU architecture __builtin_popcount() can be
turned into a helper function call instead of using hardware
instruction. But either way, keeping it simple is prefereable.
>
> #if __has_builtin(__builtin_popcount)
> return __builtin_popcount(x) == 1;
> #else
> return x && (x & (x-1)) == 0;
> #endif
>
On Thu, 2022-06-02 at 22:57 -0700, Ian Rogers wrote:
> On Thu, Jun 2, 2022 at 10:52 PM Ian Rogers <[email protected]> wrote:
> > From: Yuze Chi <[email protected]>
[]
> > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
[]
> > @@ -580,4 +580,9 @@ struct bpf_link * usdt_manager_attach_usdt(struct usdt_manager *man,
> > const char *usdt_provider, const char *usdt_name,
> > __u64 usdt_cookie);
> >
> > +static inline bool is_pow_of_2(size_t x)
> > +{
> > + return x && (x & (x - 1)) == 0;
> > +}
> > +
> > #endif /* __LIBBPF_LIBBPF_INTERNAL_H */
If speed of execution is a potential issue, maybe:
#if __has_builtin(__builtin_popcount)
return __builtin_popcount(x) == 1;
#else
return x && (x & (x-1)) == 0;
#endif
On Thu, Jun 2, 2022 at 10:52 PM Ian Rogers <[email protected]> wrote:
>
> From: Yuze Chi <[email protected]>
>
> Move the correct definition from linker.c into libbpf_internal.h.
>
Sorry I missed this:
Fixes: 0087a681fa8c ("libbpf: Automatically fix up
BPF_MAP_TYPE_RINGBUF size, if necessary")
Thanks,
Ian
> Reported-by: Yuze Chi <[email protected]>
> Signed-off-by: Yuze Chi <[email protected]>
> Signed-off-by: Ian Rogers <[email protected]>
> ---
> tools/lib/bpf/libbpf.c | 5 -----
> tools/lib/bpf/libbpf_internal.h | 5 +++++
> tools/lib/bpf/linker.c | 5 -----
> 3 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 3f4f18684bd3..346f941bb995 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4954,11 +4954,6 @@ bpf_object__populate_internal_map(struct bpf_object *obj, struct bpf_map *map)
>
> static void bpf_map__destroy(struct bpf_map *map);
>
> -static bool is_pow_of_2(size_t x)
> -{
> - return x && (x & (x - 1));
> -}
> -
> static size_t adjust_ringbuf_sz(size_t sz)
> {
> __u32 page_sz = sysconf(_SC_PAGE_SIZE);
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index 4abdbe2fea9d..ef5d975078e5 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -580,4 +580,9 @@ struct bpf_link * usdt_manager_attach_usdt(struct usdt_manager *man,
> const char *usdt_provider, const char *usdt_name,
> __u64 usdt_cookie);
>
> +static inline bool is_pow_of_2(size_t x)
> +{
> + return x && (x & (x - 1)) == 0;
> +}
> +
> #endif /* __LIBBPF_LIBBPF_INTERNAL_H */
> diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
> index 9aa016fb55aa..85c0fddf55d1 100644
> --- a/tools/lib/bpf/linker.c
> +++ b/tools/lib/bpf/linker.c
> @@ -697,11 +697,6 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> return err;
> }
>
> -static bool is_pow_of_2(size_t x)
> -{
> - return x && (x & (x - 1)) == 0;
> -}
> -
> static int linker_sanity_check_elf(struct src_obj *obj)
> {
> struct src_sec *sec;
> --
> 2.36.1.255.ge46751e96f-goog
>
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <[email protected]>:
On Thu, 2 Jun 2022 22:51:56 -0700 you wrote:
> From: Yuze Chi <[email protected]>
>
> Move the correct definition from linker.c into libbpf_internal.h.
>
> Reported-by: Yuze Chi <[email protected]>
> Signed-off-by: Yuze Chi <[email protected]>
> Signed-off-by: Ian Rogers <[email protected]>
>
> [...]
Here is the summary with links:
- [v2] libbpf: Fix is_pow_of_2
https://git.kernel.org/bpf/bpf-next/c/f913ad6559e3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
On Fri, Jun 3, 2022 at 11:30 AM <[email protected]> wrote:
>
> Hello:
>
> This patch was applied to bpf/bpf-next.git (master)
> by Andrii Nakryiko <[email protected]>:
>
> On Thu, 2 Jun 2022 22:51:56 -0700 you wrote:
> > From: Yuze Chi <[email protected]>
> >
> > Move the correct definition from linker.c into libbpf_internal.h.
> >
> > Reported-by: Yuze Chi <[email protected]>
> > Signed-off-by: Yuze Chi <[email protected]>
> > Signed-off-by: Ian Rogers <[email protected]>
> >
> > [...]
>
> Here is the summary with links:
> - [v2] libbpf: Fix is_pow_of_2
> https://git.kernel.org/bpf/bpf-next/c/f913ad6559e3
>
> You are awesome, thank you!
Will this patch get added to 5.19?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/bpf/libbpf.c#n4948
Thanks,
Ian
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html
>
>
On Tue, Jun 14, 2022 at 1:41 PM Ian Rogers <[email protected]> wrote:
>
> On Fri, Jun 3, 2022 at 11:30 AM <[email protected]> wrote:
> >
> > Hello:
> >
> > This patch was applied to bpf/bpf-next.git (master)
> > by Andrii Nakryiko <[email protected]>:
> >
> > On Thu, 2 Jun 2022 22:51:56 -0700 you wrote:
> > > From: Yuze Chi <[email protected]>
> > >
> > > Move the correct definition from linker.c into libbpf_internal.h.
> > >
> > > Reported-by: Yuze Chi <[email protected]>
> > > Signed-off-by: Yuze Chi <[email protected]>
> > > Signed-off-by: Ian Rogers <[email protected]>
> > >
> > > [...]
> >
> > Here is the summary with links:
> > - [v2] libbpf: Fix is_pow_of_2
> > https://git.kernel.org/bpf/bpf-next/c/f913ad6559e3
> >
> > You are awesome, thank you!
>
> Will this patch get added to 5.19?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/bpf/libbpf.c#n4948
>
I've applied it to bpf-next, so as it stands right now - no. Do you
need this for perf?
> Thanks,
> Ian
>
> > --
> > Deet-doot-dot, I am a bot.
> > https://korg.docs.kernel.org/patchwork/pwbot.html
> >
> >
On Thu, Jun 16, 2022 at 2:00 PM Andrii Nakryiko
<[email protected]> wrote:
>
> On Tue, Jun 14, 2022 at 1:41 PM Ian Rogers <[email protected]> wrote:
> >
> > On Fri, Jun 3, 2022 at 11:30 AM <[email protected]> wrote:
> > >
> > > Hello:
> > >
> > > This patch was applied to bpf/bpf-next.git (master)
> > > by Andrii Nakryiko <[email protected]>:
> > >
> > > On Thu, 2 Jun 2022 22:51:56 -0700 you wrote:
> > > > From: Yuze Chi <[email protected]>
> > > >
> > > > Move the correct definition from linker.c into libbpf_internal.h.
> > > >
> > > > Reported-by: Yuze Chi <[email protected]>
> > > > Signed-off-by: Yuze Chi <[email protected]>
> > > > Signed-off-by: Ian Rogers <[email protected]>
> > > >
> > > > [...]
> > >
> > > Here is the summary with links:
> > > - [v2] libbpf: Fix is_pow_of_2
> > > https://git.kernel.org/bpf/bpf-next/c/f913ad6559e3
> > >
> > > You are awesome, thank you!
> >
> > Will this patch get added to 5.19?
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/bpf/libbpf.c#n4948
> >
>
> I've applied it to bpf-next, so as it stands right now - no. Do you
> need this for perf?
Nope. We carry it as a patch against 5.19 in Google and was surprised
to see I didn't need to drop the patch. Our internal code had
encountered the bug, hence needing the fix. I'd expect others could
encounter it, but I'm unaware of an issue with it and perf.
Thanks,
Ian
> > Thanks,
> > Ian
> >
> > > --
> > > Deet-doot-dot, I am a bot.
> > > https://korg.docs.kernel.org/patchwork/pwbot.html
> > >
> > >
On Thu, Jun 16, 2022 at 2:07 PM Ian Rogers <[email protected]> wrote:
>
> On Thu, Jun 16, 2022 at 2:00 PM Andrii Nakryiko
> <[email protected]> wrote:
> >
> > On Tue, Jun 14, 2022 at 1:41 PM Ian Rogers <[email protected]> wrote:
> > >
> > > On Fri, Jun 3, 2022 at 11:30 AM <[email protected]> wrote:
> > > >
> > > > Hello:
> > > >
> > > > This patch was applied to bpf/bpf-next.git (master)
> > > > by Andrii Nakryiko <[email protected]>:
> > > >
> > > > On Thu, 2 Jun 2022 22:51:56 -0700 you wrote:
> > > > > From: Yuze Chi <[email protected]>
> > > > >
> > > > > Move the correct definition from linker.c into libbpf_internal.h.
> > > > >
> > > > > Reported-by: Yuze Chi <[email protected]>
> > > > > Signed-off-by: Yuze Chi <[email protected]>
> > > > > Signed-off-by: Ian Rogers <[email protected]>
> > > > >
> > > > > [...]
> > > >
> > > > Here is the summary with links:
> > > > - [v2] libbpf: Fix is_pow_of_2
> > > > https://git.kernel.org/bpf/bpf-next/c/f913ad6559e3
> > > >
> > > > You are awesome, thank you!
> > >
> > > Will this patch get added to 5.19?
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/bpf/libbpf.c#n4948
> > >
> >
> > I've applied it to bpf-next, so as it stands right now - no. Do you
> > need this for perf?
>
> Nope. We carry it as a patch against 5.19 in Google and was surprised
> to see I didn't need to drop the patch. Our internal code had
> encountered the bug, hence needing the fix. I'd expect others could
> encounter it, but I'm unaware of an issue with it and perf.
>
So the fix is in Github mirror ([0]) and it is expected that everyone
is using libbpf based on Github repo, so not sure why you'd care about
this fix in bpf tree. I somehow assumed that you need it due to perf,
but was a bit surprised that perf is affected because I don't think
it's using BPF ringbuf.
So I guess the question I have is why you don't use libbpf from [0]
and what can be done to switch to the official libbpf repo?
[0] https://github.com/libbpf/libbpf
> Thanks,
> Ian
>
> > > Thanks,
> > > Ian
> > >
> > > > --
> > > > Deet-doot-dot, I am a bot.
> > > > https://korg.docs.kernel.org/patchwork/pwbot.html
> > > >
> > > >
On Thu, Jun 16, 2022 at 4:55 PM Andrii Nakryiko
<[email protected]> wrote:
>
> On Thu, Jun 16, 2022 at 2:07 PM Ian Rogers <[email protected]> wrote:
> >
> > On Thu, Jun 16, 2022 at 2:00 PM Andrii Nakryiko
> > <[email protected]> wrote:
> > >
> > > On Tue, Jun 14, 2022 at 1:41 PM Ian Rogers <[email protected]> wrote:
> > > >
> > > > On Fri, Jun 3, 2022 at 11:30 AM <[email protected]> wrote:
> > > > >
> > > > > Hello:
> > > > >
> > > > > This patch was applied to bpf/bpf-next.git (master)
> > > > > by Andrii Nakryiko <[email protected]>:
> > > > >
> > > > > On Thu, 2 Jun 2022 22:51:56 -0700 you wrote:
> > > > > > From: Yuze Chi <[email protected]>
> > > > > >
> > > > > > Move the correct definition from linker.c into libbpf_internal.h.
> > > > > >
> > > > > > Reported-by: Yuze Chi <[email protected]>
> > > > > > Signed-off-by: Yuze Chi <[email protected]>
> > > > > > Signed-off-by: Ian Rogers <[email protected]>
> > > > > >
> > > > > > [...]
> > > > >
> > > > > Here is the summary with links:
> > > > > - [v2] libbpf: Fix is_pow_of_2
> > > > > https://git.kernel.org/bpf/bpf-next/c/f913ad6559e3
> > > > >
> > > > > You are awesome, thank you!
> > > >
> > > > Will this patch get added to 5.19?
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/bpf/libbpf.c#n4948
> > > >
> > >
> > > I've applied it to bpf-next, so as it stands right now - no. Do you
> > > need this for perf?
> >
> > Nope. We carry it as a patch against 5.19 in Google and was surprised
> > to see I didn't need to drop the patch. Our internal code had
> > encountered the bug, hence needing the fix. I'd expect others could
> > encounter it, but I'm unaware of an issue with it and perf.
> >
>
> So the fix is in Github mirror ([0]) and it is expected that everyone
> is using libbpf based on Github repo, so not sure why you'd care about
> this fix in bpf tree. I somehow assumed that you need it due to perf,
> but was a bit surprised that perf is affected because I don't think
> it's using BPF ringbuf.
>
> So I guess the question I have is why you don't use libbpf from [0]
> and what can be done to switch to the official libbpf repo?
>
> [0] https://github.com/libbpf/libbpf
Agreed on not following Linus' tree for libbpf. Not sure about having
such an obvious bug in Linus' tree.
Thanks,
Ian
> > Thanks,
> > Ian
> >
> > > > Thanks,
> > > > Ian
> > > >
> > > > > --
> > > > > Deet-doot-dot, I am a bot.
> > > > > https://korg.docs.kernel.org/patchwork/pwbot.html
> > > > >
> > > > >