Hi Maciej,
On 2/9/2024 6:02 AM, Maciej Wieczor-Retman wrote:
...
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> index 39fc9303b8e8..d4b7bf8a6780 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -294,6 +294,71 @@ static int cat_run_test(const struct resctrl_test *test, const struct user_param
> return ret;
> }
>
> +static int noncont_cat_run_test(const struct resctrl_test *test,
> + const struct user_params *uparams)
> +{
> + unsigned long full_cache_mask, cont_mask, noncont_mask;
> + unsigned int eax, ebx, ecx, edx, ret, sparse_masks;
I missed that "ret" is "unsigned int" while the test expects it to
be signed ("if (ret < 0)") and it is used to have return value of functions
that return < 0 on error.
> + char schemata[64];
> + int bit_center;
> +
> + /* Check to compare sparse_masks content to CPUID output. */
> + ret = resource_info_unsigned_get(test->resource, "sparse_masks", &sparse_masks);
> + if (ret)
> + return ret;
> +
> + if (!strcmp(test->resource, "L3"))
> + __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
> + else if (!strcmp(test->resource, "L2"))
> + __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
> + else
> + return -EINVAL;
> +
> + if (sparse_masks != ((ecx >> 3) & 1)) {
> + ksft_print_msg("CPUID output doesn't match 'sparse_masks' file content!\n");
> + return 1;
> + }
> +
> + /* Write checks initialization. */
> + ret = get_full_cbm(test->resource, &full_cache_mask);
> + if (ret < 0)
> + return ret;
> + bit_center = count_bits(full_cache_mask) / 2;
It would be nice if no new static check issues are introduced into the
resctrl selftests. I did a quick check and this is a problematic portion.
We know that the full_cache_mask cannot have zero bits but it is not
obvious to the checkers, thus thinking that bit_center may be zero
resulting in a bit shift of "-2" bits attempt later on. Could you please
add some error checking to ensure expected values to avoid extra noise from
checkers when this code lands upstream?
Thank you
Reinette
