diff mbox series

selftests/resctrl: Change a few printed messages

Message ID 20210406005242.3248706-1-fenghua.yu@intel.com
State Superseded
Headers show
Series selftests/resctrl: Change a few printed messages | expand

Commit Message

Fenghua Yu April 6, 2021, 12:52 a.m. UTC
A few printed messages contain pass/fail strings which should be shown
in test results. Remove the pass/fail strings in the messages to avoid
confusion.

Add "\n" at the end of one printed message.

Suggested-by: Shuah Khan <shuah@kernel.org>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
This is a follow-up patch of recent resctrl selftest patches and can be
applied cleanly to:
git git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git
branch next.

 tools/testing/selftests/resctrl/cache.c     | 3 +--
 tools/testing/selftests/resctrl/mba_test.c  | 9 +++------
 tools/testing/selftests/resctrl/mbm_test.c  | 3 +--
 tools/testing/selftests/resctrl/resctrlfs.c | 7 ++-----
 4 files changed, 7 insertions(+), 15 deletions(-)

Comments

Shuah Khan April 7, 2021, 2:33 p.m. UTC | #1
On 4/5/21 6:52 PM, Fenghua Yu wrote:
> A few printed messages contain pass/fail strings which should be shown

> in test results. Remove the pass/fail strings in the messages to avoid

> confusion.

> 

> Add "\n" at the end of one printed message.

> 

> Suggested-by: Shuah Khan <shuah@kernel.org>

> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>

> ---

> This is a follow-up patch of recent resctrl selftest patches and can be

> applied cleanly to:

> git git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git

> branch next.

> 

>   tools/testing/selftests/resctrl/cache.c     | 3 +--

>   tools/testing/selftests/resctrl/mba_test.c  | 9 +++------

>   tools/testing/selftests/resctrl/mbm_test.c  | 3 +--

>   tools/testing/selftests/resctrl/resctrlfs.c | 7 ++-----

>   4 files changed, 7 insertions(+), 15 deletions(-)

> 

> diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c

> index 362e3a418caa..310bbc997c60 100644

> --- a/tools/testing/selftests/resctrl/cache.c

> +++ b/tools/testing/selftests/resctrl/cache.c

> @@ -301,8 +301,7 @@ int show_cache_info(unsigned long sum_llc_val, int no_of_bits,

>   	ret = platform && abs((int)diff_percent) > max_diff_percent &&

>   	      (cmt ? (abs(avg_diff) > max_diff) : true);

>   

> -	ksft_print_msg("%s cache miss rate within %d%%\n",

> -		       ret ? "Fail:" : "Pass:", max_diff_percent);

> +	ksft_print_msg("Check cache miss rate within %d%%\n", max_diff_percent);


You need %s and pass in the ret ? "Fail:" : "Pass:" result for the
message to read correctly.

I am seeing:

# Check kernel support for resctrl filesystem

It should say the following:

# Fail Check kernel support for resctrl filesystem


Same for other such messages.
>   

>   	ksft_print_msg("Percent diff=%d\n", abs((int)diff_percent));

>   	ksft_print_msg("Number of bits: %d\n", no_of_bits);

> diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c

> index 26f12ad4c663..a909a745754f 100644

> --- a/tools/testing/selftests/resctrl/mba_test.c

> +++ b/tools/testing/selftests/resctrl/mba_test.c

> @@ -80,9 +80,7 @@ static void show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc)

>   		avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc;

>   		avg_diff_per = (int)(avg_diff * 100);

>   

> -		ksft_print_msg("%s MBA: diff within %d%% for schemata %u\n",

> -			       avg_diff_per > MAX_DIFF_PERCENT ?

> -			       "Fail:" : "Pass:",

> +		ksft_print_msg("Check MBA diff within %d%% for schemata %u\n",

>   			       MAX_DIFF_PERCENT,

>   			       ALLOCATION_MAX - ALLOCATION_STEP * allocation);

>   

> @@ -93,10 +91,9 @@ static void show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc)

>   			failed = true;

>   	}

>   

> -	ksft_print_msg("%s schemata change using MBA\n",

> -		       failed ? "Fail:" : "Pass:");

> +	ksft_print_msg("Check schemata change using MBA\n");

>   	if (failed)

> -		ksft_print_msg("At least one test failed");

> +		ksft_print_msg("At least one test failed\n");

>   }

>   

>   static int check_results(void)

> diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c

> index 02b1ed03f1e5..e2e7ee4ec630 100644

> --- a/tools/testing/selftests/resctrl/mbm_test.c

> +++ b/tools/testing/selftests/resctrl/mbm_test.c

> @@ -37,8 +37,7 @@ show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, int span)

>   	avg_diff_per = (int)(avg_diff * 100);

>   

>   	ret = avg_diff_per > MAX_DIFF_PERCENT;

> -	ksft_print_msg("%s MBM: diff within %d%%\n",

> -		       ret ? "Fail:" : "Pass:", MAX_DIFF_PERCENT);

> +	ksft_print_msg("Check MBM diff within %d%%\n", MAX_DIFF_PERCENT);


Here

>   	ksft_print_msg("avg_diff_per: %d%%\n", avg_diff_per);

>   	ksft_print_msg("Span (MB): %d\n", span);

>   	ksft_print_msg("avg_bw_imc: %lu\n", avg_bw_imc);

> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c

> index ade5f2b8b843..91cb3c48a7da 100644

> --- a/tools/testing/selftests/resctrl/resctrlfs.c

> +++ b/tools/testing/selftests/resctrl/resctrlfs.c

> @@ -570,15 +570,12 @@ bool check_resctrlfs_support(void)

>   

>   	fclose(inf);

>   

> -	ksft_print_msg("%s kernel supports resctrl filesystem\n",

> -		       ret ? "Pass:" : "Fail:");

> -

> +	ksft_print_msg("Check kernel support for resctrl filesystem\n");


Here

>   	if (!ret)

>   		return ret;

>   

>   	dp = opendir(RESCTRL_PATH);

> -	ksft_print_msg("%s resctrl mountpoint \"%s\" exists\n",

> -		       dp ? "Pass:" : "Fail:", RESCTRL_PATH);

> +	ksft_print_msg("Check resctrl mountpoint \"%s\"\n", RESCTRL_PATH);


Here

>   	if (dp)

>   		closedir(dp);

>   

> 


thanks,
-- Shuah
Fenghua Yu April 7, 2021, 5:12 p.m. UTC | #2
Hi, Shuah,

On Wed, Apr 07, 2021 at 08:33:23AM -0600, Shuah Khan wrote:
> On 4/5/21 6:52 PM, Fenghua Yu wrote:

> > -	ksft_print_msg("%s cache miss rate within %d%%\n",

> > -		       ret ? "Fail:" : "Pass:", max_diff_percent);

> > +	ksft_print_msg("Check cache miss rate within %d%%\n", max_diff_percent);

> 

> You need %s and pass in the ret ? "Fail:" : "Pass:" result for the

> message to read correctly.


Should I keep the ":" after "Pass"/"Fail"?

> 

> I am seeing:

> 

> # Check kernel support for resctrl filesystem

> 

> It should say the following:

> 

> # Fail Check kernel support for resctrl filesystem


i.e. should the printed messages be like the following?
# Fail: Check kernel support for resctrl filesystem
or
# Pass: Check kernel support for resctrl filesystem

Thanks.

-Fenghua
Shuah Khan April 7, 2021, 6:15 p.m. UTC | #3
On 4/7/21 11:12 AM, Fenghua Yu wrote:
> Hi, Shuah,

> 

> On Wed, Apr 07, 2021 at 08:33:23AM -0600, Shuah Khan wrote:

>> On 4/5/21 6:52 PM, Fenghua Yu wrote:

>>> -	ksft_print_msg("%s cache miss rate within %d%%\n",

>>> -		       ret ? "Fail:" : "Pass:", max_diff_percent);

>>> +	ksft_print_msg("Check cache miss rate within %d%%\n", max_diff_percent);

>>

>> You need %s and pass in the ret ? "Fail:" : "Pass:" result for the

>> message to read correctly.

> 

> Should I keep the ":" after "Pass"/"Fail"?

> 


Yes please.

>>

>> I am seeing:

>>

>> # Check kernel support for resctrl filesystem

>>

>> It should say the following:

>>

>> # Fail Check kernel support for resctrl filesystem

> 

> i.e. should the printed messages be like the following?

> # Fail: Check kernel support for resctrl filesystem

> or

> # Pass: Check kernel support for resctrl filesystem

> 


This looks good.

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
index 362e3a418caa..310bbc997c60 100644
--- a/tools/testing/selftests/resctrl/cache.c
+++ b/tools/testing/selftests/resctrl/cache.c
@@ -301,8 +301,7 @@  int show_cache_info(unsigned long sum_llc_val, int no_of_bits,
 	ret = platform && abs((int)diff_percent) > max_diff_percent &&
 	      (cmt ? (abs(avg_diff) > max_diff) : true);
 
-	ksft_print_msg("%s cache miss rate within %d%%\n",
-		       ret ? "Fail:" : "Pass:", max_diff_percent);
+	ksft_print_msg("Check cache miss rate within %d%%\n", max_diff_percent);
 
 	ksft_print_msg("Percent diff=%d\n", abs((int)diff_percent));
 	ksft_print_msg("Number of bits: %d\n", no_of_bits);
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index 26f12ad4c663..a909a745754f 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -80,9 +80,7 @@  static void show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc)
 		avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc;
 		avg_diff_per = (int)(avg_diff * 100);
 
-		ksft_print_msg("%s MBA: diff within %d%% for schemata %u\n",
-			       avg_diff_per > MAX_DIFF_PERCENT ?
-			       "Fail:" : "Pass:",
+		ksft_print_msg("Check MBA diff within %d%% for schemata %u\n",
 			       MAX_DIFF_PERCENT,
 			       ALLOCATION_MAX - ALLOCATION_STEP * allocation);
 
@@ -93,10 +91,9 @@  static void show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc)
 			failed = true;
 	}
 
-	ksft_print_msg("%s schemata change using MBA\n",
-		       failed ? "Fail:" : "Pass:");
+	ksft_print_msg("Check schemata change using MBA\n");
 	if (failed)
-		ksft_print_msg("At least one test failed");
+		ksft_print_msg("At least one test failed\n");
 }
 
 static int check_results(void)
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index 02b1ed03f1e5..e2e7ee4ec630 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -37,8 +37,7 @@  show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, int span)
 	avg_diff_per = (int)(avg_diff * 100);
 
 	ret = avg_diff_per > MAX_DIFF_PERCENT;
-	ksft_print_msg("%s MBM: diff within %d%%\n",
-		       ret ? "Fail:" : "Pass:", MAX_DIFF_PERCENT);
+	ksft_print_msg("Check MBM diff within %d%%\n", MAX_DIFF_PERCENT);
 	ksft_print_msg("avg_diff_per: %d%%\n", avg_diff_per);
 	ksft_print_msg("Span (MB): %d\n", span);
 	ksft_print_msg("avg_bw_imc: %lu\n", avg_bw_imc);
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index ade5f2b8b843..91cb3c48a7da 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -570,15 +570,12 @@  bool check_resctrlfs_support(void)
 
 	fclose(inf);
 
-	ksft_print_msg("%s kernel supports resctrl filesystem\n",
-		       ret ? "Pass:" : "Fail:");
-
+	ksft_print_msg("Check kernel support for resctrl filesystem\n");
 	if (!ret)
 		return ret;
 
 	dp = opendir(RESCTRL_PATH);
-	ksft_print_msg("%s resctrl mountpoint \"%s\" exists\n",
-		       dp ? "Pass:" : "Fail:", RESCTRL_PATH);
+	ksft_print_msg("Check resctrl mountpoint \"%s\"\n", RESCTRL_PATH);
 	if (dp)
 		closedir(dp);