From: Alan Maguire <alan.maguire@oracle.com>
To: Eduard Zingerman <eddyz87@gmail.com>,
andrii@kernel.org, jolsa@kernel.org, acme@redhat.com,
quentin@isovalent.com
Cc: mykolal@fb.com, ast@kernel.org, daniel@iogearbox.net,
martin.lau@linux.dev, song@kernel.org, yonghong.song@linux.dev,
john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
haoluo@google.com, houtao1@huawei.com, bpf@vger.kernel.org,
masahiroy@kernel.org, mcgrof@kernel.org, nathan@kernel.org
Subject: Re: [PATCH v3 bpf-next 01/11] libbpf: add btf__distill_base() creating split BTF with distilled base BTF
Date: Mon, 13 May 2024 18:23:53 +0100 [thread overview]
Message-ID: <57424cd8-c656-4ca0-80fb-288f83156ec9@oracle.com> (raw)
In-Reply-To: <cdc19260a1d1e37649f64bab731c21cb4e5422ff.camel@gmail.com>
On 10/05/2024 20:14, Eduard Zingerman wrote:
> On Fri, 2024-05-10 at 11:30 +0100, Alan Maguire wrote:
>
> [...]
>
> Hi Alan,
>
> A two minor notes below, otherwise I think this looks good.
>
thanks for reviewing! Replies below..
> [...]
>
>> +static int btf_add_distilled_type_ids(__u32 *id, void *ctx)
>> +{
>> + struct btf_distill *dist = ctx;
>> + struct btf_type *t = btf_type_by_id(dist->pipe.src, *id);
>> + int err;
>> +
>> + if (!*id)
>> + return 0;
>> + /* split BTF id, not needed */
>> + if (*id >= dist->split_start_id)
>> + return 0;
>> + /* already added ? */
>> + if (BTF_ID(dist->ids[*id]) > 0)
>> + return 0;
>> +
>> + /* only a subset of base BTF types should be referenced from split
>> + * BTF; ensure nothing unexpected is referenced.
>> + */
>> + switch (btf_kind(t)) {
>> + case BTF_KIND_INT:
>> + case BTF_KIND_FLOAT:
>> + case BTF_KIND_FWD:
>> + case BTF_KIND_ARRAY:
>> + case BTF_KIND_STRUCT:
>> + case BTF_KIND_UNION:
>> + case BTF_KIND_TYPEDEF:
>> + case BTF_KIND_ENUM:
>> + case BTF_KIND_ENUM64:
>> + case BTF_KIND_PTR:
>> + case BTF_KIND_CONST:
>> + case BTF_KIND_RESTRICT:
>> + case BTF_KIND_VOLATILE:
>> + case BTF_KIND_FUNC_PROTO:
>
> I think BTF_KIND_TYPE_TAG should be in this list.
>
You're right; sorry, you mentioned that last time too and I missed
fixing it for v3.
>> + dist->ids[*id] |= *id;
>> + break;
>> + default:
>> + pr_warn("unexpected reference to base type[%u] of kind [%u] when creating distilled base BTF.\n",
>> + *id, btf_kind(t));
>> + return -EINVAL;
>> + }
>
> [...]
>
>> +static int btf_add_distilled_types(struct btf_distill *dist)
>> +{
>> + bool adding_to_base = dist->pipe.dst->start_id == 1;
>> + int id = btf__type_cnt(dist->pipe.dst);
>> + struct btf_type *t;
>> + int i, err = 0;
>> +
>> + /* Add types for each of the required references to either distilled
>> + * base or split BTF, depending on type characteristics.
>> + */
>> + for (i = 1; i < dist->split_start_id; i++) {
>> + const char *name;
>> + int kind;
>> +
>> + if (!BTF_ID(dist->ids[i]))
>> + continue;
>> + t = btf_type_by_id(dist->pipe.src, i);
>> + kind = btf_kind(t);
>> + name = btf__name_by_offset(dist->pipe.src, t->name_off);
>> +
>> + /* Named int, float, fwd struct, union, enum[64] are added to
>> + * base; everything else is added to split BTF.
>> + */
>> + switch (kind) {
>> + case BTF_KIND_INT:
>> + case BTF_KIND_FLOAT:
>> + case BTF_KIND_FWD:
>> + case BTF_KIND_STRUCT:
>> + case BTF_KIND_UNION:
>> + case BTF_KIND_ENUM:
>> + case BTF_KIND_ENUM64:
>> + if ((adding_to_base && !t->name_off) || (!adding_to_base && t->name_off))
>> + continue;
>> + break;
>> + default:
>> + if (adding_to_base)
>> + continue;
>> + break;
>> + }
>> + if (dist->ids[i] & BTF_EMBEDDED_COMPOSITE) {
>> + /* If a named struct/union in base BTF is referenced as a type
>> + * in split BTF without use of a pointer - i.e. as an embedded
>> + * struct/union - add an empty struct/union preserving size
>> + * since size must be consistent when relocating split and
>> + * possibly changed base BTF.
>> + */
>> + err = btf_add_composite(dist->pipe.dst, kind, name, t->size);
>> + } else if (btf_is_eligible_named_fwd(t)) {
>> + /* If not embedded, use a fwd for named struct/unions since we
>> + * can match via name without any other details.
>> + */
>> + switch (kind) {
>> + case BTF_KIND_STRUCT:
>> + err = btf__add_fwd(dist->pipe.dst, name, BTF_FWD_STRUCT);
>> + break;
>> + case BTF_KIND_UNION:
>> + err = btf__add_fwd(dist->pipe.dst, name, BTF_FWD_UNION);
>> + break;
>> + case BTF_KIND_ENUM:
>> + err = btf__add_enum(dist->pipe.dst, name, sizeof(int));
>
> I think that the size of the enum should be read from base BTF.
> When inspecting BTF generated for selftests kernel config there
> are 14 enums with size=1.
>
good idea; we can use t->size for both enum and enum64 cases above.
>> + break;
>> + case BTF_KIND_ENUM64:
>> + err = btf__add_enum(dist->pipe.dst, name, sizeof(__u64));
>> + break;
>> + default:
>> + pr_warn("unexpected kind [%u] when creating distilled base BTF.\n",
>> + btf_kind(t));
>> + return -EINVAL;
>> + }
>> + } else {
>> + err = btf_add_type(&dist->pipe, t);
>> + }
>> + if (err < 0)
>> + break;
>> + dist->ids[i] = id++;
>> + }
>> + return err;
>> +}
>
> [...]
next prev parent reply other threads:[~2024-05-13 17:39 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-10 10:30 [PATCH v3 bpf-next 00/11] bpf: support resilient split BTF Alan Maguire
2024-05-10 10:30 ` [PATCH v3 bpf-next 01/11] libbpf: add btf__distill_base() creating split BTF with distilled base BTF Alan Maguire
2024-05-10 19:14 ` Eduard Zingerman
2024-05-13 17:23 ` Alan Maguire [this message]
2024-05-10 10:30 ` [PATCH v3 bpf-next 02/11] selftests/bpf: test distilled base, split BTF generation Alan Maguire
2024-05-10 10:30 ` [PATCH v3 bpf-next 03/11] libbpf: add btf__parse_opts() API for flexible BTF parsing Alan Maguire
2024-05-11 9:40 ` Eduard Zingerman
2024-05-13 16:25 ` Alan Maguire
2024-05-13 16:59 ` Eduard Zingerman
2024-05-10 10:30 ` [PATCH v3 bpf-next 04/11] bpftool: support displaying raw split BTF using base BTF section as base Alan Maguire
2024-05-13 10:57 ` Quentin Monnet
2024-05-10 10:30 ` [PATCH v3 bpf-next 05/11] resolve_btfids: use .BTF.base ELF section as base BTF if -B option is used Alan Maguire
2024-05-10 10:30 ` [PATCH v3 bpf-next 06/11] kbuild, bpf: add module-specific pahole/resolve_btfids flags for distilled base BTF Alan Maguire
2024-05-10 10:30 ` [PATCH v3 bpf-next 07/11] libbpf: split BTF relocation Alan Maguire
2024-05-10 22:26 ` Eduard Zingerman
2024-05-13 17:51 ` Alan Maguire
2024-05-10 10:30 ` [PATCH v3 bpf-next 08/11] selftests/bpf: extend distilled BTF tests to cover " Alan Maguire
2024-05-10 22:46 ` Eduard Zingerman
2024-05-10 10:30 ` [PATCH v3 bpf-next 09/11] module, bpf: store BTF base pointer in struct module Alan Maguire
2024-05-10 10:30 ` [PATCH v3 bpf-next 10/11] libbpf,bpf: share BTF relocate-related code with kernel Alan Maguire
2024-05-11 1:46 ` Eduard Zingerman
2024-05-14 16:14 ` Alan Maguire
2024-05-15 6:56 ` Eduard Zingerman
2024-05-10 10:30 ` [PATCH v3 bpf-next 11/11] bpftool: support displaying relocated-with-base split BTF Alan Maguire
2024-05-11 9:32 ` Eduard Zingerman
2024-05-13 11:12 ` Quentin Monnet
2024-05-14 16:33 ` Alan Maguire
2024-05-11 9:28 ` [PATCH v3 bpf-next 00/11] bpf: support resilient " Eduard Zingerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57424cd8-c656-4ca0-80fb-288f83156ec9@oracle.com \
--to=alan.maguire@oracle.com \
--cc=acme@redhat.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=haoluo@google.com \
--cc=houtao1@huawei.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=martin.lau@linux.dev \
--cc=masahiroy@kernel.org \
--cc=mcgrof@kernel.org \
--cc=mykolal@fb.com \
--cc=nathan@kernel.org \
--cc=quentin@isovalent.com \
--cc=sdf@google.com \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).