Hi Ilpo,
On 4/18/2023 4:44 AM, Ilpo Järvinen wrote:
> Callers of get_cbm_mask() are required to pass a string into which
> the CBM bit mask is read into. Neither CAT nor CMT tests need the
There is a double "into" above. Perhaps the second can be dropped?
> mask as string but just convert it into an unsigned long value.
>
> The bit mask reader can only read .../cbm_mask files.
>
> Generalize the bit mask reading function into get_bit_mask() such that
> it can be used to handle other files besides the .../cbm_mask and
> handle the unsigned long conversion within within get_bit_mask() using
> fscanf(). Alter get_cbm_mask() to construct the filename for
> get_bit_mask().
>
> Co-developed-by: Fenghua Yu <fenghua.yu@xxxxxxxxx>
> Signed-off-by: Fenghua Yu <fenghua.yu@xxxxxxxxx>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> ---
> tools/testing/selftests/resctrl/cat_test.c | 5 +--
> tools/testing/selftests/resctrl/cmt_test.c | 5 +--
> tools/testing/selftests/resctrl/resctrl.h | 2 +-
> tools/testing/selftests/resctrl/resctrlfs.c | 50 +++++++++++++++------
> 4 files changed, 40 insertions(+), 22 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> index a998e6397518..9bf5d05d9e74 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -18,7 +18,6 @@
> #define MAX_DIFF 1000000
>
> static int count_of_bits;
> -static char cbm_mask[256];
> static unsigned long long_mask;
> static unsigned long cache_size;
>
> @@ -101,12 +100,10 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
> cache_size = 0;
>
> /* Get default cbm mask for L3/L2 cache */
> - ret = get_cbm_mask(cache_type, cbm_mask);
> + ret = get_cbm_mask(cache_type, &long_mask);
> if (ret)
> return ret;
>
> - long_mask = strtoul(cbm_mask, NULL, 16);
> -
> /* Get L3/L2 cache size */
> ret = get_cache_size(cpu_no, cache_type, &cache_size);
> if (ret)
> diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
> index 2d434c03cbba..ae54bbabbd91 100644
> --- a/tools/testing/selftests/resctrl/cmt_test.c
> +++ b/tools/testing/selftests/resctrl/cmt_test.c
> @@ -17,7 +17,6 @@
> #define MAX_DIFF_PERCENT 15
>
> static int count_of_bits;
> -static char cbm_mask[256];
> static unsigned long long_mask;
> static unsigned long cache_size;
>
> @@ -82,12 +81,10 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
> if (!validate_resctrl_feature_request(CMT_STR))
> return -1;
>
> - ret = get_cbm_mask("L3", cbm_mask);
> + ret = get_cbm_mask("L3", &long_mask);
> if (ret)
> return ret;
I think this is a good change. It does raise the question why long_mask
is a global variable so I think it may make things go smoother if
the patch making long_mask local is moved to be before this patch.
Reinette
