| Message ID | 20230418114506.46788-3-ilpo.jarvinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/resctrl: Fixes, cleanups, and rewritten CAT test | expand |
Hi Ilpo, On 4/18/2023 4:44 AM, Ilpo Järvinen wrote: > CAT test only validates that the number of CBM bits is not too large > but it could be too small too. Could you please elaborate how this scenario could occur? > Check and return error before starting the CAT test if the number of > CBM bits is too small. > > Fixes: 09a67934625a ("selftests/resctrl: Don't hard code value of "no_of_bits" variable") This fix is not clear to me. This commit being fixed already contains an explicit test that will bail out of no_of_bits <= 0. It is not clear to me why it is necessary to adding a test for < 1 as a fix for this commit. > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > --- > tools/testing/selftests/resctrl/cat_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c > index fb1443f888c4..722c9cd4120d 100644 > --- a/tools/testing/selftests/resctrl/cat_test.c > +++ b/tools/testing/selftests/resctrl/cat_test.c > @@ -129,7 +129,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) > if (!n) > n = count_of_bits / 2; > > - if (n > count_of_bits - 1) { > + if (n < 1 || n > count_of_bits - 1) { > ksft_print_msg("Invalid input value for no_of_bits n!\n"); > ksft_print_msg("Please enter value in range 1 to %d\n", > count_of_bits - 1); Reinette
On Fri, 21 Apr 2023, Reinette Chatre wrote: > Hi Ilpo, > > On 4/18/2023 4:44 AM, Ilpo Järvinen wrote: > > CAT test only validates that the number of CBM bits is not too large > > but it could be too small too. > > Could you please elaborate how this scenario could occur? > > > Check and return error before starting the CAT test if the number of > > CBM bits is too small. > > > > Fixes: 09a67934625a ("selftests/resctrl: Don't hard code value of "no_of_bits" variable") > > This fix is not clear to me. This commit being fixed already contains > an explicit test that will bail out of no_of_bits <= 0. It is not clear to me > why it is necessary to adding a test for < 1 as a fix for this commit. Ah, indeed, it's checked on the higher level so this fix is unnecessary. I missed it entirely while taking this change out from a more complex change and even failed to make the connection when I stared at the very if <= 0 check not so many days ago.
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index fb1443f888c4..722c9cd4120d 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -129,7 +129,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) if (!n) n = count_of_bits / 2; - if (n > count_of_bits - 1) { + if (n < 1 || n > count_of_bits - 1) { ksft_print_msg("Invalid input value for no_of_bits n!\n"); ksft_print_msg("Please enter value in range 1 to %d\n", count_of_bits - 1);
CAT test only validates that the number of CBM bits is not too large but it could be too small too. Check and return error before starting the CAT test if the number of CBM bits is too small. Fixes: 09a67934625a ("selftests/resctrl: Don't hard code value of "no_of_bits" variable") Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> --- tools/testing/selftests/resctrl/cat_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)