| Message ID | 20130820182012.GA814@amt.cnet (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Il 20/08/2013 20:20, Marcelo Tosatti ha scritto: > > The offset to add to the hosts monotonic time, kvmclock_offset, is > calculated against the monotonic time at KVM_SET_CLOCK ioctl time. > > Request a master clock update at this time, to reduce a potentially > unbounded difference between the values of the masterclock and > the clock value used to calculate kvmclock_offset. > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > Index: linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c > =================================================================== > --- linux-2.6-kvmclock-fixes.orig/arch/x86/kvm/x86.c > +++ linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c > @@ -3806,6 +3806,7 @@ long kvm_arch_vm_ioctl(struct file *filp > delta = user_ns.clock - now_ns; > local_irq_enable(); > kvm->arch.kvmclock_offset = delta; > + kvm_gen_update_masterclock(kvm); > break; > } > case KVM_GET_CLOCK: { Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> While reviewing this patch, which BTW looks good, I noticed the handling of KVM_REQ_MCLOCK_INPROGRESS, the dummy request that is never processed and is only used to block guest entry. It seems to me that this bit is not necessary. After KVM_REQ_CLOCK_UPDATE is issued, no guest entries will happen because kvm_guest_time_update will try to take the pvclock_gtod_sync_lock, currently taken by kvm_gen_update_masterclock. Thus, you do not need the dummy request. You can simply issue KVM_REQ_CLOCK_UPDATE before calling pvclock_update_vm_gtod_copy (with the side effect of exiting VCPUs). VCPUs will stall in kvm_guest_time_update until pvclock_gtod_sync_lock is released by kvm_gen_update_masterclock. What do you think? On top of this, optionally the spinlock could become an rw_semaphore so that clock updates for different VCPUs will not be serialized. The effect is probably not visible, though. Paolo > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 22/08/2013 19:05, Paolo Bonzini ha scritto: > Il 20/08/2013 20:20, Marcelo Tosatti ha scritto: >> >> The offset to add to the hosts monotonic time, kvmclock_offset, is >> calculated against the monotonic time at KVM_SET_CLOCK ioctl time. >> >> Request a master clock update at this time, to reduce a potentially >> unbounded difference between the values of the masterclock and >> the clock value used to calculate kvmclock_offset. >> >> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> >> >> Index: linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c >> =================================================================== >> --- linux-2.6-kvmclock-fixes.orig/arch/x86/kvm/x86.c >> +++ linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c >> @@ -3806,6 +3806,7 @@ long kvm_arch_vm_ioctl(struct file *filp >> delta = user_ns.clock - now_ns; >> local_irq_enable(); >> kvm->arch.kvmclock_offset = delta; >> + kvm_gen_update_masterclock(kvm); >> break; >> } >> case KVM_GET_CLOCK: { > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Actually, what commit is this patch based on? You are calling the function at line 3799, but it is only declared at line 5786, and this causes: /root/kvm-kmod/x86/x86.c: In function ‘kvm_arch_vm_ioctl’: /root/kvm-kmod/x86/x86.c:3799:3: error: implicit declaration of function ‘kvm_gen_update_masterclock’ /root/kvm-kmod/x86/x86.c: At top level: /root/kvm-kmod/x86/x86.c:5786:13: warning: conflicting types for ‘kvm_gen_update_masterclock’ /root/kvm-kmod/x86/x86.c:5786:13: error: static declaration of ‘kvm_gen_update_masterclock’ follows non-static declaration /root/kvm-kmod/x86/x86.c:3799:3: note: previous implicit declaration of ‘kvm_gen_update_masterclock’ was here Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 22, 2013 at 07:05:20PM +0200, Paolo Bonzini wrote: > Il 20/08/2013 20:20, Marcelo Tosatti ha scritto: > > > > The offset to add to the hosts monotonic time, kvmclock_offset, is > > calculated against the monotonic time at KVM_SET_CLOCK ioctl time. > > > > Request a master clock update at this time, to reduce a potentially > > unbounded difference between the values of the masterclock and > > the clock value used to calculate kvmclock_offset. > > > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > > > Index: linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c > > =================================================================== > > --- linux-2.6-kvmclock-fixes.orig/arch/x86/kvm/x86.c > > +++ linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c > > @@ -3806,6 +3806,7 @@ long kvm_arch_vm_ioctl(struct file *filp > > delta = user_ns.clock - now_ns; > > local_irq_enable(); > > kvm->arch.kvmclock_offset = delta; > > + kvm_gen_update_masterclock(kvm); > > break; > > } > > case KVM_GET_CLOCK: { > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > > While reviewing this patch, which BTW looks good, I noticed the handling > of KVM_REQ_MCLOCK_INPROGRESS, the dummy request that is never processed > and is only used to block guest entry. > > It seems to me that this bit is not necessary. After > KVM_REQ_CLOCK_UPDATE is issued, no guest entries will happen because > kvm_guest_time_update will try to take the pvclock_gtod_sync_lock, > currently taken by kvm_gen_update_masterclock. Not entirely clear, to cancel guest entry the bit is necessary: if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests || need_resched() || signal_pending(current)) { vcpu->mode = OUTSIDE_GUEST_MODE; smp_wmb(); local_irq_enable(); preempt_enable(); r = 1; goto cancel_injection; } > Thus, you do not need the dummy request. You can simply issue > KVM_REQ_CLOCK_UPDATE before calling pvclock_update_vm_gtod_copy (with > the side effect of exiting VCPUs). VCPUs will stall in > kvm_guest_time_update until pvclock_gtod_sync_lock is released by > kvm_gen_update_masterclock. What do you think? Not sure its safe. Can you describe the safety of your proposal in more detail ? > On top of this, optionally the spinlock could become an rw_semaphore so > that clock updates for different VCPUs will not be serialized. The > effect is probably not visible, though. Still not clear of the benefits, but this area certainly welcomes performance improvements (the global kick is one thing we discussed and that should be improved). -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 23, 2013 at 12:00:46PM +0200, Paolo Bonzini wrote: > Il 22/08/2013 19:05, Paolo Bonzini ha scritto: > > Il 20/08/2013 20:20, Marcelo Tosatti ha scritto: > >> > >> The offset to add to the hosts monotonic time, kvmclock_offset, is > >> calculated against the monotonic time at KVM_SET_CLOCK ioctl time. > >> > >> Request a master clock update at this time, to reduce a potentially > >> unbounded difference between the values of the masterclock and > >> the clock value used to calculate kvmclock_offset. > >> > >> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > >> > >> Index: linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c > >> =================================================================== > >> --- linux-2.6-kvmclock-fixes.orig/arch/x86/kvm/x86.c > >> +++ linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c > >> @@ -3806,6 +3806,7 @@ long kvm_arch_vm_ioctl(struct file *filp > >> delta = user_ns.clock - now_ns; > >> local_irq_enable(); > >> kvm->arch.kvmclock_offset = delta; > >> + kvm_gen_update_masterclock(kvm); > >> break; > >> } > >> case KVM_GET_CLOCK: { > > > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > > Actually, what commit is this patch based on? You are calling the > function at line 3799, but it is only declared at line 5786, and this > causes: > > /root/kvm-kmod/x86/x86.c: In function ‘kvm_arch_vm_ioctl’: > /root/kvm-kmod/x86/x86.c:3799:3: error: implicit declaration of function ‘kvm_gen_update_masterclock’ > /root/kvm-kmod/x86/x86.c: At top level: > /root/kvm-kmod/x86/x86.c:5786:13: warning: conflicting types for ‘kvm_gen_update_masterclock’ > /root/kvm-kmod/x86/x86.c:5786:13: error: static declaration of ‘kvm_gen_update_masterclock’ follows non-static declaration > /root/kvm-kmod/x86/x86.c:3799:3: note: previous implicit declaration of ‘kvm_gen_update_masterclock’ was here > > Paolo Wrong tree, my bad, sorry Paolo (sent v2). -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 28/08/2013 04:52, Marcelo Tosatti ha scritto: > On Thu, Aug 22, 2013 at 07:05:20PM +0200, Paolo Bonzini wrote: >> Il 20/08/2013 20:20, Marcelo Tosatti ha scritto: >>> >>> The offset to add to the hosts monotonic time, kvmclock_offset, is >>> calculated against the monotonic time at KVM_SET_CLOCK ioctl time. >>> >>> Request a master clock update at this time, to reduce a potentially >>> unbounded difference between the values of the masterclock and >>> the clock value used to calculate kvmclock_offset. >>> >>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> >>> >>> Index: linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c >>> =================================================================== >>> --- linux-2.6-kvmclock-fixes.orig/arch/x86/kvm/x86.c >>> +++ linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c >>> @@ -3806,6 +3806,7 @@ long kvm_arch_vm_ioctl(struct file *filp >>> delta = user_ns.clock - now_ns; >>> local_irq_enable(); >>> kvm->arch.kvmclock_offset = delta; >>> + kvm_gen_update_masterclock(kvm); >>> break; >>> } >>> case KVM_GET_CLOCK: { >> >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >> >> While reviewing this patch, which BTW looks good, I noticed the handling >> of KVM_REQ_MCLOCK_INPROGRESS, the dummy request that is never processed >> and is only used to block guest entry. >> >> It seems to me that this bit is not necessary. After >> KVM_REQ_CLOCK_UPDATE is issued, no guest entries will happen because >> kvm_guest_time_update will try to take the pvclock_gtod_sync_lock, >> currently taken by kvm_gen_update_masterclock. > > Not entirely clear, to cancel guest entry the bit is necessary: > > if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests > || need_resched() || signal_pending(current)) { > vcpu->mode = OUTSIDE_GUEST_MODE; > smp_wmb(); > local_irq_enable(); > preempt_enable(); > r = 1; > goto cancel_injection; > } Yes, KVM_REQ_CLOCK_UPDATE is enough to cancel guest entries too. See code below. >> Thus, you do not need the dummy request. You can simply issue >> KVM_REQ_CLOCK_UPDATE before calling pvclock_update_vm_gtod_copy (with >> the side effect of exiting VCPUs). VCPUs will stall in >> kvm_guest_time_update until pvclock_gtod_sync_lock is released by >> kvm_gen_update_masterclock. What do you think? > > Not sure its safe. Can you describe the safety of your proposal in more > detail ? Here is the code I was thinking of: spin_lock(&ka->pvclock_gtod_sync_lock); make_all_cpus_request(kvm, KVM_REQ_CLOCK_UPDATE); /* * No guest entries from this point: VCPUs will be spinning * on pvclock_gtod_sync_lock in kvm_guest_time_update. */ pvclock_update_vm_gtod_copy(kvm); /* * Let kvm_guest_time_update continue: entering the guest * is now allowed too. */ spin_unlock(&ka->pvclock_gtod_sync_lock); KVM_REQ_CLOCK_UPDATE is used to cancel guest entry and execute kvm_guest_time_update. But kvm_guest_time_update will spin on pvclock_gtod_sync_lock until pvclock_update_vm_gtod_copy exits and kvm_gen_update_masterclock releases the spinlock. >> On top of this, optionally the spinlock could become an rw_semaphore >> so that clock updates for different VCPUs will not be serialized. The >> effect is probably not visible, though. > > Still not clear of the benefits, but this area certainly welcomes > performance improvements (the global kick is one thing we discussed > and that should be improved). This unfortunately does not eliminate the global kick, so there is likely no performance improvement yet. It simplifies the logic a bit though. The change I suggested above is to make pvclock_gtod_sync_lock an rwsem or, probably better, a seqlock. VCPUs reading ka->use_master_clock, ka->master_cycle_now, ka->master_kernel_ns can then run concurrently, with no locked operations in kvm_guest_time_update. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 28, 2013 at 02:37:20PM +0200, Paolo Bonzini wrote: > Il 28/08/2013 04:52, Marcelo Tosatti ha scritto: > > On Thu, Aug 22, 2013 at 07:05:20PM +0200, Paolo Bonzini wrote: > >> Il 20/08/2013 20:20, Marcelo Tosatti ha scritto: > >>> > >>> The offset to add to the hosts monotonic time, kvmclock_offset, is > >>> calculated against the monotonic time at KVM_SET_CLOCK ioctl time. > >>> > >>> Request a master clock update at this time, to reduce a potentially > >>> unbounded difference between the values of the masterclock and > >>> the clock value used to calculate kvmclock_offset. > >>> > >>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > >>> > >>> Index: linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c > >>> =================================================================== > >>> --- linux-2.6-kvmclock-fixes.orig/arch/x86/kvm/x86.c > >>> +++ linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c > >>> @@ -3806,6 +3806,7 @@ long kvm_arch_vm_ioctl(struct file *filp > >>> delta = user_ns.clock - now_ns; > >>> local_irq_enable(); > >>> kvm->arch.kvmclock_offset = delta; > >>> + kvm_gen_update_masterclock(kvm); > >>> break; > >>> } > >>> case KVM_GET_CLOCK: { > >> > >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > >> > >> While reviewing this patch, which BTW looks good, I noticed the handling > >> of KVM_REQ_MCLOCK_INPROGRESS, the dummy request that is never processed > >> and is only used to block guest entry. > >> > >> It seems to me that this bit is not necessary. After > >> KVM_REQ_CLOCK_UPDATE is issued, no guest entries will happen because > >> kvm_guest_time_update will try to take the pvclock_gtod_sync_lock, > >> currently taken by kvm_gen_update_masterclock. > > > > Not entirely clear, to cancel guest entry the bit is necessary: > > > > if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests > > || need_resched() || signal_pending(current)) { > > vcpu->mode = OUTSIDE_GUEST_MODE; > > smp_wmb(); > > local_irq_enable(); > > preempt_enable(); > > r = 1; > > goto cancel_injection; > > } > > Yes, KVM_REQ_CLOCK_UPDATE is enough to cancel guest entries too. See > code below. > > >> Thus, you do not need the dummy request. You can simply issue > >> KVM_REQ_CLOCK_UPDATE before calling pvclock_update_vm_gtod_copy (with > >> the side effect of exiting VCPUs). VCPUs will stall in > >> kvm_guest_time_update until pvclock_gtod_sync_lock is released by > >> kvm_gen_update_masterclock. What do you think? > > > > Not sure its safe. Can you describe the safety of your proposal in more > > detail ? > > Here is the code I was thinking of: > > spin_lock(&ka->pvclock_gtod_sync_lock); > make_all_cpus_request(kvm, KVM_REQ_CLOCK_UPDATE); > > /* > * No guest entries from this point: VCPUs will be spinning > * on pvclock_gtod_sync_lock in kvm_guest_time_update. > */ > pvclock_update_vm_gtod_copy(kvm); > > /* > * Let kvm_guest_time_update continue: entering the guest > * is now allowed too. > */ > spin_unlock(&ka->pvclock_gtod_sync_lock); > > KVM_REQ_CLOCK_UPDATE is used to cancel guest entry and execute > kvm_guest_time_update. But kvm_guest_time_update will spin on > pvclock_gtod_sync_lock until pvclock_update_vm_gtod_copy exits and > kvm_gen_update_masterclock releases the spinlock. Not safe because there are places which set KVM_REQ_CLOCK_UPDATE without kicking target vcpu out of guest mode. Unless you use a modified make_all_cpus_request. The point of REQ_MCLOCK_INPROGRESS request is to guarantee that the following is not possible: - 2 vcpus in guest mode with per-vcpu kvmclock areas with different {system_timestamp, tsc_offset} values. To achieve that: - Kick all vcpus out of guest mode (via a request bit that can't be cleared). - Update the {system_timestamp, tsc_offset} values. - Clear the request bit. > >> On top of this, optionally the spinlock could become an rw_semaphore > >> so that clock updates for different VCPUs will not be serialized. The > >> effect is probably not visible, though. > > > > Still not clear of the benefits, but this area certainly welcomes > > performance improvements (the global kick is one thing we discussed > > and that should be improved). > > This unfortunately does not eliminate the global kick, so there is > likely no performance improvement yet. It simplifies the logic a bit > though. > > The change I suggested above is to make pvclock_gtod_sync_lock an rwsem > or, probably better, a seqlock. > VCPUs reading ka->use_master_clock, > ka->master_cycle_now, ka->master_kernel_ns can then run concurrently, > with no locked operations in kvm_guest_time_update. Good point. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 03/09/2013 05:03, Marcelo Tosatti ha scritto: > > Here is the code I was thinking of: > > > > spin_lock(&ka->pvclock_gtod_sync_lock); > > make_all_cpus_request(kvm, KVM_REQ_CLOCK_UPDATE); > > > > /* > > * No guest entries from this point: VCPUs will be spinning > > * on pvclock_gtod_sync_lock in kvm_guest_time_update. > > */ > > pvclock_update_vm_gtod_copy(kvm); > > > > /* > > * Let kvm_guest_time_update continue: entering the guest > > * is now allowed too. > > */ > > spin_unlock(&ka->pvclock_gtod_sync_lock); > > > > KVM_REQ_CLOCK_UPDATE is used to cancel guest entry and execute > > kvm_guest_time_update. But kvm_guest_time_update will spin on > > pvclock_gtod_sync_lock until pvclock_update_vm_gtod_copy exits and > > kvm_gen_update_masterclock releases the spinlock. > > Not safe because there are places which set KVM_REQ_CLOCK_UPDATE without > kicking target vcpu out of guest mode. Unless you use a modified > make_all_cpus_request. make_all_cpus_request does force an exit, even if the bit is already set in vcpus->requests. But maybe here is where I'm missing something. > The point of REQ_MCLOCK_INPROGRESS request is to guarantee that the > following is not possible: > > - 2 vcpus in guest mode with per-vcpu kvmclock areas with > different {system_timestamp, tsc_offset} values. Understood. > To achieve that: > > - Kick all vcpus out of guest mode (via a request bit that can't be > cleared). > - Update the {system_timestamp, tsc_offset} values. > - Clear the request bit. After make_all_cpus_request, all VCPUs will be out of guest mode, and the request bit will not be cleared until pvclock_gtod_sync_lock is released. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 03, 2013 at 12:42:40PM +0200, Paolo Bonzini wrote: > Il 03/09/2013 05:03, Marcelo Tosatti ha scritto: > > > Here is the code I was thinking of: > > > > > > spin_lock(&ka->pvclock_gtod_sync_lock); > > > make_all_cpus_request(kvm, KVM_REQ_CLOCK_UPDATE); > > > > > > /* > > > * No guest entries from this point: VCPUs will be spinning > > > * on pvclock_gtod_sync_lock in kvm_guest_time_update. > > > */ > > > pvclock_update_vm_gtod_copy(kvm); > > > > > > /* > > > * Let kvm_guest_time_update continue: entering the guest > > > * is now allowed too. > > > */ > > > spin_unlock(&ka->pvclock_gtod_sync_lock); > > > > > > KVM_REQ_CLOCK_UPDATE is used to cancel guest entry and execute > > > kvm_guest_time_update. But kvm_guest_time_update will spin on > > > pvclock_gtod_sync_lock until pvclock_update_vm_gtod_copy exits and > > > kvm_gen_update_masterclock releases the spinlock. > > > > Not safe because there are places which set KVM_REQ_CLOCK_UPDATE without > > kicking target vcpu out of guest mode. Unless you use a modified > > make_all_cpus_request. > > make_all_cpus_request does force an exit, even if the bit is already set > in vcpus->requests. But maybe here is where I'm missing something. No, you're right. My bad. > > The point of REQ_MCLOCK_INPROGRESS request is to guarantee that the > > following is not possible: > > > > - 2 vcpus in guest mode with per-vcpu kvmclock areas with > > different {system_timestamp, tsc_offset} values. > > Understood. > > > To achieve that: > > > > - Kick all vcpus out of guest mode (via a request bit that can't be > > cleared). > > - Update the {system_timestamp, tsc_offset} values. > > - Clear the request bit. > > After make_all_cpus_request, all VCPUs will be out of guest mode, and > the request bit will not be cleared until pvclock_gtod_sync_lock is > released. > > Paolo It seems more obfuscated to rely on an implicit pvclock_gtod_sync_lock blocking than an explicit request bit that can't be cleared, but perhaps thats personal opinion. OTOH, you can also argue that the request bit abuses the vcpu->requests request mechanism, because there is not a request in fact (it only blocks guest entry). If you have reasons to update, and you are sure its safe, feel free to modify it. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 03/09/2013 15:56, Marcelo Tosatti ha scritto: >>> The point of REQ_MCLOCK_INPROGRESS request is to guarantee that the >>> following is not possible: >>> >>> - 2 vcpus in guest mode with per-vcpu kvmclock areas with >>> different {system_timestamp, tsc_offset} values. >> >> Understood. >> >>> To achieve that: >>> >>> - Kick all vcpus out of guest mode (via a request bit that can't be >>> cleared). >>> - Update the {system_timestamp, tsc_offset} values. >>> - Clear the request bit. >> >> After make_all_cpus_request, all VCPUs will be out of guest mode, and >> the request bit will not be cleared until pvclock_gtod_sync_lock is >> released. > > It seems more obfuscated to rely on an implicit pvclock_gtod_sync_lock > blocking than an explicit request bit that can't be cleared, but perhaps > thats personal opinion. Yes, this gets dangerously close to personal opinion territory. :) > OTOH, you can also argue that the request bit abuses the vcpu->requests > request mechanism, because there is not a request in fact (it only > blocks guest entry). The busy-wait is a bit ugly, but otherwise it's fine. > If you have reasons to update, and you are sure its safe, feel > free to modify it. No reason yet apart from removing a few lines of code, but thanks for discussing this. If you want to do the seqlock change for pvclock_gtod_sync_lock, I'll be glad to review the patch. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c =================================================================== --- linux-2.6-kvmclock-fixes.orig/arch/x86/kvm/x86.c +++ linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c @@ -3806,6 +3806,7 @@ long kvm_arch_vm_ioctl(struct file *filp delta = user_ns.clock - now_ns; local_irq_enable(); kvm->arch.kvmclock_offset = delta; + kvm_gen_update_masterclock(kvm); break; } case KVM_GET_CLOCK: {
The offset to add to the hosts monotonic time, kvmclock_offset, is calculated against the monotonic time at KVM_SET_CLOCK ioctl time. Request a master clock update at this time, to reduce a potentially unbounded difference between the values of the masterclock and the clock value used to calculate kvmclock_offset. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html