mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Menglong Dong <menglong.dong@linux.dev>
To: Qiliang Yuan <realwujing@gmail.com>
Cc: alexei.starovoitov@gmail.com, andrii@kernel.org, ast@kernel.org,
	bpf@vger.kernel.org, daniel@iogearbox.net, eddyz87@gmail.com,
	haoluo@google.com, john.fastabend@gmail.com, jolsa@kernel.org,
	kpsingh@kernel.org, linux-kernel@vger.kernel.org,
	martin.lau@linux.dev, memxor@gmail.com, realwujing@gmail.com,
	realwujing@qq.com, sdf@fomichev.me, song@kernel.org,
	yonghong.song@linux.dev, yuanql9@chinatelecom.cn
Subject: Re: [PATCH v2] bpf/verifier: implement slab cache for verifier state list
Date: Sat, 17 Jan 2026 19:27:53 +0800	[thread overview]
Message-ID: <6205668.MhkbZ0Pkbq@7950hx> (raw)
In-Reply-To: <20260117032612.10008-1-yuanql9@chinatelecom.cn>

On 2026/1/17 11:26, Qiliang Yuan wrote:
> On Fri, 16 Jan 2026 22:50:36 +0800, Menglong Dong <menglong.dong@linux.dev> wrote:
> > On 2026/1/16 21:29, Qiliang Yuan wrote:
> > > The BPF verifier's state exploration logic in is_state_visited() frequently
> > > allocates and deallocates 'struct bpf_verifier_state_list' nodes. Currently,
> > > these allocations use generic kzalloc(), which leads to significant memory
> > > management overhead and page faults during high-complexity verification,
> > > especially in multi-core parallel scenarios.
> > > 
> > > This patch introduces a dedicated 'bpf_verifier_state_list' slab cache to
> > > optimize these allocations, providing better speed, reduced fragmentation,
> > > and improved cache locality. All allocation and deallocation paths are
> > > migrated to use kmem_cache_zalloc() and kmem_cache_free().
> > > 
> > > Performance evaluation using a stress test (1000 conditional branches)
> > > executed in parallel on 32 CPU cores for 60 seconds shows significant
> > > improvements:
> > 
> > This patch is a little mess. First, don't send a new version by replying to
> > your previous version.
> 
> Hi Menglong,
> 
> Congratulations on obtaining your @linux.dev email! It is great to see your
> contribution to the community being recognized.
> 
> The core logic remains unchanged. Following suggestions from several
> reviewers, I've added the perf benchmark data and sent this as a reply to the
> previous thread to keep the context and review history easier to track.

You can put the link of your previous version to the change log. I
suspect the patchwork can't even detect this new version if you send
it as a reply.

> 
> > > Metric              | Baseline      | Patched       | Delta (%)
> > > --------------------|---------------|---------------|----------
> > > Page Faults         | 12,377,064    | 8,534,044     | -31.05%
> > > IPC                 | 1.17          | 1.22          | +4.27%
> > > CPU Cycles          | 1,795.37B     | 1,700.33B     | -5.29%
> > > Instructions        | 2,102.99B     | 2,074.27B     | -1.37%
> > 
> > And the test case is odd too. What performance improvement do we
> > get from this testing result? You run the veristat infinitely and record the
> > performance with perf for 60s, so what can we get? Shouldn't you
> > run the veristat for certain times and see the performance, such as
> > the duration or the CPU cycles?
> 
> Following suggestions from several reviewers, I aimed to provide perf
> benchmark data for comparison. However, existing veristat tests do not
> frequently trigger the specific state list allocation paths I modified. This
> is why I constructed a dedicated stress test and included the code in the
> commit message to clearly demonstrate the performance gains.
> 
> > You optimize the verifier to reduce the verifying duration in your case,
> > which seems to be a complex BPF program and consume much time
> > in verifier. So what performance increasing do you get in your case?
> 
> The performance gains are primarily seen in the 31.05% reduction in Page Faults
> and the 4.27% increase in IPC. These metrics indicate that moving to a
> dedicated slab cache significantly reduces memory management overhead and
> improves instruction throughput. Specifically, the reduction in CPU cycles
> (-5.29%) confirms that the verifier spends less time on internal allocation
> logic, which is crucial for complex BPF programs that involve deep state
> exploration.

You introduce the slab cache to speed up the verifier, so I think we need
a comparison, such as how long a complex BPF program can take in
the verifier. If it is no more than 1ms, then I think it doesn't make much
sense to obtain the 5% speeding up. After all, it's not a BPF runtime
overhead.

> 
> > > Suggested-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > > Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
> > 
> > You don't need to add all the reviewers here, unless big changes is
> > made.
> 
> That makes sense, thanks for the advice. I'll refine this in the next version.
> 
> > > Signed-off-by: Qiliang Yuan <realwujing@gmail.com>
> > > ---
> > > On Mon, 2026-01-12 at 19:15 +0100, Kumar Kartikeya Dwivedi wrote:
> > > > Did you run any numbers on whether this improves verification performance?
> > > > Without any compelling evidence, I would leave things as-is.
> > 
> > This is not how we write change logs, please see how other people
> > do.
> 
> Actually, the content following the 'Signed-off-by' line and the '---' marker 
> is specifically designed to be ignored by 'git am' when the patch is applied. 
> Only the text above the triple-dash is preserved as the permanent commit 
> message. I intentionally placed the responses to previous reviewer comments 
> in that section so that you could see the context and history during review 
> without those discussions being permanently recorded in the git log. You can 
> verify this behavior by testing 'git am' on a similar patch.
> 
> It's for this very reason that I decided to include the reply to reviewers 
> directly within the v2 patch.

I think we don't do it this way, and it makes the patch look a mess. You can
reply directly in the mail.

> 





  parent reply	other threads:[~2026-01-17 11:28 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-12 12:24 [PATCH] " wujing
2026-01-12 18:15 ` Kumar Kartikeya Dwivedi
2026-01-14  8:28   ` [PATCH bpf-next] " wujing
2026-01-14 15:59     ` Alexei Starovoitov
2026-01-15  3:19       ` wujing
2026-01-16 13:29   ` [PATCH v2] " Qiliang Yuan
2026-01-16 14:50     ` Menglong Dong
2026-01-17  3:26       ` Qiliang Yuan
2026-01-17  6:24         ` Alexei Starovoitov
2026-01-17 11:08           ` Menglong Dong
2026-01-17 11:27         ` Menglong Dong [this message]
2026-01-18  9:46           ` Qiliang Yuan

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=6205668.MhkbZ0Pkbq@7950hx \
    --to=menglong.dong@linux.dev \
    --cc=alexei.starovoitov@gmail.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=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=realwujing@gmail.com \
    --cc=realwujing@qq.com \
    --cc=sdf@fomichev.me \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    --cc=yuanql9@chinatelecom.cn \
    /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

all inboxes | Powered by JetHome®