[DTrace-devel] [PATCH] bpf: ensure speculations drops are reported when needed

[Nick Alcock]

On 30 May 2023, Kris Van Hees via DTrace-devel verbalised:

> The unrolled loop in dt_speculation() would return without recording
> a speculation drop if an iteration was attempted beyond the configured
> NSPEC value.  Depending on how the compiler optimized the code, that
> could lead to an early return from the function without ever calling
> dt_no_spec().
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
>  bpf/speculation.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/bpf/speculation.c b/bpf/speculation.c
> index 82d09e1a..7f4ebd34 100644
> --- a/bpf/speculation.c
> +++ b/bpf/speculation.c
> @@ -54,7 +54,7 @@ noinline uint32_t dt_speculation(void)
>  #define SEARCH(n)							\
>  	do {								\
>  		if (n > (uint64_t) &NSPEC)				\
> -			return 0;					\
> +			break;						\

... thus hitting

        return dt_no_spec(busy ? DT_STATE_SPEC_BUSY : DT_STATE_SPEC_UNAVAIL);

at the end.

If this was a real loop, I'd prefer to turn this return 0 into the
same dt_no_spec() call, but since that's being unrolled and thus
repeated 16 times, your change is clearly preferable.

(How did this ever work? Are the OL8/9 optimizers simply buggy?)

Reviewed-by: Nick Alcock <nick.alcock at oracle.com>

-- 
NULL && (void)