Re: [PATCH bpf-next] bpf: Prevent inlining of bpf_fentry_test7()

[Eduard Zingerman <eddyz87@gmail.com>]

On Sat, 2023-08-26 at 13:08 -0700, Yonghong Song wrote:
> With latest clang18, I hit test_progs failures for the following test:
>   #13/2    bpf_cookie/multi_kprobe_link_api:FAIL
>   #13/3    bpf_cookie/multi_kprobe_attach_api:FAIL
>   #13      bpf_cookie:FAIL
>   #75      fentry_fexit:FAIL
>   #76/1    fentry_test/fentry:FAIL
>   #76      fentry_test:FAIL
>   #80/1    fexit_test/fexit:FAIL
>   #80      fexit_test:FAIL
>   #110/1   kprobe_multi_test/skel_api:FAIL
>   #110/2   kprobe_multi_test/link_api_addrs:FAIL
>   #110/3   kprobe_multi_test/link_api_syms:FAIL
>   #110/4   kprobe_multi_test/attach_api_pattern:FAIL
>   #110/5   kprobe_multi_test/attach_api_addrs:FAIL
>   #110/6   kprobe_multi_test/attach_api_syms:FAIL
>   #110     kprobe_multi_test:FAIL
> 
> For example, for #13/2, the error messages are
>   ...
>   kprobe_multi_test_run:FAIL:kprobe_test7_result unexpected kprobe_test7_result: actual 0 != expected 1
>   ...
>   kprobe_multi_test_run:FAIL:kretprobe_test7_result unexpected kretprobe_test7_result: actual 0 != expected 1
> 
> clang17 does not have this issue.
> 
> Further investigation shows that kernel func bpf_fentry_test7(), used
> in the above tests, is inlined by the compiler although it is
> marked as noinline.
> 
>   int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg)
>   {
>         return (long)arg;
>   }
> 
> It is known that for simple functions like the above (e.g. just returning
> a constant or an input argument), the clang compiler may still do inlining
> for a noinline function. Adding 'asm volatile ("")' in the beginning of the
> bpf_fentry_test7() can prevent inlining.

Can confirm, this patch fixes listed flaky tests for me.
(when using LLVM main 651e644595b7 from 6 days ago).

Tested-by: Eduard Zingerman <eddyz87@gmail.com>

> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>  net/bpf/test_run.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 57a7a64b84ed..0841f8d82419 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -543,6 +543,7 @@ struct bpf_fentry_test_t {
>  
>  int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg)
>  {
> +	asm volatile ("");
>  	return (long)arg;
>  }
>