On 2010-08-02 16:43, Glauber Costa wrote:
> On Sat, Jul 31, 2010 at 01:55:10PM -1000, Zachary Amsden wrote:
>> On 07/31/2010 06:36 AM, Arjan Koers wrote:
>>> On 2010-07-31 13:53, Arjan Koers wrote:
>>>> The kernel boots successfully when CONFIG_PRINTK_TIME is not set.
>>>>
>>> The problem occurs when this message is printed:
>>>
>>> [    0.016000] kvm-clock: cpu 1, msr 0:1511c01, secondary cpu clock
>>>
>>> When I disable that printk, the kernel boots with
>>> CONFIG_PRINTK_TIME=y
>>>
>>> --- a/arch/x86/kernel/kvmclock.c
>>> +++ b/arch/x86/kernel/kvmclock.c
>>> @@ -131,8 +131,8 @@ static int kvm_register_clock(char *txt)
>>>     int low, high;
>>>     low = (int)__pa(&per_cpu(hv_clock, cpu)) | 1;
>>>     high = ((u64)__pa(&per_cpu(hv_clock, cpu))>>  32);
>>> -   printk(KERN_INFO "kvm-clock: cpu %d, msr %x:%x, %s\n",
>>> -          cpu, high, low, txt);
>>> +   /*printk(KERN_INFO "kvm-clock: cpu %d, msr %x:%x, %s\n",
>>> +          cpu, high, low, txt);*/
>>>
>>>     return native_write_msr_safe(msr_kvm_system_time, low, high);
>>>  }
>>>
>>> So the problem appears to be that the clock of the second CPU
>>> is used too soon (or that clock setup should finish earlier).
>>
>> That's almost hilarious.  The printk from setting up the kvm clock
>> is invoking the kvm clock before it is setup.
>>
>> There's no reason other printks couldn't do the same thing, however.
>> I think it's safest to keep an initialized flag and check for it
>> before attempting to return a meaningful value.
> 
> I was on vacations, just got back.
> 
> I think it is safe to just patch our own use of it. Before that, all other
> printks will be handled by the main cpu anyway, since it'll be the only one 
> active
> at the moment. The only possible offenders for this are us, and the cpu 
> initialization
> code, which is already fragile in multiple ways anyway.
> 
> A flag would only make things more complicated and dirty

Maybe you could add a sanity check in pvclock_clocksource_read
after 'do { ... } while (version != src->version)' that
returns last_value if offset is extremely large?


I've performed some more boot tests (about 20) with the patch that
moves the printk after native_write_msr_safe and it works for me.
Andre Przywara confirmed to me that it also fixes his problem.

A slightly modified version of the patch for 2.6.34.1 also works
(800+ successful boot cycles).

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to