Re: [PATCH 3/7] selftests/bpf: Emit nop,nop10 instructions combo for x86_64 arch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit modifies `USDT_NOP` in `usdt.h` to an 11-byte `nop,nop10` sequence but does not update `prog_tests/usdt.c`, causing the selftest to break and introducing a git bisect regression.
--

commit 600c1890b9793e001b5d8d9027629fb122af9024
Author: Jiri Olsa <jolsa@kernel.org>

selftests/bpf: Emit nop,nop10 instructions combo for x86_64 arch
    
This commit updates the USDT_NOP macro in usdt.h for x86_64 to emit an
11-byte nop, nop10 instruction sequence instead of the previous nop, nop5
sequence. It synchronizes with the latest usdt.h changes to utilize nop10
optimization support.

> diff --git a/tools/testing/selftests/bpf/usdt.h b/tools/testing/selftests/bpf/usdt.h
> --- a/tools/testing/selftests/bpf/usdt.h
> +++ b/tools/testing/selftests/bpf/usdt.h

[ ... ]

> @@ -313,7 +313,7 @@ struct usdt_sema { volatile unsigned short active; };
>  #if defined(__ia64__) || defined(__s390__) || defined(__s390x__)
>  #define USDT_NOP			nop 0
>  #elif defined(__x86_64__)
> -#define USDT_NOP                       .byte 0x90, 0x0f, 0x1f, 0x44, 0x00, 0x0 /* nop, nop5 */
> +#define USDT_NOP                       .byte 0x90, 0x66, 0x66, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 /* nop, nop10 */

Does changing this instruction sequence from 6 bytes to 11 bytes without
updating prog_tests/usdt.c in the same commit cause a test breakage?

The selftest in prog_tests/usdt.c hardcodes the old 6-byte nop1_nop5_combo
and searches for it using find_instr(usdt_2, nop1_nop5_combo, 6). Because
the old sequence is no longer present in usdt_2.c, find_instr will return
NULL, causing the ASSERT_OK_PTR assertion to fail.

I see this is addressed later in the series in commit
ca88e95623000540d5b706ce364d5b5abf75b4c1. Would it be better to squash the
test update into this commit to avoid breaking git bisect?

Additionally, looking at that later commit, it updates nop1_nop10_combo to
11 bytes but appears to continue passing 6 as the size argument to
find_instr. Should that size argument be updated to 11 as well?

>  #else
>  #define USDT_NOP			nop
>  #endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260514135342.22130-1-jolsa@kernel.org?part=3