diff mbox series

[v2,16/24] selftests/resctrl: Create cache_alloc_size() helper

Message ID 20230418114506.46788-17-ilpo.jarvinen@linux.intel.com
State New
Headers show
Series selftests/resctrl: Fixes, cleanups, and rewritten CAT test | expand

Commit Message

Ilpo Järvinen April 18, 2023, 11:44 a.m. UTC
CAT and CMT tests calculate the span size from the n-bits cache
allocation on their own.

Add cache_alloc_size() helper which calculates size of the cache
allocation for the given number of bits to avoid duplicating code.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 tools/testing/selftests/resctrl/cache.c    | 27 ++++++++++++++++++++++
 tools/testing/selftests/resctrl/cat_test.c |  8 +++++--
 tools/testing/selftests/resctrl/cmt_test.c |  4 +++-
 tools/testing/selftests/resctrl/resctrl.h  |  2 ++
 4 files changed, 38 insertions(+), 3 deletions(-)

Comments

Reinette Chatre April 22, 2023, 12:19 a.m. UTC | #1
Hi Ilpo,

On 4/18/2023 4:44 AM, Ilpo Järvinen wrote:
> CAT and CMT tests calculate the span size from the n-bits cache
> allocation on their own.
> 
> Add cache_alloc_size() helper which calculates size of the cache
> allocation for the given number of bits to avoid duplicating code.

This patch is very heavy on the usage of allocation when I think it
only refers to the cache size ... how that size is used by the caller
is independent from this. 

Compare to how it sounds with some small changes to changelog:

	CAT and CMT tests calculate the span size from the capacity
	bitmask independently.
	
	Add cache_size() helper which calculates the size of the
	cache for the given number of bits to avoid duplicating code.

I think removing "alloc" helps to convey what this code actually does.

> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  tools/testing/selftests/resctrl/cache.c    | 27 ++++++++++++++++++++++
>  tools/testing/selftests/resctrl/cat_test.c |  8 +++++--
>  tools/testing/selftests/resctrl/cmt_test.c |  4 +++-
>  tools/testing/selftests/resctrl/resctrl.h  |  2 ++
>  4 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
> index 6bc912de38be..b983af394e33 100644
> --- a/tools/testing/selftests/resctrl/cache.c
> +++ b/tools/testing/selftests/resctrl/cache.c
> @@ -15,6 +15,33 @@ static struct read_format rf_cqm;
>  static int fd_lm;
>  char llc_occup_path[1024];
>  
> +/*
> + * cache_alloc_size - Calculate alloc size for given cache alloc mask

"cache_size - Calculate number of bytes represented by bitmask" ?
Please feel free to improve.


> + * @cpu_no:		CPU number
> + * @cache_type:		Cache level L2/L3
> + * @alloc_mask:		Cache alloc mask

The description is mostly a rewrite of the variable name. Can it be
more descriptive?

> + * @alloc_size:		Alloc size returned on success

I do not think the utility should assume anything about how
the value it provides should be used. Instead it should just reflect
what the value is.

> + *
> + * Returns: 0 on success with @alloc_size filled, non-zero on error.
> + */
> +int cache_alloc_size(int cpu_no, char *cache_type, unsigned long alloc_mask,
> +		     unsigned long *alloc_size)
> +{
> +	unsigned long cache_size, full_mask;
> +	int ret;
> +
> +	ret = get_cbm_mask(cache_type, &full_mask);
> +	if (ret)
> +		return ret;
> +
> +	ret = get_cache_size(cpu_no, cache_type, &cache_size);
> +	if (ret)
> +		return ret;
> +
> +	*alloc_size = cache_size * count_bits(alloc_mask) / count_bits(full_mask);
> +	return 0;
> +}
> +
>  static void initialize_perf_event_attr(void)
>  {
>  	pea_llc_miss.type = PERF_TYPE_HARDWARE;
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> index 9bf5d05d9e74..d3fbd4de9f8a 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -140,7 +140,9 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
>  	/* Set param values for parent thread which will be allocated bitmask
>  	 * with (max_bits - n) bits
>  	 */
> -	param.span = cache_size * (count_of_bits - n) / count_of_bits;
> +	ret = cache_alloc_size(cpu_no, cache_type, l_mask, &param.span);
> +	if (ret)
> +		return ret;
>  	strcpy(param.ctrlgrp, "c2");
>  	strcpy(param.mongrp, "m2");
>  	strcpy(param.filename, RESULT_FILE_NAME2);
> @@ -162,7 +164,9 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
>  		param.mask = l_mask_1;
>  		strcpy(param.ctrlgrp, "c1");
>  		strcpy(param.mongrp, "m1");
> -		param.span = cache_size * n / count_of_bits;
> +		ret = cache_alloc_size(cpu_no, cache_type, l_mask_1, &param.span);
> +		if (ret)
> +			exit(-1);
>  		strcpy(param.filename, RESULT_FILE_NAME1);
>  		param.num_of_runs = 0;
>  		param.cpu_no = sibling_cpu_no;

Did this change intend to remove the duplicate code mentioned
in the changelog? I was expecting the calls to get_cbm_mask() and
get_cache_size() within cat_perf_miss_val() to be removed.

> diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
> index ae54bbabbd91..efe77e0f1d4c 100644
> --- a/tools/testing/selftests/resctrl/cmt_test.c
> +++ b/tools/testing/selftests/resctrl/cmt_test.c
> @@ -105,10 +105,12 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
>  		.cpu_no		= cpu_no,
>  		.filename	= RESULT_FILE_NAME,
>  		.mask		= ~(long_mask << n) & long_mask,
> -		.span		= cache_size * n / count_of_bits,
>  		.num_of_runs	= 0,
>  		.setup		= cmt_setup,
>  	};
> +	ret = cache_alloc_size(cpu_no, "L3", param.mask, &param.span);
> +	if (ret)
> +		return ret;
>  
>  	if (strcmp(benchmark_cmd[0], "fill_buf") == 0)
>  		sprintf(benchmark_cmd[1], "%lu", param.span);

Same here regarding removal of code.

> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
> index bcc95faa5b4e..65425d92684e 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -108,6 +108,8 @@ 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, unsigned long *mask);
>  int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size);
> +int cache_alloc_size(int cpu_no, char *cache_type, unsigned long alloc_mask,
> +		     unsigned long *alloc_size);
>  void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
>  int signal_handler_register(void);
>  void signal_handler_unregister(void);


Reinette
Ilpo Järvinen April 24, 2023, 4:28 p.m. UTC | #2
On Fri, 21 Apr 2023, Reinette Chatre wrote:

> On 4/18/2023 4:44 AM, Ilpo Järvinen wrote:
> > CAT and CMT tests calculate the span size from the n-bits cache
> > allocation on their own.
> > 
> > Add cache_alloc_size() helper which calculates size of the cache
> > allocation for the given number of bits to avoid duplicating code.
> 
> This patch is very heavy on the usage of allocation when I think it
> only refers to the cache size ... how that size is used by the caller
> is independent from this. 
> 
> Compare to how it sounds with some small changes to changelog:
> 
> 	CAT and CMT tests calculate the span size from the capacity
> 	bitmask independently.
> 	
> 	Add cache_size() helper which calculates the size of the
> 	cache for the given number of bits to avoid duplicating code.
> 
> I think removing "alloc" helps to convey what this code actually does.

Does it? Without something to indicate its not the full cache size, 
there's possiblity for confusion. While the tests are mostly interested 
in the allocated size, the full cache size is also collected (solely for 
printing it out, IIRC). Maybe I should rename those variable to 
total_cache_size or something like that to mitigate the confusion?

> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> >  tools/testing/selftests/resctrl/cache.c    | 27 ++++++++++++++++++++++
> >  tools/testing/selftests/resctrl/cat_test.c |  8 +++++--
> >  tools/testing/selftests/resctrl/cmt_test.c |  4 +++-
> >  tools/testing/selftests/resctrl/resctrl.h  |  2 ++
> >  4 files changed, 38 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
> > index 6bc912de38be..b983af394e33 100644
> > --- a/tools/testing/selftests/resctrl/cache.c
> > +++ b/tools/testing/selftests/resctrl/cache.c
> > @@ -15,6 +15,33 @@ static struct read_format rf_cqm;
> >  static int fd_lm;
> >  char llc_occup_path[1024];
> >  
> > +/*
> > + * cache_alloc_size - Calculate alloc size for given cache alloc mask
> 
> "cache_size - Calculate number of bytes represented by bitmask" ?
> Please feel free to improve.
> 
> 
> > + * @cpu_no:		CPU number
> > + * @cache_type:		Cache level L2/L3
> > + * @alloc_mask:		Cache alloc mask
> 
> The description is mostly a rewrite of the variable name. Can it be
> more descriptive?
> 
> > + * @alloc_size:		Alloc size returned on success
> 
> I do not think the utility should assume anything about how
> the value it provides should be used. Instead it should just reflect
> what the value is.

I was just referring to that the value is filled only on success.

> > + * Returns: 0 on success with @alloc_size filled, non-zero on error.
> > + */
> > +int cache_alloc_size(int cpu_no, char *cache_type, unsigned long alloc_mask,
> > +		     unsigned long *alloc_size)
> > +{
> > +	unsigned long cache_size, full_mask;
> > +	int ret;
> > +
> > +	ret = get_cbm_mask(cache_type, &full_mask);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = get_cache_size(cpu_no, cache_type, &cache_size);
> > +	if (ret)
> > +		return ret;
> > +
> > +	*alloc_size = cache_size * count_bits(alloc_mask) / count_bits(full_mask);
> > +	return 0;
> > +}
> > +
> >  static void initialize_perf_event_attr(void)
> >  {
> >  	pea_llc_miss.type = PERF_TYPE_HARDWARE;
> > diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> > index 9bf5d05d9e74..d3fbd4de9f8a 100644
> > --- a/tools/testing/selftests/resctrl/cat_test.c
> > +++ b/tools/testing/selftests/resctrl/cat_test.c
> > @@ -140,7 +140,9 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
> >  	/* Set param values for parent thread which will be allocated bitmask
> >  	 * with (max_bits - n) bits
> >  	 */
> > -	param.span = cache_size * (count_of_bits - n) / count_of_bits;
> > +	ret = cache_alloc_size(cpu_no, cache_type, l_mask, &param.span);
> > +	if (ret)
> > +		return ret;
> >  	strcpy(param.ctrlgrp, "c2");
> >  	strcpy(param.mongrp, "m2");
> >  	strcpy(param.filename, RESULT_FILE_NAME2);
> > @@ -162,7 +164,9 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
> >  		param.mask = l_mask_1;
> >  		strcpy(param.ctrlgrp, "c1");
> >  		strcpy(param.mongrp, "m1");
> > -		param.span = cache_size * n / count_of_bits;
> > +		ret = cache_alloc_size(cpu_no, cache_type, l_mask_1, &param.span);
> > +		if (ret)
> > +			exit(-1);
> >  		strcpy(param.filename, RESULT_FILE_NAME1);
> >  		param.num_of_runs = 0;
> >  		param.cpu_no = sibling_cpu_no;
> 
> Did this change intend to remove the duplicate code mentioned
> in the changelog?

It removes n CBM bits -> cache size calculations by collecting the 
calculation into one place.

cache_alloc_size() takes mask instead of n (CBM bits) as input which makes 
things easier down the line when the new CAT test starts to tweak the 
alloc size. The new CAT test would otherwise need to track both the mask 
and n.

cache_alloc_size() is independent of what caller requires so the full mask 
is not passed from the caller.

> I was expecting the calls to get_cbm_mask() and get_cache_size() within 
> cat_perf_miss_val() to be removed.

I would have wanted to remove get_cache_size() but it would mean removing 
cache size print or moving it to elsewhere.

get_cbm_mask() cannot be removed as it's used by the test to calculate the 
mask the test wants (but it no longer has to determine the size itself but 
uses this new helper instead).

I can try to amend the changelog to explain things better.

> > diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
> > index ae54bbabbd91..efe77e0f1d4c 100644
> > --- a/tools/testing/selftests/resctrl/cmt_test.c
> > +++ b/tools/testing/selftests/resctrl/cmt_test.c
> > @@ -105,10 +105,12 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
> >  		.cpu_no		= cpu_no,
> >  		.filename	= RESULT_FILE_NAME,
> >  		.mask		= ~(long_mask << n) & long_mask,
> > -		.span		= cache_size * n / count_of_bits,
> >  		.num_of_runs	= 0,
> >  		.setup		= cmt_setup,
> >  	};
> > +	ret = cache_alloc_size(cpu_no, "L3", param.mask, &param.span);
> > +	if (ret)
> > +		return ret;
> >  
> >  	if (strcmp(benchmark_cmd[0], "fill_buf") == 0)
> >  		sprintf(benchmark_cmd[1], "%lu", param.span);
> 
> Same here regarding removal of code.
> 
> > diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
> > index bcc95faa5b4e..65425d92684e 100644
> > --- a/tools/testing/selftests/resctrl/resctrl.h
> > +++ b/tools/testing/selftests/resctrl/resctrl.h
> > @@ -108,6 +108,8 @@ 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, unsigned long *mask);
> >  int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size);
> > +int cache_alloc_size(int cpu_no, char *cache_type, unsigned long alloc_mask,
> > +		     unsigned long *alloc_size);
> >  void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
> >  int signal_handler_register(void);
> >  void signal_handler_unregister(void);
> 
> 
> Reinette
>
Reinette Chatre April 24, 2023, 4:45 p.m. UTC | #3
Hi Ilpo,

On 4/24/2023 9:28 AM, Ilpo Järvinen wrote:
> On Fri, 21 Apr 2023, Reinette Chatre wrote:
> 
>> On 4/18/2023 4:44 AM, Ilpo Järvinen wrote:
>>> CAT and CMT tests calculate the span size from the n-bits cache
>>> allocation on their own.
>>>
>>> Add cache_alloc_size() helper which calculates size of the cache
>>> allocation for the given number of bits to avoid duplicating code.
>>
>> This patch is very heavy on the usage of allocation when I think it
>> only refers to the cache size ... how that size is used by the caller
>> is independent from this. 
>>
>> Compare to how it sounds with some small changes to changelog:
>>
>> 	CAT and CMT tests calculate the span size from the capacity
>> 	bitmask independently.
>> 	
>> 	Add cache_size() helper which calculates the size of the
>> 	cache for the given number of bits to avoid duplicating code.
>>
>> I think removing "alloc" helps to convey what this code actually does.
> 
> Does it? Without something to indicate its not the full cache size, 
> there's possiblity for confusion. While the tests are mostly interested 
> in the allocated size, the full cache size is also collected (solely for 
> printing it out, IIRC). Maybe I should rename those variable to 
> total_cache_size or something like that to mitigate the confusion?

This patch adds and use a utility that converts a bitmask into bytes.
I do not think it should dictate what the meaning or usage of the bitmask
is.

>>> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>>> ---
>>>  tools/testing/selftests/resctrl/cache.c    | 27 ++++++++++++++++++++++
>>>  tools/testing/selftests/resctrl/cat_test.c |  8 +++++--
>>>  tools/testing/selftests/resctrl/cmt_test.c |  4 +++-
>>>  tools/testing/selftests/resctrl/resctrl.h  |  2 ++
>>>  4 files changed, 38 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
>>> index 6bc912de38be..b983af394e33 100644
>>> --- a/tools/testing/selftests/resctrl/cache.c
>>> +++ b/tools/testing/selftests/resctrl/cache.c
>>> @@ -15,6 +15,33 @@ static struct read_format rf_cqm;
>>>  static int fd_lm;
>>>  char llc_occup_path[1024];
>>>  
>>> +/*
>>> + * cache_alloc_size - Calculate alloc size for given cache alloc mask
>>
>> "cache_size - Calculate number of bytes represented by bitmask" ?
>> Please feel free to improve.
>>
>>
>>> + * @cpu_no:		CPU number
>>> + * @cache_type:		Cache level L2/L3
>>> + * @alloc_mask:		Cache alloc mask
>>
>> The description is mostly a rewrite of the variable name. Can it be
>> more descriptive?
>>
>>> + * @alloc_size:		Alloc size returned on success
>>
>> I do not think the utility should assume anything about how
>> the value it provides should be used. Instead it should just reflect
>> what the value is.
> 
> I was just referring to that the value is filled only on success.

I understand. My comment was about the naming of the parameter, which can,
for example, just be "size".

>>> + * Returns: 0 on success with @alloc_size filled, non-zero on error.
>>> + */
>>> +int cache_alloc_size(int cpu_no, char *cache_type, unsigned long alloc_mask,
>>> +		     unsigned long *alloc_size)
>>> +{
>>> +	unsigned long cache_size, full_mask;
>>> +	int ret;
>>> +
>>> +	ret = get_cbm_mask(cache_type, &full_mask);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = get_cache_size(cpu_no, cache_type, &cache_size);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	*alloc_size = cache_size * count_bits(alloc_mask) / count_bits(full_mask);
>>> +	return 0;
>>> +}
>>> +
>>>  static void initialize_perf_event_attr(void)
>>>  {
>>>  	pea_llc_miss.type = PERF_TYPE_HARDWARE;
>>> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
>>> index 9bf5d05d9e74..d3fbd4de9f8a 100644
>>> --- a/tools/testing/selftests/resctrl/cat_test.c
>>> +++ b/tools/testing/selftests/resctrl/cat_test.c
>>> @@ -140,7 +140,9 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
>>>  	/* Set param values for parent thread which will be allocated bitmask
>>>  	 * with (max_bits - n) bits
>>>  	 */
>>> -	param.span = cache_size * (count_of_bits - n) / count_of_bits;
>>> +	ret = cache_alloc_size(cpu_no, cache_type, l_mask, &param.span);
>>> +	if (ret)
>>> +		return ret;
>>>  	strcpy(param.ctrlgrp, "c2");
>>>  	strcpy(param.mongrp, "m2");
>>>  	strcpy(param.filename, RESULT_FILE_NAME2);
>>> @@ -162,7 +164,9 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
>>>  		param.mask = l_mask_1;
>>>  		strcpy(param.ctrlgrp, "c1");
>>>  		strcpy(param.mongrp, "m1");
>>> -		param.span = cache_size * n / count_of_bits;
>>> +		ret = cache_alloc_size(cpu_no, cache_type, l_mask_1, &param.span);
>>> +		if (ret)
>>> +			exit(-1);
>>>  		strcpy(param.filename, RESULT_FILE_NAME1);
>>>  		param.num_of_runs = 0;
>>>  		param.cpu_no = sibling_cpu_no;
>>
>> Did this change intend to remove the duplicate code mentioned
>> in the changelog?
> 
> It removes n CBM bits -> cache size calculations by collecting the 
> calculation into one place.
> 
> cache_alloc_size() takes mask instead of n (CBM bits) as input which makes 
> things easier down the line when the new CAT test starts to tweak the 
> alloc size. The new CAT test would otherwise need to track both the mask 
> and n.
> 
> cache_alloc_size() is independent of what caller requires so the full mask 
> is not passed from the caller.
> 
>> I was expecting the calls to get_cbm_mask() and get_cache_size() within 
>> cat_perf_miss_val() to be removed.
> 
> I would have wanted to remove get_cache_size() but it would mean removing 
> cache size print or moving it to elsewhere.
> 
> get_cbm_mask() cannot be removed as it's used by the test to calculate the 
> mask the test wants (but it no longer has to determine the size itself but 
> uses this new helper instead).
> 
> I can try to amend the changelog to explain things better.

The current motivation for this patch is to avoid duplicating code but as I
see it it introduces more duplicated code.

Reinette
diff mbox series

Patch

diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
index 6bc912de38be..b983af394e33 100644
--- a/tools/testing/selftests/resctrl/cache.c
+++ b/tools/testing/selftests/resctrl/cache.c
@@ -15,6 +15,33 @@  static struct read_format rf_cqm;
 static int fd_lm;
 char llc_occup_path[1024];
 
+/*
+ * cache_alloc_size - Calculate alloc size for given cache alloc mask
+ * @cpu_no:		CPU number
+ * @cache_type:		Cache level L2/L3
+ * @alloc_mask:		Cache alloc mask
+ * @alloc_size:		Alloc size returned on success
+ *
+ * Returns: 0 on success with @alloc_size filled, non-zero on error.
+ */
+int cache_alloc_size(int cpu_no, char *cache_type, unsigned long alloc_mask,
+		     unsigned long *alloc_size)
+{
+	unsigned long cache_size, full_mask;
+	int ret;
+
+	ret = get_cbm_mask(cache_type, &full_mask);
+	if (ret)
+		return ret;
+
+	ret = get_cache_size(cpu_no, cache_type, &cache_size);
+	if (ret)
+		return ret;
+
+	*alloc_size = cache_size * count_bits(alloc_mask) / count_bits(full_mask);
+	return 0;
+}
+
 static void initialize_perf_event_attr(void)
 {
 	pea_llc_miss.type = PERF_TYPE_HARDWARE;
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 9bf5d05d9e74..d3fbd4de9f8a 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -140,7 +140,9 @@  int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
 	/* Set param values for parent thread which will be allocated bitmask
 	 * with (max_bits - n) bits
 	 */
-	param.span = cache_size * (count_of_bits - n) / count_of_bits;
+	ret = cache_alloc_size(cpu_no, cache_type, l_mask, &param.span);
+	if (ret)
+		return ret;
 	strcpy(param.ctrlgrp, "c2");
 	strcpy(param.mongrp, "m2");
 	strcpy(param.filename, RESULT_FILE_NAME2);
@@ -162,7 +164,9 @@  int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
 		param.mask = l_mask_1;
 		strcpy(param.ctrlgrp, "c1");
 		strcpy(param.mongrp, "m1");
-		param.span = cache_size * n / count_of_bits;
+		ret = cache_alloc_size(cpu_no, cache_type, l_mask_1, &param.span);
+		if (ret)
+			exit(-1);
 		strcpy(param.filename, RESULT_FILE_NAME1);
 		param.num_of_runs = 0;
 		param.cpu_no = sibling_cpu_no;
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
index ae54bbabbd91..efe77e0f1d4c 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -105,10 +105,12 @@  int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
 		.cpu_no		= cpu_no,
 		.filename	= RESULT_FILE_NAME,
 		.mask		= ~(long_mask << n) & long_mask,
-		.span		= cache_size * n / count_of_bits,
 		.num_of_runs	= 0,
 		.setup		= cmt_setup,
 	};
+	ret = cache_alloc_size(cpu_no, "L3", param.mask, &param.span);
+	if (ret)
+		return ret;
 
 	if (strcmp(benchmark_cmd[0], "fill_buf") == 0)
 		sprintf(benchmark_cmd[1], "%lu", param.span);
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index bcc95faa5b4e..65425d92684e 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -108,6 +108,8 @@  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, unsigned long *mask);
 int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size);
+int cache_alloc_size(int cpu_no, char *cache_type, unsigned long alloc_mask,
+		     unsigned long *alloc_size);
 void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
 int signal_handler_register(void);
 void signal_handler_unregister(void);