From: sashiko-bot@kernel.org
To: "Jiri Olsa" <jolsa@kernel.org>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCHv5 06/13] libbpf: Change has_nop_combo to work on top of nop10
Date: Wed, 01 Jul 2026 11:34:13 +0000 [thread overview]
Message-ID: <20260701113413.8372D1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260701111337.53943-7-jolsa@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch breaks USDT optimization selftests and functionally fails to enable the optimization because the kernel does not support optimizing 10-byte NOPs.
--
commit 7b26e4395329f0d2e4ebce57947c8615c8552ea2
Author: Jiri Olsa <jolsa@kernel.org>
libbpf: Change has_nop_combo to work on top of nop10
This commit modifies libbpf's USDT parsing to check for an 11-byte NOP
combination (nop + nop10) instead of the previous 6-byte combination
(nop + nop5). The intent is to leverage faster uprobe handling.
> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> index 57fb82bb81b58..d2ecd3daab961 100644
> --- a/tools/lib/bpf/usdt.c
> +++ b/tools/lib/bpf/usdt.c
[ ... ]
> @@ -604,14 +604,14 @@ static int parse_usdt_spec(struct usdt_spec *spec, const struct usdt_note *note,
> #if defined(__x86_64__)
> static bool has_nop_combo(int fd, long off)
> {
> - unsigned char nop_combo[6] = {
> - 0x90, 0x0f, 0x1f, 0x44, 0x00, 0x00 /* nop,nop5 */
> + unsigned char nop_combo[11] = {
> + 0x90, 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00,
> };
> - unsigned char buf[6];
> + unsigned char buf[11];
>
> - if (pread(fd, buf, 6, off) != 6)
> + if (pread(fd, buf, 11, off) != 11)
> return false;
> - return memcmp(buf, nop_combo, 6) == 0;
> + return memcmp(buf, nop_combo, 11) == 0;
> }
[Severity: High]
Will this modification break the USDT optimization selftests and prevent uprobes
from being optimized?
The BPF selftests still emit a 6-byte NOP combo:
tools/testing/selftests/bpf/usdt.h:
#define USDT_NOP .byte 0x90, 0x0f, 0x1f, 0x44, 0x00, 0x0 /* nop, nop5 */
Because libbpf now strictly requires an 11-byte sequence, it skips the
optimization for existing tests. This causes subtest_optimized_attach() in
tools/testing/selftests/bpf/prog_tests/usdt.c to fail its assertion:
ASSERT_EQ(*(addr_2 + 1), 0xe8, "call");
The assertion fails because the uprobe falls back to an unoptimized 1-byte INT3
(0xcc) instead of the optimized 5-byte call (0xe8) at addr_2 + 1.
Furthermore, if user-space were to emit the 10-byte NOP, is the kernel currently
capable of optimizing it? Looking at arch/x86/kernel/uprobes.c:
arch/x86/kernel/uprobes.c:can_optimize() {
...
if (!insn->x86_64 || insn->length != 5)
return false;
...
}
Because the kernel strictly requires the instruction length to be exactly 5 bytes,
a 10-byte NOP will be rejected for optimization. The kernel will fall back to an
INT3 uprobe, meaning the intended uprobe syscall optimization is not achieved.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701111337.53943-7-jolsa@kernel.org?part=1
next prev parent reply other threads:[~2026-07-01 11:34 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-01 11:13 [PATCHv5 00/13] uprobes/x86: Fix red zone issue for optimized uprobes Jiri Olsa
2026-07-01 11:13 ` [PATCHv5 01/13] uprobes/x86: Use proper mm_struct in __in_uprobe_trampoline Jiri Olsa
2026-07-01 11:32 ` sashiko-bot
2026-07-03 9:55 ` [tip: perf/urgent] " tip-bot2 for Jiri Olsa
2026-07-01 11:13 ` [PATCHv5 02/13] uprobes/x86: Remove struct uprobe_trampoline object Jiri Olsa
2026-07-01 11:57 ` bot+bpf-ci
2026-07-01 11:13 ` [PATCHv5 03/13] uprobes/x86: Do not leak trampoline vma mapping on optimization failure Jiri Olsa
2026-07-01 11:13 ` [PATCHv5 04/13] uprobes/x86: Allow to copy uprobe trampolines on fork Jiri Olsa
2026-07-01 11:13 ` [PATCHv5 05/13] uprobes/x86: Move optimized uprobe from nop5 to nop10 Jiri Olsa
2026-07-01 11:57 ` bot+bpf-ci
2026-07-01 11:13 ` [PATCHv5 06/13] libbpf: Change has_nop_combo to work on top of nop10 Jiri Olsa
2026-07-01 11:34 ` sashiko-bot [this message]
2026-07-01 11:13 ` [PATCHv5 07/13] libbpf: Detect uprobe syscall with new error Jiri Olsa
2026-07-01 11:30 ` sashiko-bot
2026-07-01 11:13 ` [PATCHv5 08/13] selftests/bpf: Emit nop,nop10 instructions combo for x86_64 arch Jiri Olsa
2026-07-01 11:26 ` sashiko-bot
2026-07-01 11:13 ` [PATCHv5 09/13] selftests/bpf: Change uprobe syscall tests to use nop10 Jiri Olsa
2026-07-01 11:33 ` sashiko-bot
2026-07-01 11:13 ` [PATCHv5 10/13] selftests/bpf: Change uprobe/usdt trigger bench code " Jiri Olsa
2026-07-01 11:13 ` [PATCHv5 11/13] selftests/bpf: Add reattach tests for uprobe syscall Jiri Olsa
2026-07-01 11:13 ` [PATCHv5 12/13] selftests/bpf: Add tests for uprobe nop10 red zone clobbering Jiri Olsa
2026-07-01 11:57 ` bot+bpf-ci
2026-07-01 11:13 ` [PATCHv5 13/13] selftests/bpf: Add tests for forked/cloned optimized uprobes Jiri Olsa
2026-07-01 11:57 ` bot+bpf-ci
2026-07-01 23:13 ` [PATCHv5 00/13] uprobes/x86: Fix red zone issue for " Andrii Nakryiko
2026-07-02 11:20 ` Jiri Olsa
2026-07-02 16:20 ` Andrii Nakryiko
2026-07-03 10:58 ` Peter Zijlstra
2026-07-03 11:07 ` Jiri Olsa
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=20260701113413.8372D1F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=jolsa@kernel.org \
--cc=sashiko-reviews@lists.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 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.