diff mbox

invalid printk time during boot on PA-RISC

Message ID 20160603172231.GA4592@ls3530.box (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Helge Deller June 3, 2016, 5:22 p.m. UTC 
* Aaro Koskinen <aaro.koskinen@iki.fi>:
> Hi,
> 
> On Wed, Jun 01, 2016 at 11:12:48PM +0200, Helge Deller wrote:
> > It fixes the problem mostly for me, there is just one little glitch
> > left, which is that the lines "FP[x] enabled" will still print an
> > invalid time. I assume it's because when this text is printed, the
> > clock/interrupts for the just booted CPU aren't enabled yet.
> > 
> > I'm not sure it's worth trying to fix that. 
> > One could print out that info at a later time, or just disabled it.
> > 
> > Helge
> > 
> > [    5.720014] Enabling PDC_PAT chassis codes support v0.05
> > [    6.644542] Releasing cpu 1 now, hpa=fffffffffe781000
> > [    0.196000] FP[1] enabled: Rev 1 Model 20
> > [    7.088432] CPU(s): 2 out of 2 PA8800 (Mako) at 900.000000 MHz online
> 
> Some tools that analyze boot time based on dmesg might get confused.

Yes, valid point.
Updated patch below.

Helge

Comments

Aaro Koskinen June 3, 2016, 9:56 p.m. UTC | #1 
Hi,

On Fri, Jun 03, 2016 at 07:22:31PM +0200, Helge Deller wrote:
> * Aaro Koskinen <aaro.koskinen@iki.fi>:
> > Hi,
> > 
> > On Wed, Jun 01, 2016 at 11:12:48PM +0200, Helge Deller wrote:
> > > It fixes the problem mostly for me, there is just one little glitch
> > > left, which is that the lines "FP[x] enabled" will still print an
> > > invalid time. I assume it's because when this text is printed, the
> > > clock/interrupts for the just booted CPU aren't enabled yet.
> > > 
> > > I'm not sure it's worth trying to fix that. 
> > > One could print out that info at a later time, or just disabled it.
> > > 
> > > Helge
> > > 
> > > [    5.720014] Enabling PDC_PAT chassis codes support v0.05
> > > [    6.644542] Releasing cpu 1 now, hpa=fffffffffe781000
> > > [    0.196000] FP[1] enabled: Rev 1 Model 20
> > > [    7.088432] CPU(s): 2 out of 2 PA8800 (Mako) at 900.000000 MHz online
> > 
> > Some tools that analyze boot time based on dmesg might get confused.
> 
> Yes, valid point.
> Updated patch below.
> 
> Helge
> 
> __________
> 
> 
> parisc: Fix printk time during boot
> 
> Avoid showing invalid printk time stamps during boot.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>

Reviewed-by: Aaro Koskinen <aaro.koskinen@iki.fi>

A.

> diff --git a/arch/parisc/kernel/processor.c b/arch/parisc/kernel/processor.c
> index e81ccf1..5adc339 100644
> --- a/arch/parisc/kernel/processor.c
> +++ b/arch/parisc/kernel/processor.c
> @@ -324,8 +324,9 @@ int init_per_cpu(int cpunum)
>  		per_cpu(cpu_data, cpunum).fp_rev = coproc_cfg.revision;
>  		per_cpu(cpu_data, cpunum).fp_model = coproc_cfg.model;
>  
> -		printk(KERN_INFO  "FP[%d] enabled: Rev %ld Model %ld\n",
> -			cpunum, coproc_cfg.revision, coproc_cfg.model);
> +		if (cpunum == 0)
> +			printk(KERN_INFO  "FP[%d] enabled: Rev %ld Model %ld\n",
> +				cpunum, coproc_cfg.revision, coproc_cfg.model);
>  
>  		/*
>  		** store status register to stack (hopefully aligned)
> diff --git a/arch/parisc/kernel/time.c b/arch/parisc/kernel/time.c
> index 58dd680..31ec99a 100644
> --- a/arch/parisc/kernel/time.c
> +++ b/arch/parisc/kernel/time.c
> @@ -309,11 +309,6 @@ void __init time_init(void)
>  	clocks_calc_mult_shift(&cyc2ns_mul, &cyc2ns_shift, current_cr16_khz,
>  				NSEC_PER_MSEC, 0);
>  
> -#if defined(CONFIG_HAVE_UNSTABLE_SCHED_CLOCK) && defined(CONFIG_64BIT)
> -	/* At bootup only one 64bit CPU is online and cr16 is "stable" */
> -	set_sched_clock_stable();
> -#endif
> -
>  	start_cpu_itimer();	/* get CPU 0 started */
>  
>  	/* register at clocksource framework */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mikulas Patocka June 6, 2016, 4:51 p.m. UTC | #2 
On Fri, 3 Jun 2016, Helge Deller wrote:

> * Aaro Koskinen <aaro.koskinen@iki.fi>:
> > Hi,
> > 
> > On Wed, Jun 01, 2016 at 11:12:48PM +0200, Helge Deller wrote:
> > > It fixes the problem mostly for me, there is just one little glitch
> > > left, which is that the lines "FP[x] enabled" will still print an
> > > invalid time. I assume it's because when this text is printed, the
> > > clock/interrupts for the just booted CPU aren't enabled yet.
> > > 
> > > I'm not sure it's worth trying to fix that. 
> > > One could print out that info at a later time, or just disabled it.
> > > 
> > > Helge
> > > 
> > > [    5.720014] Enabling PDC_PAT chassis codes support v0.05
> > > [    6.644542] Releasing cpu 1 now, hpa=fffffffffe781000
> > > [    0.196000] FP[1] enabled: Rev 1 Model 20
> > > [    7.088432] CPU(s): 2 out of 2 PA8800 (Mako) at 900.000000 MHz online
> > 
> > Some tools that analyze boot time based on dmesg might get confused.
> 
> Yes, valid point.
> Updated patch below.
> 
> Helge
> 
> __________
> 
> 
> parisc: Fix printk time during boot
> 
> Avoid showing invalid printk time stamps during boot.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> diff --git a/arch/parisc/kernel/processor.c b/arch/parisc/kernel/processor.c
> index e81ccf1..5adc339 100644
> --- a/arch/parisc/kernel/processor.c
> +++ b/arch/parisc/kernel/processor.c
> @@ -324,8 +324,9 @@ int init_per_cpu(int cpunum)
>  		per_cpu(cpu_data, cpunum).fp_rev = coproc_cfg.revision;
>  		per_cpu(cpu_data, cpunum).fp_model = coproc_cfg.model;
>  
> -		printk(KERN_INFO  "FP[%d] enabled: Rev %ld Model %ld\n",
> -			cpunum, coproc_cfg.revision, coproc_cfg.model);
> +		if (cpunum == 0)
> +			printk(KERN_INFO  "FP[%d] enabled: Rev %ld Model %ld\n",
> +				cpunum, coproc_cfg.revision, coproc_cfg.model);

BTW. you could also remove "[%d] cpunum" from printk --- if cpunum is 
always 0 here.

Mikulas

>  
>  		/*
>  		** store status register to stack (hopefully aligned)
> diff --git a/arch/parisc/kernel/time.c b/arch/parisc/kernel/time.c
> index 58dd680..31ec99a 100644
> --- a/arch/parisc/kernel/time.c
> +++ b/arch/parisc/kernel/time.c
> @@ -309,11 +309,6 @@ void __init time_init(void)
>  	clocks_calc_mult_shift(&cyc2ns_mul, &cyc2ns_shift, current_cr16_khz,
>  				NSEC_PER_MSEC, 0);
>  
> -#if defined(CONFIG_HAVE_UNSTABLE_SCHED_CLOCK) && defined(CONFIG_64BIT)
> -	/* At bootup only one 64bit CPU is online and cr16 is "stable" */
> -	set_sched_clock_stable();
> -#endif
> -
>  	start_cpu_itimer();	/* get CPU 0 started */
>  
>  	/* register at clocksource framework */
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Helge Deller June 7, 2016, 4:47 p.m. UTC | #3 
On 06.06.2016 18:51, Mikulas Patocka wrote:
> 
> 
> On Fri, 3 Jun 2016, Helge Deller wrote:
> 
>> * Aaro Koskinen <aaro.koskinen@iki.fi>:
>>> Hi,
>>>
>>> On Wed, Jun 01, 2016 at 11:12:48PM +0200, Helge Deller wrote:
>>>> It fixes the problem mostly for me, there is just one little glitch
>>>> left, which is that the lines "FP[x] enabled" will still print an
>>>> invalid time. I assume it's because when this text is printed, the
>>>> clock/interrupts for the just booted CPU aren't enabled yet.
>>>>
>>>> I'm not sure it's worth trying to fix that. 
>>>> One could print out that info at a later time, or just disabled it.
>>>>
>>>> Helge
>>>>
>>>> [    5.720014] Enabling PDC_PAT chassis codes support v0.05
>>>> [    6.644542] Releasing cpu 1 now, hpa=fffffffffe781000
>>>> [    0.196000] FP[1] enabled: Rev 1 Model 20
>>>> [    7.088432] CPU(s): 2 out of 2 PA8800 (Mako) at 900.000000 MHz online
>>>
>>> Some tools that analyze boot time based on dmesg might get confused.
>>
>> Yes, valid point.
>> Updated patch below.
>>
>> Helge
>>
>> __________
>>
>>
>> parisc: Fix printk time during boot
>>
>> Avoid showing invalid printk time stamps during boot.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> diff --git a/arch/parisc/kernel/processor.c b/arch/parisc/kernel/processor.c
>> index e81ccf1..5adc339 100644
>> --- a/arch/parisc/kernel/processor.c
>> +++ b/arch/parisc/kernel/processor.c
>> @@ -324,8 +324,9 @@ int init_per_cpu(int cpunum)
>>  		per_cpu(cpu_data, cpunum).fp_rev = coproc_cfg.revision;
>>  		per_cpu(cpu_data, cpunum).fp_model = coproc_cfg.model;
>>  
>> -		printk(KERN_INFO  "FP[%d] enabled: Rev %ld Model %ld\n",
>> -			cpunum, coproc_cfg.revision, coproc_cfg.model);
>> +		if (cpunum == 0)
>> +			printk(KERN_INFO  "FP[%d] enabled: Rev %ld Model %ld\n",
>> +				cpunum, coproc_cfg.revision, coproc_cfg.model);
> 
> BTW. you could also remove "[%d] cpunum" from printk --- if cpunum is 
> always 0 here.

Right. But I pushed my patch already upstream.
Maybe in the next round, or we drop this line completely.

Helge

--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/parisc/kernel/processor.c b/arch/parisc/kernel/processor.c
index e81ccf1..5adc339 100644
--- a/arch/parisc/kernel/processor.c
+++ b/arch/parisc/kernel/processor.c
@@ -324,8 +324,9 @@  int init_per_cpu(int cpunum)
 		per_cpu(cpu_data, cpunum).fp_rev = coproc_cfg.revision;
 		per_cpu(cpu_data, cpunum).fp_model = coproc_cfg.model;
 
-		printk(KERN_INFO  "FP[%d] enabled: Rev %ld Model %ld\n",
-			cpunum, coproc_cfg.revision, coproc_cfg.model);
+		if (cpunum == 0)
+			printk(KERN_INFO  "FP[%d] enabled: Rev %ld Model %ld\n",
+				cpunum, coproc_cfg.revision, coproc_cfg.model);
 
 		/*
 		** store status register to stack (hopefully aligned)
diff --git a/arch/parisc/kernel/time.c b/arch/parisc/kernel/time.c
index 58dd680..31ec99a 100644
--- a/arch/parisc/kernel/time.c
+++ b/arch/parisc/kernel/time.c
@@ -309,11 +309,6 @@  void __init time_init(void)
 	clocks_calc_mult_shift(&cyc2ns_mul, &cyc2ns_shift, current_cr16_khz,
 				NSEC_PER_MSEC, 0);
 
-#if defined(CONFIG_HAVE_UNSTABLE_SCHED_CLOCK) && defined(CONFIG_64BIT)
-	/* At bootup only one 64bit CPU is online and cr16 is "stable" */
-	set_sched_clock_stable();
-#endif
-
 	start_cpu_itimer();	/* get CPU 0 started */
 
 	/* register at clocksource framework */