diff mbox series

[09/24] selftests/resctrl: Remove unnecessary __u64 -> unsigned long conversion

Message ID 20231024092634.7122-10-ilpo.jarvinen@linux.intel.com (mailing list archive)
State New
Headers show
Series selftests/resctrl: CAT test improvements & generalized test framework | expand

Commit Message

Ilpo Järvinen Oct. 24, 2023, 9:26 a.m. UTC 
Perf counters are __u64 but the code converts them to unsigned long
before printing them out.

Remove unnecessary type conversion and the potential loss of meaningful
bits due to different sizes of types.

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

Comments

Reinette Chatre Nov. 2, 2023, 5:48 p.m. UTC | #1 
Hi Ilpo,

On 10/24/2023 2:26 AM, Ilpo Järvinen wrote:
> Perf counters are __u64 but the code converts them to unsigned long
> before printing them out.
> 
> Remove unnecessary type conversion and the potential loss of meaningful
> bits due to different sizes of types.

This motivation is not clear to me. Is this work done in
preparation for 32 bit testing? This raises a lot more topics if
this is the case.

Reinette
Ilpo Järvinen Nov. 3, 2023, 9:19 a.m. UTC | #2 
On Thu, 2 Nov 2023, Reinette Chatre wrote:
> On 10/24/2023 2:26 AM, Ilpo Järvinen wrote:
> > Perf counters are __u64 but the code converts them to unsigned long
> > before printing them out.
> > 
> > Remove unnecessary type conversion and the potential loss of meaningful
> > bits due to different sizes of types.
> 
> This motivation is not clear to me. Is this work done in
> preparation for 32 bit testing? This raises a lot more topics if
> this is the case.

So you oppose stating that second part after "and"?

My main motivation was keeping the types consistent when I noted that the 
code does unnecessary conversion to unsigned long (that's the first part 
before "and").

Of course it has the added benefit of being 32-bit compatible but
it was just a thought I added "automatically" without paying much 
attention on whether it's realistic to assume resctrl selftest as whole 
would work with 32-bit or not (objectively, the after patch code is 32-bit 
compatible but it of course gives no guarantees about the rest of the 
resctrl selftest code base).
diff mbox series

Patch

diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
index 95489d4b42b7..d39ef4eebc37 100644
--- a/tools/testing/selftests/resctrl/cache.c
+++ b/tools/testing/selftests/resctrl/cache.c
@@ -84,9 +84,8 @@  static int reset_enable_llc_perf(pid_t pid, int cpu_no)
  *
  * Return: =0 on success.  <0 on failure.
  */
-static int get_llc_perf(unsigned long *llc_perf_miss)
+static int get_llc_perf(__u64 *llc_perf_miss)
 {
-	__u64 total_misses;
 	int ret;
 
 	/* Stop counters after one span to get miss rate */
@@ -99,8 +98,7 @@  static int get_llc_perf(unsigned long *llc_perf_miss)
 		return -1;
 	}
 
-	total_misses = rf_cqm.values[0].value;
-	*llc_perf_miss = total_misses;
+	*llc_perf_miss = rf_cqm.values[0].value;
 
 	return 0;
 }
@@ -148,14 +146,12 @@  static int get_llc_occu_resctrl(unsigned long *llc_occupancy)
  *
  * Return:		0 on success. non-zero on failure.
  */
-static int print_results_cache(char *filename, int bm_pid,
-			       unsigned long llc_value)
+static int print_results_cache(char *filename, int bm_pid, __u64 llc_value)
 {
 	FILE *fp;
 
 	if (strcmp(filename, "stdio") == 0 || strcmp(filename, "stderr") == 0) {
-		printf("Pid: %d \t LLC_value: %lu\n", bm_pid,
-		       llc_value);
+		printf("Pid: %d \t LLC_value: %llu\n", bm_pid, llc_value);
 	} else {
 		fp = fopen(filename, "a");
 		if (!fp) {
@@ -163,7 +159,7 @@  static int print_results_cache(char *filename, int bm_pid,
 
 			return errno;
 		}
-		fprintf(fp, "Pid: %d \t llc_value: %lu\n", bm_pid, llc_value);
+		fprintf(fp, "Pid: %d \t llc_value: %llu\n", bm_pid, llc_value);
 		fclose(fp);
 	}
 
@@ -172,7 +168,7 @@  static int print_results_cache(char *filename, int bm_pid,
 
 static int measure_llc_perf(struct resctrl_val_param *param, int bm_pid)
 {
-	unsigned long llc_perf_miss = 0;
+	__u64 llc_perf_miss = 0;
 	int ret;
 
 	/*
@@ -273,11 +269,10 @@  int cat_val(struct resctrl_val_param *param, size_t span)
  * @cache_span:		cache span
  * @lines:		cache span in lines or bytes
  */
-void show_cache_info(int no_of_bits, unsigned long avg_llc_val,
-		     size_t cache_span, bool lines)
+void show_cache_info(int no_of_bits, __u64 avg_llc_val, size_t cache_span, bool lines)
 {
 	ksft_print_msg("Number of bits: %d\n", no_of_bits);
-	ksft_print_msg("Average LLC val: %lu\n", avg_llc_val);
+	ksft_print_msg("Average LLC val: %llu\n", avg_llc_val);
 	ksft_print_msg("Cache span (%s): %zu\n", !lines ? "bytes" : "lines",
 		       cache_span);
 }
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 32f6d612a3e7..2106cc3601d9 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -41,12 +41,12 @@  static int cat_setup(struct resctrl_val_param *p)
 	return ret;
 }
 
-static int show_results_info(unsigned long sum_llc_val, int no_of_bits,
+static int show_results_info(__u64 sum_llc_val, int no_of_bits,
 			     unsigned long cache_span, unsigned long max_diff,
 			     unsigned long max_diff_percent, unsigned long num_of_runs,
 			     bool platform)
 {
-	unsigned long avg_llc_val = 0;
+	__u64 avg_llc_val = 0;
 	float diff_percent;
 	int ret;
 
@@ -68,7 +68,7 @@  static int show_results_info(unsigned long sum_llc_val, int no_of_bits,
 static int check_results(struct resctrl_val_param *param, size_t span)
 {
 	char *token_array[8], temp[512];
-	unsigned long sum_llc_perf_miss = 0;
+	__u64 sum_llc_perf_miss = 0;
 	int runs = 0, no_of_bits = 0;
 	FILE *fp;
 
@@ -93,7 +93,7 @@  static int check_results(struct resctrl_val_param *param, size_t span)
 		 * setup transition phase.
 		 */
 		if (runs > 0)
-			sum_llc_perf_miss += strtoul(token_array[3], NULL, 0);
+			sum_llc_perf_miss += strtoull(token_array[3], NULL, 0);
 		runs++;
 	}
 
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index c7655714b23f..033c49784581 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -115,8 +115,7 @@  unsigned int count_bits(unsigned long n);
 void cmt_test_cleanup(void);
 int get_core_sibling(int cpu_no);
 int measure_llc_resctrl(struct resctrl_val_param *param, int bm_pid);
-void show_cache_info(int no_of_bits, unsigned long avg_llc_val,
-		     size_t cache_span, bool lines);
+void show_cache_info(int no_of_bits, __u64 avg_llc_val, size_t cache_span, bool lines);
 
 /*
  * cache_size - Calculate the size of a cache portion