[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aP_qqVvDjB99NQNk@x1>
Date: Mon, 27 Oct 2025 18:56:57 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Jiayuan Chen <jiayuan.chen@...ux.dev>,
Alan Maguire <alan.maguire@...cle.com>,
Anton Protopopov <a.s.protopopov@...il.com>,
Yonghong Song <yonghong.song@...ux.dev>, bpf <bpf@...r.kernel.org>,
Andrii Nakryiko <andrii@...nel.org>,
Eduard Zingerman <eddyz87@...il.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
Shuah Khan <shuah@...nel.org>,
Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
Bill Wendling <morbo@...gle.com>,
Justin Stitt <justinstitt@...gle.com>,
Puranjay Mohan <puranjay@...nel.org>,
"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
clang-built-linux <llvm@...ts.linux.dev>
Subject: Re: pahole next->master. Was: [PATCH bpf-next v1] selftests/bpf:
Guard addr_space_cast code with __BPF_FEATURE_ADDR_SPACE_CAST
On Mon, Oct 27, 2025 at 12:09:11PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Oct 27, 2025 at 11:58:43AM -0300, Arnaldo Carvalho de Melo wrote:
> > On Thu, Oct 23, 2025 at 08:42:34AM -0700, Alexei Starovoitov wrote:
> > > On Thu, Oct 23, 2025 at 12:50 AM Jiayuan Chen <jiayuan.chen@...ux.dev> wrote:
> > > > thanks, but version 1.30 didn't work in my tests - even pahole's master branch fails, only the next branch works...
> >
> > > > It seems that the 'old' pahole parses some kfuncs incorrectly, for example bpf_dynptr_slice().
> > > the introduction of the 'next' branch screwed up the workflow for many people.
> > > Let's remove it and merge everything into master.
> > > People expect master branch to be the one where active development
> > > is happening and the source of truth for the latest features.
> > My bad, I've been away for too long, next is supposed to be with things
> > for a short while, testing for a few days, for CI consumption, then move
> > to master, rinse repeat.
> > I think we should go back to that model.
> The difference is small but can explain as has changes to the btf
> loader, and the reporter, as I now checked the whole thread, says that
> 'next' works for him, so I'll move what is in 'next' to 'master' now.
> Just for reference since I had done it, my investigation is below.
So I had some random 'korg/next' local branch and a 'korg' remote and
that messed up my analysis, there are more csets in the 'next' branch,
so I'm doing more tests, tomorrow I'll probably have more news.
- Arnaldo
>
> ⬢ [acme@...lbx pahole]$ git remote update korg
> Fetching korg
> ⬢ [acme@...lbx pahole]$
> ⬢ [acme@...lbx pahole]$ git remote -v | grep korg
> korg https://git.kernel.org/pub/scm/devel/pahole/pahole.git (fetch)
> korg https://git.kernel.org/pub/scm/devel/pahole/pahole.git (push)
> ⬢ [acme@...lbx pahole]$ git diff --stat korg/master korg/next
> warning: refname 'korg/next' is ambiguous.
> .github/scripts/build-pahole.sh | 23 +++++++++++++++++++++++
> .github/scripts/compare-functions.sh | 30 ++++++++++++++++++++++++++++++
> .github/workflows/test.yml | 4 ++--
> .github/workflows/vmtest.yml | 4 ++++
> CMakeLists.txt | 5 -----
> README | 4 ++++
> btf_loader.c | 23 ++++++++++++++++++++---
> dwarves_fprintf.c | 2 +-
> 8 files changed, 84 insertions(+), 11 deletions(-)
> ⬢ [acme@...lbx pahole]$
> Related to btf bitfields:
> diff --git a/btf_loader.c b/btf_loader.c
> index f4f9f65289b5acac..64ea68022ab04e60 100644
> --- a/btf_loader.c
> +++ b/btf_loader.c
> @@ -645,9 +645,15 @@ static int class__fixup_btf_bitfields(const struct conf_load *conf, struct tag *
> pos->byte_size = tag__size(type, cu);
> pos->bit_size = pos->byte_size * 8;
>
> - /* if BTF data is incorrect and has size == 0, skip field,
> - * instead of crashing */
> + /* If the BTF data is incorrect and has size == 0, skip field
> + * instead of crashing. However the field can be a zero or
> + * variable-length array and we still need to infer alignment.
> + */
> if (pos->byte_size == 0) {
> + pos->alignment = class__infer_alignment(conf,
> + pos->byte_offset,
> + tag__natural_alignment(type, cu),
> + smallest_offset);
> continue;
> }
>
> @@ -672,7 +678,18 @@ static int class__fixup_btf_bitfields(const struct conf_load *conf, struct tag *
> pos->byte_offset,
> tag__natural_alignment(type, cu),
> smallest_offset);
> - smallest_offset = pos->byte_offset + pos->byte_size;
> +
> + /* Compute the smallest offset between this field and the next
> + * one.
> + *
> + * In case of bitfields we need to take into account the
> + * actual size being used instead of the underlying type one as
> + * it could be larger, otherwise we could miss a hole.
> + */
> + smallest_offset = pos->byte_offset;
> + smallest_offset += pos->bitfield_size ?
> + (pos->bitfield_offset + pos->bitfield_size + 7) / 8 :
> + pos->byte_size;
> }
>
> tag_type->alignment = class__infer_alignment(conf,
Powered by blists - more mailing lists