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

[Jiri Olsa <olsajiri@gmail.com>]

On Thu, May 14, 2026 at 08:44:25PM +0000, sashiko-bot@kernel.org wrote:
> 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?

yep, selftests are modified in following change

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

yep, will update

jirka