[PATCH] x86, tsc: downgrade message for fast calibration failure

[PATCH] x86, tsc: downgrade message for fast calibration failure

[Paul Bolle]

When fast calibration of the TSC fails an error is printed:
    tsc: Fast TSC calibration failed

This message was printed at default level (ie, as a warning) before
commit c767a54ba0657e52e6edaa97cbe0b0a8bf1c1655 ("x86/debug: Add
KERN_<LEVEL> to bare printks, convert printks to pr_<level>"). But it is
more appropriate to print it at informational level. If fast calibration
fails that only means an alternative calibration strategy will be tried,
and if that strategy also fails only a warning is printed. Besides,
there's nothing one can really do after noticing a fast calibration
failure.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) This is a rather verbose commit explanation. For trivialities like
these I try to write a very short explanation. But here the verbosity is
meant to obfuscate my complete ignorance of the (calibration of the)
TSC.

1) By the way, it's debatable whether this message should be printed at
informational or at notice level. But the difference between KERN_NOTICE
and KERN_INFO is too subtle for me. I even find the difference between
KERN_ALERT and KERN_CRIT, and between KERN_ERR and KERN_WARNING a bit
subtle. Are there guidelines for choosing between those levels?

 arch/x86/kernel/tsc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index cfa5d4f..4f258ae 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -374,7 +374,7 @@ static unsigned long quick_pit_calibrate(void)
 			goto success;
 		}
 	}
-	pr_err("Fast TSC calibration failed\n");
+	pr_info("Fast TSC calibration failed\n");
 	return 0;
 
 success:
-- 
1.7.11.4

Re: [PATCH] x86, tsc: downgrade message for fast calibration failure

[Joe Perches]

On Wed, 2012-09-26 at 12:09 +0200, Paul Bolle wrote:
> When fast calibration of the TSC fails an error is printed:
>     tsc: Fast TSC calibration failed
[]
> 1) By the way, it's debatable whether this message should be printed at
> informational or at notice level. But the difference between KERN_NOTICE
> and KERN_INFO is too subtle for me. I even find the difference between
> KERN_ALERT and KERN_CRIT, and between KERN_ERR and KERN_WARNING a bit
> subtle. Are there guidelines for choosing between those levels?

The only guidelines I know are the comments in kern_levels.h
and Documentation/kernel-parameters.txt

#define KERN_EMERG	KERN_SOH "0"	/* system is unusable */
#define KERN_ALERT	KERN_SOH "1"	/* action must be taken immediately */
#define KERN_CRIT	KERN_SOH "2"	/* critical conditions */
#define KERN_ERR	KERN_SOH "3"	/* error conditions */
#define KERN_WARNING	KERN_SOH "4"	/* warning conditions */
#define KERN_NOTICE	KERN_SOH "5"	/* normal but significant condition */
#define KERN_INFO	KERN_SOH "6"	/* informational */
#define KERN_DEBUG	KERN_SOH "7"	/* debug-level messages */

> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
[]
> @@ -374,7 +374,7 @@ static unsigned long quick_pit_calibrate(void)
>  			goto success;
>  		}
>  	}
> -	pr_err("Fast TSC calibration failed\n");
> +	pr_info("Fast TSC calibration failed\n");
>  	return 0;

Perhaps wordsmithing failed to unsuccessful might make
the message seem less like an error and more like an
ordinary occurrence.

Perhaps:

 arch/x86/kernel/tsc.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index cfa5d4f..4e22540 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -374,7 +374,7 @@ static unsigned long quick_pit_calibrate(void)
 			goto success;
 		}
 	}
-	pr_err("Fast TSC calibration failed\n");
+	pr_info("Fast TSC calibration unsuccessful\n");
 	return 0;
 
 success:
@@ -521,7 +521,7 @@ unsigned long native_calibrate_tsc(void)
 
 		/* The alternative source failed as well, disable TSC */
 		if (tsc_ref_min == ULONG_MAX) {
-			pr_warn("HPET/PMTIMER calibration failed\n");
+			pr_warn("HPET/PMTIMER calibration unsuccessful\n");
 			return 0;
 		}
 
@@ -540,7 +540,7 @@ unsigned long native_calibrate_tsc(void)
 
 	/* The alternative source failed, use the PIT calibration value */
 	if (tsc_ref_min == ULONG_MAX) {
-		pr_warn("HPET/PMTIMER calibration failed. Using PIT calibration.\n");
+		pr_warn("HPET/PMTIMER calibration unsuccessful. Using PIT calibration.\n");
 		return tsc_pit_min;
 	}
 

Re: [PATCH] x86, tsc: downgrade message for fast calibration failure

[Paul Bolle]

On Wed, 2012-09-26 at 12:09 +0200, Paul Bolle wrote:
> When fast calibration of the TSC fails an error is printed:
>     tsc: Fast TSC calibration failed
> 
> This message was printed at default level (ie, as a warning) before
> commit c767a54ba0657e52e6edaa97cbe0b0a8bf1c1655 ("x86/debug: Add
> KERN_<LEVEL> to bare printks, convert printks to pr_<level>"). But it is
> more appropriate to print it at informational level. If fast calibration
> fails that only means an alternative calibration strategy will be tried,
> and if that strategy also fails only a warning is printed. Besides,
> there's nothing one can really do after noticing a fast calibration
> failure.
> 
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>

What's the status of this patch? Did anyone (besides Joe) have a look at
it? (I'm fine with the wordsmithing Joe suggested, by the way.)

> 0) This is a rather verbose commit explanation. For trivialities like
> these I try to write a very short explanation. But here the verbosity is
> meant to obfuscate my complete ignorance of the (calibration of the)
> TSC.
> 
> 1) By the way, it's debatable whether this message should be printed at
> informational or at notice level. But the difference between KERN_NOTICE
> and KERN_INFO is too subtle for me. I even find the difference between
> KERN_ALERT and KERN_CRIT, and between KERN_ERR and KERN_WARNING a bit
> subtle. Are there guidelines for choosing between those levels?
> 
>  arch/x86/kernel/tsc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index cfa5d4f..4f258ae 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -374,7 +374,7 @@ static unsigned long quick_pit_calibrate(void)
>  			goto success;
>  		}
>  	}
> -	pr_err("Fast TSC calibration failed\n");
> +	pr_info("Fast TSC calibration failed\n");
>  	return 0;
>  
>  success:


Paul Bolle