Re: [PATCH v16 15/16] barrier: timeout validity checks for smp_cond_load_relaxed_timeout()

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]

> barrier: timeout validity checks for smp_cond_load_relaxed_timeout()

TL;DR: Nits only, in the new KUnit test: iteration bounds print as unprefixed
hex ("10" for 16), which is confusing in test names, and a format string has a
stray double space.

> diff --git a/lib/tests/barrier-timeout-test.c b/lib/tests/barrier-timeout-test.c
> index 9c12cd36e7d2..88b374ceb78b 100644
> --- a/lib/tests/barrier-timeout-test.c
> +++ b/lib/tests/barrier-timeout-test.c

[ ... ]

> +static void expiry_param_to_desc(const struct smp_cond_expiry_params *p, char *desc)
> +{
> +	char iters[32] = "";
> +
> +	if (p->miniters != -1)
> +		snprintf(iters, 32, ">= %llx", (u64)p->miniters);
> +	else if (p->maxiters != -1)
> +		snprintf(iters, 32, "%s %llx", p->maxiters == 0 ? "==" : "<=", (u64)p->maxiters);
> +
> +	snprintf(desc, KUNIT_PARAM_DESC_SIZE,
> +		 "smp_cond_*_timeout: clock=%s,  timeout=%s, iterations %s",
> +		 "synthetic", p->desc, iters);
> +}

This isn't a bug, but would printing the iteration bound in decimal (%lld)
or with a 0x prefix make the test names easier to match against the checks?

With "%llx" and no prefix, the U32_MAX row (miniters = 1 << (32-28)) is
reported as "iterations >= 10" and the 1<<33 row (miniters = 1 << (33-28))
as "iterations >= 20", while test_smp_cond_relaxed() actually checks for
>= 16 and >= 32.  The timeout labels in the same table use the "1<<33"
style, so the hex counts read inconsistently in the KTAP output.

Also, is the double space after "clock=%s," in the format string intended?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38867477666