On Tue, Nov 04, 2014 at 06:10:34PM +0100, Paolo Bonzini wrote: > Extending the context we have: > > if (vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC) > > if (!ka->use_master_clock) > > do_request = 1; > > > > - if (!vcpus_matched && ka->use_master_clock) > > + if (ka->use_master_clock) > > do_request = 1; > > > > if (do_request) > > kvm_make_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu); > > The patch also makes the previous "if (!ka->use_master_clock)" redundant. > If you enter the first "if", do_request will be 1 independent of > ka->use_master_clock. So you should also drop that one, and possibly > rewrite it simply like this: > > if ((vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC) || > ka->use_master_clock) > kvm_make_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu); > > But this brings the question: what is vclock_mode in your case? If it > is VCLOCK_TSC, are you sure that the bug is fixed because you modified > the second "if", or could it be fixed also by removing instead the > "if (!ka->use_master_clock)"? This would leave the optimization in the > case "!vcpus_matched && ka->use_master_clock". Or is the optimization > always invalid? The bug is fixed by always updating masterclock values, when it is enabled. > A different way to state the same question: can you explain the > resulting condition > > ((vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC) || > ka->use_master_clock) > > ? Please add a comment to kvm_track_tsc_matching that clarifies this > logic. > > Paolo Sure, sending v2. -- 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
