[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <OSZPR01MB63280E04EDAE0CCB21B591978B489@OSZPR01MB6328.jpnprd01.prod.outlook.com>
Date: Wed, 31 May 2023 06:03:14 +0000
From: "Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>
To: 'Ilpo Järvinen'
<ilpo.jarvinen@...ux.intel.com>,
"linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
Reinette Chatre <reinette.chatre@...el.com>,
Fenghua Yu <fenghua.yu@...el.com>,
Shuah Khan <shuah@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 15/24] selftests/resctrl: Refactor get_cbm_mask()
Hi Ilpo,
> 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 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@...el.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@...el.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> ---
> 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;
>
> - long_mask = strtoul(cbm_mask, NULL, 16);
> -
> ret = get_cache_size(cpu_no, "L3", &cache_size);
> if (ret)
> return ret;
> diff --git a/tools/testing/selftests/resctrl/resctrl.h
> b/tools/testing/selftests/resctrl/resctrl.h
> index ba36eb5fdf0d..bcc95faa5b4e 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -106,7 +106,7 @@ void tests_cleanup(void); void
> mbm_test_cleanup(void); int mba_schemata_change(int cpu_no, char
> *bw_report, char **benchmark_cmd); void mba_test_cleanup(void); -int
> get_cbm_mask(char *cache_type, char *cbm_mask);
> +int get_cbm_mask(char *cache_type, unsigned long *mask);
> int get_cache_size(int cpu_no, char *cache_type, unsigned long
> *cache_size); void ctrlc_handler(int signum, siginfo_t *info, void *ptr); int
> signal_handler_register(void); diff --git
> a/tools/testing/selftests/resctrl/resctrlfs.c
> b/tools/testing/selftests/resctrl/resctrlfs.c
> index 7fef9068d7fd..f01ecfa64063 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -186,30 +186,29 @@ int get_cache_size(int cpu_no, char *cache_type,
> unsigned long *cache_size)
> #define CORE_SIBLINGS_PATH "/sys/bus/cpu/devices/cpu"
>
> /*
> - * get_cbm_mask - Get cbm mask for given cache
> - * @cache_type: Cache level L2/L3
> - * @cbm_mask: cbm_mask returned as a string
> + * get_bit_mask - Get bit mask from given file
> + * @filename: File containing the mask
> + * @mask: The bit mask returned as unsigned long
> *
> * Return: = 0 on success, < 0 on failure.
> */
> -int get_cbm_mask(char *cache_type, char *cbm_mask)
> +static int get_bit_mask(char *filename, unsigned long *mask)
> {
> - char cbm_mask_path[1024];
> FILE *fp;
>
> - if (!cbm_mask)
> + if (!filename || !mask)
> return -1;
>
> - sprintf(cbm_mask_path, "%s/%s/cbm_mask", INFO_PATH,
> cache_type);
> -
> - fp = fopen(cbm_mask_path, "r");
> + fp = fopen(filename, "r");
> if (!fp) {
> - perror("Failed to open cache level");
> -
> + fprintf(stderr, "Failed to open bit mask file '%s': %s\n",
> + filename, strerror(errno));
> return -1;
> }
> - if (fscanf(fp, "%s", cbm_mask) <= 0) {
> - perror("Could not get max cbm_mask");
> +
> + if (fscanf(fp, "%lx", mask) <= 0) {
> + fprintf(stderr, "Could not read bit mask file '%s': %s\n",
> + filename, strerror(errno));
> fclose(fp);
>
> return -1;
> @@ -219,6 +218,31 @@ int get_cbm_mask(char *cache_type, char
> *cbm_mask)
> return 0;
> }
>
> +/*
> + * get_cbm_bits - Get number of bits in cbm mask
Is it get_cbm_mask?
Best regards,
Shaopeng TAN
> + * @cache_type: Cache level L2/L3
> + * @mask: cbm_mask returned as unsigned long
> + *
> + * Return: = 0 on success, < 0 on failure.
> + */
> +int get_cbm_mask(char *cache_type, unsigned long *mask) {
> + char cbm_mask_path[1024];
> + int ret;
> +
> + if (!cache_type)
> + return -1;
> +
> + snprintf(cbm_mask_path, sizeof(cbm_mask_path),
> "%s/%s/cbm_mask",
> + INFO_PATH, cache_type);
> +
> + ret = get_bit_mask(cbm_mask_path, mask);
> + if (ret)
> + return -1;
> +
> + return 0;
> +}
> +
> /*
> * get_core_sibling - Get sibling core id from the same socket for given CPU
> * @cpu_no: CPU number
> --
> 2.30.2
Powered by blists - more mailing lists