diff mbox series

[05/24] selftests/resctrl: Create cache_size() helper

Message ID	20231024092634.7122-6-ilpo.jarvinen@linux.intel.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2; 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>, Shuah Khan <shuah@kernel.org>, Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>, =?utf-8?q?Maciej_Wiecz=C3=B3r-R?= =?utf-8?q?etman?= <maciej.wieczor-retman@intel.com>, Fenghua Yu <fenghua.yu@intel.com> Cc: linux-kernel@vger.kernel.org, =?utf-8?q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com> Subject: [PATCH 05/24] selftests/resctrl: Create cache_size() helper Date: Tue, 24 Oct 2023 12:26:15 +0300 Message-Id: <20231024092634.7122-6-ilpo.jarvinen@linux.intel.com> In-Reply-To: <20231024092634.7122-1-ilpo.jarvinen@linux.intel.com> References: <20231024092634.7122-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: CAT test improvements & generalized test framework \| [00/24] selftests/resctrl: CAT test improvements & generalized test framework [01/24] selftests/resctrl: Split fill_buf to allow tests finer-grained control [02/24] selftests/resctrl: Refactor fill_buf functions [03/24] selftests/resctrl: Refactor get_cbm_mask() [04/24] selftests/resctrl: Mark get_cache_size() cache_type const [05/24] selftests/resctrl: Create cache_size() helper [06/24] selftests/resctrl: Exclude shareable bits from schemata in CAT test [07/24] selftests/resctrl: Split measure_cache_vals() function [08/24] selftests/resctrl: Split show_cache_info() to test specific and generic parts [09/24] selftests/resctrl: Remove unnecessary __u64 -> unsigned long conversion [10/24] selftests/resctrl: Remove nested calls in perf event handling [11/24] selftests/resctrl: Consolidate naming of perf event related things [12/24] selftests/resctrl: Improve perf init [13/24] selftests/resctrl: Convert perf related globals to locals [14/24] selftests/resctrl: Move cat_val() to cat_test.c and rename to cat_test() [15/24] selftests/resctrl: Read in less obvious order to defeat prefetch optimizations [16/24] selftests/resctrl: Rewrite Cache Allocation Technology (CAT) test [17/24] selftests/resctrl: Create struct for input parameter [18/24] selftests/resctrl: Introduce generalized test framework [19/24] selftests/resctrl: Pass write_schemata() resource instead of test name [20/24] selftests/resctrl: Add helper to convert L2/3 to integer [21/24] selftests/resctrl: Get resource id from cache id [22/24] selftests/resctrl: Add test groups and name L3 CAT test L3_CAT [23/24] selftests/resctrl: Add L2 CAT test [24/24] selftests/resctrl: Ignore failures from L2 CAT test with <= 2 bits

Commit Message

Ilpo Järvinen Oct. 24, 2023, 9:26 a.m. UTC

  CAT and CMT tests calculate the span size from the n-bits cache
allocation on their own.

Add cache_size() helper which calculates size of the cache portion for
the given number of bits and use it to replace the existing span
calculations. This also prepares for the new CAT test that will need to
determine the size of the cache portion also during results processing.

cache_size local variables were renamed out of the way to
cache_total_size.

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

Comments

Reinette Chatre Nov. 2, 2023, 5:47 p.m. UTC | #1

Hi Ilpo,

On 10/24/2023 2:26 AM, Ilpo Järvinen wrote:
> CAT and CMT tests calculate the span size from the n-bits cache
> allocation on their own.
> 
> Add cache_size() helper which calculates size of the cache portion for
> the given number of bits and use it to replace the existing span
> calculations. This also prepares for the new CAT test that will need to
> determine the size of the cache portion also during results processing.
> 
> cache_size local variables were renamed out of the way to
> cache_total_size.

Please do stick to imperative mood ... "Rename cache_size local
variables ..."


...

> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
> index 2f3f0ee439d8..da06b2d492f9 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -117,4 +117,18 @@ int show_cache_info(unsigned long sum_llc_val, int no_of_bits,
>  		    unsigned long max_diff_percent, unsigned long num_of_runs,
>  		    bool platform, bool cmt);
>  
> +/*
> + * cache_size - Calculate the size of a cache portion
> + * @cache_size:	Cache size in bytes
> + * @mask:	Cache portion mask
> + * @cache_mask:	Full bitmask for the cache
> + *
> + * Return: The size of the cache portion in bytes.
> + */
> +static inline int cache_size(unsigned long cache_size, unsigned long mask,
> +			     unsigned long cache_mask)
> +{
> +	return cache_size * count_bits(mask) / count_bits(cache_mask);
> +}
> +
>  #endif /* RESCTRL_H */


The get_cache_size() and cache_size() naming appears similar enough to me
to cause confusion. Considering the "portion" term above, what do you think
of "cache_portion_size()" or even "cache_portion_bytes()"?

Reinette

Ilpo Järvinen Nov. 3, 2023, 8:53 a.m. UTC | #2

On Thu, 2 Nov 2023, Reinette Chatre wrote:

> Hi Ilpo,
> 
> On 10/24/2023 2:26 AM, Ilpo Järvinen wrote:
> > CAT and CMT tests calculate the span size from the n-bits cache
> > allocation on their own.
> > 
> > Add cache_size() helper which calculates size of the cache portion for
> > the given number of bits and use it to replace the existing span
> > calculations. This also prepares for the new CAT test that will need to
> > determine the size of the cache portion also during results processing.
> > 
> > cache_size local variables were renamed out of the way to
> > cache_total_size.
> 
> Please do stick to imperative mood ... "Rename cache_size local
> variables ..."
> 
> 
> ...
> 
> > diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
> > index 2f3f0ee439d8..da06b2d492f9 100644
> > --- a/tools/testing/selftests/resctrl/resctrl.h
> > +++ b/tools/testing/selftests/resctrl/resctrl.h
> > @@ -117,4 +117,18 @@ int show_cache_info(unsigned long sum_llc_val, int no_of_bits,
> >  		    unsigned long max_diff_percent, unsigned long num_of_runs,
> >  		    bool platform, bool cmt);
> >  
> > +/*
> > + * cache_size - Calculate the size of a cache portion
> > + * @cache_size:	Cache size in bytes
> > + * @mask:	Cache portion mask
> > + * @cache_mask:	Full bitmask for the cache
> > + *
> > + * Return: The size of the cache portion in bytes.
> > + */
> > +static inline int cache_size(unsigned long cache_size, unsigned long mask,
> > +			     unsigned long cache_mask)
> > +{
> > +	return cache_size * count_bits(mask) / count_bits(cache_mask);
> > +}
> > +
> >  #endif /* RESCTRL_H */
> 
> 
> The get_cache_size() and cache_size() naming appears similar enough to me
> to cause confusion. Considering the "portion" term above, what do you think
> of "cache_portion_size()" or even "cache_portion_bytes()"?

Yes, I'm more than happy to rename them. This naming was what you 
suggested earlier. (I used cache_alloc_size() or something like that
initially and you were against using "alloc" in the name.)

Reinette Chatre Nov. 3, 2023, 10:49 p.m. UTC | #3

Hi Ilpo,

On 11/3/2023 1:53 AM, Ilpo Järvinen wrote:
> 
> Yes, I'm more than happy to rename them. This naming was what you 
> suggested earlier. (I used cache_alloc_size() or something like that
> initially and you were against using "alloc" in the name.)

My apologies for giving poor guidance. I cannot recall the thread behind
that guidance but looking at these changes together the cache_size() and
get_cache_size() naming does seem close enough to cause confusion.

Reinette

diff mbox series

Patch

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 4852bbda2e71..80861c362a53 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -91,7 +91,7 @@  int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
 {
 	unsigned long l_mask, l_mask_1;
 	int ret, pipefd[2], sibling_cpu_no;
-	unsigned long cache_size = 0;
+	unsigned long cache_total_size = 0;
 	unsigned long long_mask;
 	int count_of_bits;
 	char pipe_message;
@@ -103,10 +103,10 @@  int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
 		return ret;
 
 	/* Get L3/L2 cache size */
-	ret = get_cache_size(cpu_no, cache_type, &cache_size);
+	ret = get_cache_size(cpu_no, cache_type, &cache_total_size);
 	if (ret)
 		return ret;
-	ksft_print_msg("Cache size :%lu\n", cache_size);
+	ksft_print_msg("Cache size :%lu\n", cache_total_size);
 
 	/* Get max number of bits from default-cabm mask */
 	count_of_bits = count_bits(long_mask);
@@ -138,7 +138,7 @@  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
 	 */
-	span = cache_size * (count_of_bits - n) / count_of_bits;
+	span = cache_size(cache_total_size, l_mask, long_mask);
 	strcpy(param.ctrlgrp, "c2");
 	strcpy(param.mongrp, "m2");
 	strcpy(param.filename, RESULT_FILE_NAME2);
@@ -160,7 +160,7 @@  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");
-		span = cache_size * n / count_of_bits;
+		span = cache_size(cache_total_size, l_mask_1, long_mask);
 		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 a6c79edc33cd..e8997ff5bc04 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -72,7 +72,7 @@  int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd)
 {
 	const char * const *cmd = benchmark_cmd;
 	const char *new_cmd[BENCHMARK_ARGS];
-	unsigned long cache_size = 0;
+	unsigned long cache_total_size = 0;
 	unsigned long long_mask;
 	char *span_str = NULL;
 	int count_of_bits;
@@ -83,10 +83,10 @@  int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd)
 	if (ret)
 		return ret;
 
-	ret = get_cache_size(cpu_no, "L3", &cache_size);
+	ret = get_cache_size(cpu_no, "L3", &cache_total_size);
 	if (ret)
 		return ret;
-	ksft_print_msg("Cache size :%lu\n", cache_size);
+	ksft_print_msg("Cache size :%lu\n", cache_total_size);
 
 	count_of_bits = count_bits(long_mask);
 
@@ -107,7 +107,7 @@  int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd)
 		.setup		= cmt_setup,
 	};
 
-	span = cache_size * n / count_of_bits;
+	span = cache_size(cache_total_size, param.mask, long_mask);
 
 	if (strcmp(cmd[0], "fill_buf") == 0) {
 		/* Duplicate the command to be able to replace span in it */
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 2f3f0ee439d8..da06b2d492f9 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -117,4 +117,18 @@  int show_cache_info(unsigned long sum_llc_val, int no_of_bits,
 		    unsigned long max_diff_percent, unsigned long num_of_runs,
 		    bool platform, bool cmt);
 
+/*
+ * cache_size - Calculate the size of a cache portion
+ * @cache_size:	Cache size in bytes
+ * @mask:	Cache portion mask
+ * @cache_mask:	Full bitmask for the cache
+ *
+ * Return: The size of the cache portion in bytes.
+ */
+static inline int cache_size(unsigned long cache_size, unsigned long mask,
+			     unsigned long cache_mask)
+{
+	return cache_size * count_bits(mask) / count_bits(cache_mask);
+}
+
 #endif /* RESCTRL_H */