> On May 5, 2022, at 4:09 AM, Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote: > > Jon Kohler <jon@xxxxxxxxxxx> writes: > >>> On May 4, 2022, at 5:47 PM, Sean Christopherson <seanjc@xxxxxxxxxx> wrote: >>> > > ... > >> >> The net problem here is really that the stat is likely incorrect; however, >> one other oddity I didn’t quite understand after looking into this is that >> the call site for all of this is in record_steal_time(), which is only called >> from vcpu_enter_guest(), and that is called *after* >> kvm_service_local_tlb_flush_requests(), which also calls >> kvm_vcpu_flush_tlb_guest() if request == KVM_REQ_TLB_FLUSH_GUEST >> >> That request may be there set from a few different places. >> >> I don’t have any proof of this, but it seems to me like we might have a >> situation where we double flush? >> >> Put another way, I wonder if there is any sense behind maybe hoisting >> if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu)) up before >> Other tlb flushes, and have it clear the FLUSH_GUEST if it was set? > > Indeed, if we move KVM_REQ_STEAL_UPDATE check/record_steal_time() call > in vcpu_enter_guest() before kvm_service_local_tlb_flush_requests(), we > can probably get aways with kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, > vcpu) in record_steal_time() which would help to avoid double flushing. Thanks, Vitaly, I’ll rework this one and incorporate that. In the mean time, do you have any suggestions on Sean's concern about losing the trace in situations where pv tlb flushing isn’t happening? > > -- > Vitaly >
