All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: Xin Liu <liuxin350@huawei.com>,
	ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	martin.lau@linux.dev, song@kernel.org, yhs@fb.com,
	john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
	haoluo@google.com, jolsa@kernel.org
Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
	yanan@huawei.com, wuchangye@huawei.com, xiesongyang@huawei.com,
	kongweibin2@huawei.com, zhangmingyi5@huawei.com,
	liwei883@huawei.com
Subject: Re: [PATCH] libbpf: extending BTF_KIND_INIT to accommodate some unusual types
Date: Tue, 23 Apr 2024 13:12:04 -0700	[thread overview]
Message-ID: <a79006a7-a5af-4e30-8424-d54145bcd538@linux.dev> (raw)
In-Reply-To: <20240423131503.361149-1-liuxin350@huawei.com>


On 4/23/24 6:15 AM, Xin Liu wrote:
> On Mon, 22 Apr 2024 10:43:38 -0700 Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
>> On Mon, Apr 22, 2024 at 7:46 AM Xin Liu <liuxin350@huawei.com> wrote:
>>> In btf__add_int, the size of the new btf_kind_int type is limited.
>>> When the size is greater than 16, btf__add_int fails to be added
>>> and -EINVAL is returned. This is usually effective.
>>>
>>> However, when the built-in type __builtin_aarch64_simd_xi in the
>>> NEON instruction is used in the code in the arm64 system, the value
>>> of DW_AT_byte_size is 64. This causes btf__add_int to fail to
>>> properly add btf information to it.
>>>
>>> like this:
>>>    ...
>>>     <1><cf>: Abbrev Number: 2 (DW_TAG_base_type)
>>>      <d0>   DW_AT_byte_size   : 64              // over max size 16
>>>      <d1>   DW_AT_encoding    : 5        (signed)
>>>      <d2>   DW_AT_name        : (indirect string, offset: 0x53): __builtin_aarch64_simd_xi
>>>     <1><d6>: Abbrev Number: 0
>>>    ...
>>>
>>> An easier way to solve this problem is to treat it as a base type
>>> and set byte_size to 64. This patch is modified along these lines.
>>>
>>> Fixes: 4a3b33f8579a ("libbpf: Add BTF writing APIs")
>>> Signed-off-by: Xin Liu <liuxin350@huawei.com>
>>> ---
>>>   tools/lib/bpf/btf.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
>>> index 2d0840ef599a..0af121293b65 100644
>>> --- a/tools/lib/bpf/btf.c
>>> +++ b/tools/lib/bpf/btf.c
>>> @@ -1934,7 +1934,7 @@ int btf__add_int(struct btf *btf, const char *name, size_t byte_sz, int encoding
>>>          if (!name || !name[0])
>>>                  return libbpf_err(-EINVAL);
>>>          /* byte_sz must be power of 2 */
>>> -       if (!byte_sz || (byte_sz & (byte_sz - 1)) || byte_sz > 16)
>>> +       if (!byte_sz || (byte_sz & (byte_sz - 1)) || byte_sz > 64)
>>
>> maybe we should just remove byte_sz upper limit? We can probably
>> imagine 256-byte integers at some point, so why bother artificially
>> restricting it?
>>
>> pw-bot: cr
> In the current definition of btf_kind_int, bits has only 8 bits, followed
> by 8 bits of unused interval. When we expand, we should only use 16 bits
> at most, so the maximum value should be 8192(1 << 16 / 8), directly removing
> the limit of byte_sz. It may not fit the current design. For INT type btfs
> greater than 255, how to dump is still a challenge.

Looking at this patch. Now I remember that I have an old pahole patch
to address similar issues
   https://lore.kernel.org/bpf/20230426055030.3743074-1-yhs@fb.com/
which is not merged and I forgot that.

In that particular case, the int size is 1024 bytes.
Currently the int type more than 16 bytes cannot be dumped in libbpf.
Do you have a particular use case to use your__builtin_aarch64_simd_xi() type
in bpf program? I guess probably not as BPF does not support
builtin function your__builtin_aarch64_simd_xi().

>
> Does the current version support a maximum of 8192 bytes?
>
>>>                  return libbpf_err(-EINVAL);
>>>          if (encoding & ~(BTF_INT_SIGNED | BTF_INT_CHAR | BTF_INT_BOOL))
>>>                  return libbpf_err(-EINVAL);
>>> --
>>> 2.33.0
>>>

  parent reply	other threads:[~2024-04-23 20:12 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-23 13:15 [PATCH] libbpf: extending BTF_KIND_INIT to accommodate some unusual types Xin Liu
2024-04-23 14:30 ` Alan Maguire
2024-04-24  6:52   ` Xin Liu
2024-04-23 20:12 ` Yonghong Song [this message]
2024-04-24  7:06   ` Xin Liu
2024-04-24 22:11     ` Yonghong Song
  -- strict thread matches above, loose matches on Subject: below --
2024-04-22 14:45 Xin Liu
2024-04-22 17:43 ` Andrii Nakryiko

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=a79006a7-a5af-4e30-8424-d54145bcd538@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kongweibin2@huawei.com \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuxin350@huawei.com \
    --cc=liwei883@huawei.com \
    --cc=martin.lau@linux.dev \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=wuchangye@huawei.com \
    --cc=xiesongyang@huawei.com \
    --cc=yanan@huawei.com \
    --cc=yhs@fb.com \
    --cc=zhangmingyi5@huawei.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.