public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: Ihor Solodrai <ihor.solodrai@linux.dev>
To: Chengkaitao <pilgrimtao@gmail.com>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Martin KaFai Lau <martin.lau@linux.dev>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>, Eduard <eddyz87@gmail.com>,
	Song Liu <song@kernel.org>,
	Yonghong Song <yonghong.song@linux.dev>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@fomichev.me>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>, Shuah Khan <shuah@kernel.org>,
	Chengkaitao <chengkaitao@kylinos.cn>,
	"open list:KERNEL SELFTEST FRAMEWORK"
	<linux-kselftest@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH bpf-next v9 1/9] bpf: refactor kfunc checks using table-driven approach in verifier
Date: Fri, 3 Apr 2026 21:49:19 -0700	[thread overview]
Message-ID: <1fcda49a-caeb-42b9-9727-d1ab3d07d84b@linux.dev> (raw)
In-Reply-To: <CAAWJmAYxbEX0M3JFJn11WOfvRoLrxnuXtkDp5omt-FJYwc8mpg@mail.gmail.com>



On 4/3/26 10:41 AM, Chengkaitao wrote:
> On Tue, Mar 31, 2026 at 1:05 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
>> On Sun, Mar 29, 2026 at 7:05 AM Chengkaitao <pilgrimtao@gmail.com> wrote:
>>>
>>> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>>>
>>> [...]
>>> +
>>> +/* Kfunc family related to spin_lock. */
>>> +static const enum special_kfunc_type bpf_res_spin_lock_api_kfuncs[] = {
>>> +       KF_bpf_res_spin_lock,
>>> +       KF_bpf_res_spin_unlock,
>>> +       KF_bpf_res_spin_lock_irqsave,
>>> +       KF_bpf_res_spin_unlock_irqrestore,
>>> +};
>>
>> I think it's a step in the wrong direction.
>> I'd wait for Ihor's BTF_ID_NAMED cleanup.
> 
> After reading Ihor's messages on the list, if I understand correctly,
> our two approaches seem to target different problems. What Ihor's
> work appears to achieve is the ability to remove the entire enum
> special_kfunc_type. My goal, on the other hand, is to replace many
> scattered func_id == special_kfunc_list[...] comparisons with a
> table-driven approach.

Hi Kaitao,

I appreciate your efforts, however after a quick pass over the changes
you propose (both here and in the new series) with respect to BTF_ID
macros and special_kfuncs_list, I don't understand what problem you're
trying to solve.

The inherent complexity is in the fact that the verifier must know
when a particular BTF id identifies a specific kfunc, or whether it
belongs to some pre-defined set of ids. This is why
special_kfuncs_list and other BTF_ID_SET/LIST-s exist.

And so there is no way around defining those ids and sets *somewhere*,
and so far BTF_ID_* macros did a fine job of that, all things
considered.

AFAICT your changes simply move around the same definitions from
functions with if statements to constant arrays with a runtime search
on them (which is slower by the way). What is the benefit of that vs
the current implementation? We still have to maintain those arrays in
the same way we have to maintain the is_foo_kfunc helpers.

Your newer proposal [1] takes the same idea to the next level, by
introducing an entire new BTF kind, new ELF sections and a bunch of
macros that are no less complicated than existing. And all of that
just moves the same arrays "upstream" to the .BTF_ids section. Again,
I fail to see any benefits to that complexity. Having differentiation
between LIST and SET, and having to mark START and END is not a
problem that needs solving IMO.

The work I was discussing with Alexei [2] is targeted: get rid of
constant enums that mirror existing kfunc symbols by making BTF_ID
internals a bit smarter. And the benefit is clear and simple.

If you could explain what exact issue you're trying to address with
your BTF_ID refactoring patches, maybe we can try converging on a
reasonable approach to it.

Thanks!

[1] https://lore.kernel.org/bpf/20260403170900.58659-1-pilgrimtao@gmail.com/
[2] https://lore.kernel.org/bpf/21e5333c-0b57-46ce-99c8-f6c414270e70@linux.dev/


> 
> That said, once Ihor's solution lands, my approach would no longer
> be applicable as-is. So I have been thinking about an alternative,
> please see the patch linked below for details:
> https://lore.kernel.org/bpf/20260403170900.58659-1-pilgrimtao@gmail.com/
> 
>> Kaitao Cheng,
>>
>> also please start your part of code reviews.
>> Your patches are not going to be landing if you don't code review.
>>
>> https://lore.kernel.org/bpf/CAADnVQ+TKKptnNB25V3=bcdybh5G6c2DyW2sYtXvyRaVnPN8MA@mail.gmail.com/
> 
> I will do my best to carry this forward.
> 


  reply	other threads:[~2026-04-04  4:49 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-29 14:04 [PATCH bpf-next v9 0/9] bpf: Extend the bpf_list family of APIs Chengkaitao
2026-03-29 14:04 ` [PATCH bpf-next v9 1/9] bpf: refactor kfunc checks using table-driven approach in verifier Chengkaitao
2026-03-30 15:20   ` Mykyta Yatsenko
2026-03-30 17:05   ` Alexei Starovoitov
2026-04-03 17:41     ` Chengkaitao
2026-04-04  4:49       ` Ihor Solodrai [this message]
2026-04-04 10:38         ` Chengkaitao
2026-03-29 14:04 ` [PATCH bpf-next v9 2/9] bpf: refactor __bpf_list_del to take list node pointer Chengkaitao
2026-03-29 14:05 ` [PATCH bpf-next v9 3/9] bpf: clear list node owner and unlink before drop Chengkaitao
2026-03-29 14:45   ` bot+bpf-ci
2026-03-29 14:05 ` [PATCH bpf-next v9 4/9] bpf: Introduce the bpf_list_del kfunc Chengkaitao
2026-03-29 14:05 ` [PATCH bpf-next v9 5/9] bpf: refactor __bpf_list_add to take insertion point via **prev_ptr Chengkaitao
2026-03-29 14:05 ` [PATCH bpf-next v9 6/9] bpf: Add bpf_list_add_impl to insert node after a given list node Chengkaitao
2026-03-29 14:05 ` [PATCH bpf-next v9 7/9] bpf: allow bpf_list_front/back result as the prev argument of bpf_list_add_impl Chengkaitao
2026-03-29 14:05 ` [PATCH bpf-next v9 8/9] bpf: add bpf_list_is_first/last/empty kfuncs Chengkaitao
2026-03-29 14:05 ` [PATCH bpf-next v9 9/9] selftests/bpf: Add test cases for bpf_list_del/add/is_first/is_last/empty Chengkaitao

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=1fcda49a-caeb-42b9-9727-d1ab3d07d84b@linux.dev \
    --to=ihor.solodrai@linux.dev \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=chengkaitao@kylinos.cn \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=pilgrimtao@gmail.com \
    --cc=sdf@fomichev.me \
    --cc=shuah@kernel.org \
    --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