[v2,15/24] selftests/resctrl: Refactor get_cbm_mask()

Message ID	20230418114506.46788-16-ilpo.jarvinen@linux.intel.com (mailing list archive)
State	New
Headers	show Return-Path: <linux-kselftest-owner@vger.kernel.org> From: =?utf-8?q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com> To: linux-kselftest@vger.kernel.org, Reinette Chatre <reinette.chatre@intel.com>, Fenghua Yu <fenghua.yu@intel.com>, Shuah Khan <shuah@kernel.org>, linux-kernel@vger.kernel.org Cc: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>, =?utf-8?q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com> Subject: [PATCH v2 15/24] selftests/resctrl: Refactor get_cbm_mask() Date: Tue, 18 Apr 2023 14:44:57 +0300 Message-Id: <20230418114506.46788-16-ilpo.jarvinen@linux.intel.com> In-Reply-To: <20230418114506.46788-1-ilpo.jarvinen@linux.intel.com> References: <20230418114506.46788-1-ilpo.jarvinen@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	selftests/resctrl: Fixes, cleanups, and rewritten CAT test \| expand [v2,00/24] selftests/resctrl: Fixes, cleanups, and rewritten CAT test [v2,01/24] selftests/resctrl: Add resctrl.h into build deps [v2,02/24] selftests/resctrl: Check also too low values for CBM bits [v2,03/24] selftests/resctrl: Move resctrl FS mount/umount to higher level [v2,04/24] selftests/resctrl: Remove mum_resctrlfs [v2,05/24] selftests/resctrl: Make span unsigned long everywhere [v2,06/24] selftests/resctrl: Express span in bytes [v2,07/24] selftests/resctrl: Remove duplicated preparation for span arg [v2,08/24] selftests/resctrl: Don't use variable argument list for ->setup() [v2,09/24] selftests/resctrl: Remove "malloc_and_init_memory" param from run_fill_buf() [v2,10/24] selftests/resctrl: Split run_fill_buf() to alloc, work, and dealloc helpers [v2,11/24] selftests/resctrl: Remove start_buf local variable from buffer alloc func [v2,12/24] selftests/resctrl: Don't pass test name to fill_buf [v2,13/24] selftests/resctrl: Add flush_buffer() to fill_buf [v2,14/24] selftests/resctrl: Remove test type checks from cat_val() [v2,15/24] selftests/resctrl: Refactor get_cbm_mask() [v2,16/24] selftests/resctrl: Create cache_alloc_size() helper [v2,17/24] selftests/resctrl: Replace count_bits with count_consecutive_bits() [v2,18/24] selftests/resctrl: Exclude shareable bits from schemata in CAT test [v2,19/24] selftests/resctrl: Pass the real number of tests to show_cache_info() [v2,20/24] selftests/resctrl: Move CAT/CMT test global vars to func they are used [v2,21/24] selftests/resctrl: Read in less obvious order to defeat prefetch optimizations [v2,22/24] selftests/resctrl: Split measure_cache_vals() function [v2,23/24] selftests/resctrl: Split show_cache_info() to test specific and generic parts [v2,24/24] selftests/resctrl: Rewrite Cache Allocation Technology (CAT) test

Message ID

20230418114506.46788-16-ilpo.jarvinen@linux.intel.com (mailing list archive)

State

New

Headers

From: =?utf-8?q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
To: linux-kselftest@vger.kernel.org,
        Reinette Chatre <reinette.chatre@intel.com>,
        Fenghua Yu <fenghua.yu@intel.com>,
        Shuah Khan <shuah@kernel.org>, linux-kernel@vger.kernel.org
Cc: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>,
 =?utf-8?q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Subject: [PATCH v2 15/24] selftests/resctrl: Refactor get_cbm_mask()
Date: Tue, 18 Apr 2023 14:44:57 +0300
Message-Id: <20230418114506.46788-16-ilpo.jarvinen@linux.intel.com>
In-Reply-To: <20230418114506.46788-1-ilpo.jarvinen@linux.intel.com>
References: <20230418114506.46788-1-ilpo.jarvinen@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

Series

selftests/resctrl: Fixes, cleanups, and rewritten CAT test | expand

Commit Message

Ilpo Järvinen April 18, 2023, 11:44 a.m. UTC

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@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.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(-)

Comments

Reinette Chatre April 22, 2023, 12:17 a.m. UTC | #1

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@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.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;

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

Shaopeng Tan (Fujitsu) May 31, 2023, 6:03 a.m. UTC | #2

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@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.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

Ilpo Järvinen May 31, 2023, 9:24 a.m. UTC | #3

On Wed, 31 May 2023, Shaopeng Tan (Fujitsu) wrote:

> 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@intel.com>
> > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.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?

Sure, thanks for noticing this. While correcting it, I also improved the 
description to match what the function now does.

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
+ * @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

[v2,15/24] selftests/resctrl: Refactor get_cbm_mask()

Commit Message

Comments

Patch