From: Song Liu <songliubraving@fb.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Andrii Nakryiko <andriin@fb.com>, bpf <bpf@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
Alexei Starovoitov <ast@fb.com>,
"daniel@iogearbox.net" <daniel@iogearbox.net>,
Yonghong Song <yhs@fb.com>, Kernel Team <Kernel-team@fb.com>
Subject: Re: [PATCH v2 bpf-next 02/12] libbpf: implement BPF CO-RE offset relocation algorithm
Date: Wed, 31 Jul 2019 08:29:56 +0000 [thread overview]
Message-ID: <4D2E1082-5013-4A50-B75D-AB88FDCAAC52@fb.com> (raw)
In-Reply-To: <CAEf4Bza3cAoZJE+24_MBiv-8yYtAaTkAez5xq1v12cLW1-RGcw@mail.gmail.com>
> On Jul 30, 2019, at 11:52 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Jul 30, 2019 at 10:19 PM Song Liu <songliubraving@fb.com> wrote:
>>
>>
>>
>>> On Jul 30, 2019, at 6:00 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>>>
>>> On Tue, Jul 30, 2019 at 5:39 PM Song Liu <songliubraving@fb.com> wrote:
>>>>
>>>>
>>>>
>>>>> On Jul 30, 2019, at 12:53 PM, Andrii Nakryiko <andriin@fb.com> wrote:
>>>>>
>>>>> This patch implements the core logic for BPF CO-RE offsets relocations.
>>>>> Every instruction that needs to be relocated has corresponding
>>>>> bpf_offset_reloc as part of BTF.ext. Relocations are performed by trying
>>>>> to match recorded "local" relocation spec against potentially many
>>>>> compatible "target" types, creating corresponding spec. Details of the
>>>>> algorithm are noted in corresponding comments in the code.
>>>>>
>>>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
[...]
>>>>
>>>
>>> I just picked the most succinct and non-repetitive form. It's
>>> immediately apparent which type it's implicitly converted to, so I
>>> felt there is no need to repeat it. Also, just (void *) is much
>>> shorter. :)
>>
>> _All_ other code in btf.c converts the pointer to the target type.
>
> Most in libbpf.c doesn't, though. Also, I try to preserve pointer
> constness for uses that don't modify BTF types (pretty much all of
> them in libbpf), so it becomes really verbose, despite extremely short
> variable names:
>
> const struct btf_member *m = (const struct btf_member *)(t + 1);
I don't think being verbose is a big problem here. Overusing
(void *) feels like a bigger problem.
>
> Add one or two levels of nestedness and you are wrapping this line.
>
>> In some cases, it is not apparent which type it is converted to,
>> for example:
>>
>> + m = (void *)(targ_type + 1);
>>
>> I would suggest we do implicit conversion whenever possible.
>
> Implicit conversion (`m = targ_type + 1;`) is a compilation error,
> that won't work.
I misused "implicit" here. I actually meant to say
m = ((const struct btf_member *)(t + 1);
next prev parent reply other threads:[~2019-07-31 8:30 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-30 19:53 [PATCH v2 bpf-next 00/12] CO-RE offset relocations Andrii Nakryiko
2019-07-30 19:53 ` [PATCH v2 bpf-next 01/12] libbpf: add .BTF.ext offset relocation section loading Andrii Nakryiko
2019-07-30 21:19 ` Song Liu
2019-07-30 19:53 ` [PATCH v2 bpf-next 02/12] libbpf: implement BPF CO-RE offset relocation algorithm Andrii Nakryiko
2019-07-30 23:43 ` Song Liu
2019-07-30 23:55 ` Andrii Nakryiko
2019-07-31 0:16 ` Song Liu
2019-07-31 0:39 ` Song Liu
2019-07-31 1:00 ` Andrii Nakryiko
2019-07-31 5:19 ` Song Liu
2019-07-31 6:52 ` Andrii Nakryiko
2019-07-31 8:29 ` Song Liu [this message]
2019-07-31 17:18 ` Andrii Nakryiko
2019-07-31 17:46 ` Song Liu
2019-07-30 19:53 ` [PATCH v2 bpf-next 03/12] selftests/bpf: add BPF_CORE_READ relocatable read macro Andrii Nakryiko
2019-07-30 21:24 ` Song Liu
2019-07-30 21:26 ` Andrii Nakryiko
2019-07-30 21:33 ` Song Liu
2019-07-30 19:54 ` [PATCH v2 bpf-next 04/12] selftests/bpf: add CO-RE relocs testing setup Andrii Nakryiko
2019-07-30 19:54 ` [PATCH v2 bpf-next 05/12] selftests/bpf: add CO-RE relocs struct flavors tests Andrii Nakryiko
2019-07-30 19:54 ` [PATCH v2 bpf-next 06/12] selftests/bpf: add CO-RE relocs nesting tests Andrii Nakryiko
2019-07-30 19:54 ` [PATCH v2 bpf-next 07/12] selftests/bpf: add CO-RE relocs array tests Andrii Nakryiko
2019-07-30 19:54 ` [PATCH v2 bpf-next 08/12] selftests/bpf: add CO-RE relocs enum/ptr/func_proto tests Andrii Nakryiko
2019-07-30 19:54 ` [PATCH v2 bpf-next 09/12] selftests/bpf: add CO-RE relocs modifiers/typedef tests Andrii Nakryiko
2019-07-30 19:54 ` [PATCH v2 bpf-next 10/12] selftests/bpf: add CO-RE relocs ptr-as-array tests Andrii Nakryiko
2019-07-30 19:54 ` [PATCH v2 bpf-next 11/12] selftests/bpf: add CO-RE relocs ints tests Andrii Nakryiko
2019-07-30 19:54 ` [PATCH v2 bpf-next 12/12] selftests/bpf: add CO-RE relocs misc tests 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=4D2E1082-5013-4A50-B75D-AB88FDCAAC52@fb.com \
--to=songliubraving@fb.com \
--cc=Kernel-team@fb.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andriin@fb.com \
--cc=ast@fb.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=netdev@vger.kernel.org \
--cc=yhs@fb.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox