Message-ID: <20260117032612.10008-1-yuanql9@chinatelecom.cn>
Date: Sat, 17 Jan 2026 11:26:12 +0800
From: Qiliang Yuan <realwujing@...il.com>
To: menglong.dong@...ux.dev
Cc: alexei.starovoitov@...il.com,
	andrii@...nel.org,
	ast@...nel.org,
	bpf@...r.kernel.org,
	daniel@...earbox.net,
	eddyz87@...il.com,
	haoluo@...gle.com,
	john.fastabend@...il.com,
	jolsa@...nel.org,
	kpsingh@...nel.org,
	linux-kernel@...r.kernel.org,
	martin.lau@...ux.dev,
	memxor@...il.com,
	realwujing@...il.com,
	realwujing@...com,
	sdf@...ichev.me,
	song@...nel.org,
	yonghong.song@...ux.dev,
	yuanql9@...natelecom.cn
Subject: Re: [PATCH v2] bpf/verifier: implement slab cache for verifier state list

On Fri, 16 Jan 2026 22:50:36 +0800, Menglong Dong <menglong.dong@...ux.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.

> > 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.

> > Suggested-by: Kumar Kartikeya Dwivedi <memxor@...il.com>
> > Suggested-by: Eduard Zingerman <eddyz87@...il.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@...il.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.

Powered by blists - more mailing lists