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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
