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?

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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to