Re: 2.6.35-rc1 regression with pvclock and smp guests

Glauber Costa
2010-08-02T14:43:02Z

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