All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nikola Ciprich <extmaillist@linuxbox.cz>
To: Zachary Amsden <zamsden@redhat.com>
Cc: KVM list <kvm@vger.kernel.org>,
	Linux kernel list <linux-kernel@vger.kernel.org>,
	nikola.ciprich@linuxbox.cz, Avi Kivity <avi@redhat.com>,
	Glauber Costa <glommer@redhat.com>
Subject: Re: [PATCHv2] fix regression caused by e48672fa25e879f7ae21785c7efd187738139593
Date: Sat, 26 Mar 2011 15:47:29 +0100	[thread overview]
Message-ID: <20110326144729.GA29447@nik-comp.lan> (raw)
In-Reply-To: <4D8C503D.6000607@redhat.com>

> So something bothers me still about this bug.  What you did correctly  
> restores the old behavior - but it shouldn't be fixing a bug.
>
> The only reason you need to schedule an update for the KVM clock area is  
> if a new VCPU has been created, you have an unstable TSC.. or something  
> changes the VM's kvmclock offset.
>
> So this change could in fact be hiding an underlying bug - either an  
> unstable TSC is not being properly reported, the KVM clock offset is  
> being changed, we are missing a KVM clock update for secondary VCPUs -  
> or something else we don't yet understand is going on.
>
> Nikola, can you try the patch below, which reverts your change and  
> attempts to fix other possible sources of the problem, and see if it  
> still reproduces?
with Your patch, 32b SMP guests boot fine as well, without it, they
don't, so if You consider this better fix, I can acknowledge it as
working :)
n.

>
> Thanks,
>
> Zach

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 58f517b..42618fb 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2127,8 +2127,10 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>  		if (check_tsc_unstable()) {
>  			kvm_x86_ops->adjust_tsc_offset(vcpu, -tsc_delta);
>  			vcpu->arch.tsc_catchup = 1;
> +			kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
>  		}
> -		kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
> +		if (vcpu->cpu == -1)
> +			kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
>  		if (vcpu->cpu != cpu)
>  			kvm_migrate_timers(vcpu);
>  		vcpu->cpu = cpu;
> @@ -3534,6 +3536,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
>  		struct kvm_clock_data user_ns;
>  		u64 now_ns;
>  		s64 delta;
> +		struct kvm_vcpu *vcpu;
> +		int i;
>  
>  		r = -EFAULT;
>  		if (copy_from_user(&user_ns, argp, sizeof(user_ns)))
> @@ -3549,6 +3553,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
>  		delta = user_ns.clock - now_ns;
>  		local_irq_enable();
>  		kvm->arch.kvmclock_offset = delta;
> +		kvm_for_each_vcpu(i, vcpu, kvm)
> +			kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
>  		break;
>  	}
>  	case KVM_GET_CLOCK: {


-- 
-------------------------------------
Ing. Nikola CIPRICH
LinuxBox.cz, s.r.o.
28. rijna 168, 709 01 Ostrava

tel.:   +420 596 603 142
fax:    +420 596 621 273
mobil:  +420 777 093 799

www.linuxbox.cz

mobil servis: +420 737 238 656
email servis: servis@linuxbox.cz
-------------------------------------

  reply	other threads:[~2011-03-26 14:47 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-07 10:18 [PATCH] add missing guest clock update removed by e48672fa25e879f7ae21785c7efd187738139593 Nikola Ciprich
2011-03-08 19:58 ` Zachary Amsden
2011-03-09 19:30   ` Nikola Ciprich
2011-03-09 21:29     ` Zachary Amsden
2011-03-09 22:36       ` [PATCHv2] fix regression caused " Nikola Ciprich
2011-03-10  7:01         ` Zachary Amsden
2011-03-10  9:07         ` Avi Kivity
2011-03-25  8:20         ` Zachary Amsden
2011-03-26 14:47           ` Nikola Ciprich [this message]
2011-04-11 16:12           ` Nikola Ciprich
2011-04-12 15:36             ` Zachary Amsden

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110326144729.GA29447@nik-comp.lan \
    --to=extmaillist@linuxbox.cz \
    --cc=avi@redhat.com \
    --cc=glommer@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nikola.ciprich@linuxbox.cz \
    --cc=zamsden@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.