On Mon, Aug 02, 2010 at 10:26:30AM -1000, Zachary Amsden wrote: > On 08/02/2010 04:43 AM, 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 > Can we just do this? > Initialize hv_clock to zero > > This stops callers from getting random values if data is accessed before > clock is initialized; instead they will get zeroed clock values (because > computation involves a multiplication by a factor in hv_clock). > > Signed-off-by: Zachary Amsden <zamsden@xxxxxxxxxx> > > diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c > index eb9b76c..e7acd0d 100644 > --- a/arch/x86/kernel/kvmclock.c > +++ b/arch/x86/kernel/kvmclock.c > @@ -40,7 +40,7 @@ static int parse_no_kvmclock(char *arg) > early_param("no-kvmclock", parse_no_kvmclock); > > /* The hypervisor will put information about time periodically here */ > -static DEFINE_PER_CPU_SHARED_ALIGNED(struct pvclock_vcpu_time_info, hv_clock); > +static DEFINE_PER_CPU_SHARED_ALIGNED(struct pvclock_vcpu_time_info, hv_clock) = {0}; > static struct pvclock_wall_clock wall_clock; We can, but I am a little bit afraid that it won't initialize all the per-cpu areas. If it does, it is fine, though. -- 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