On 05/30/2012 11:03 PM, Marcelo Tosatti wrote: > On Sun, May 20, 2012 at 04:49:27PM +0300, Avi Kivity wrote: >> Instead of using a atomic operation per active request, use just one >> to get all requests at once, then check them with local ops. This >> probably isn't any faster, since simultaneous requests are rare, but >> it does reduce code size. >> >> Signed-off-by: Avi Kivity <avi@xxxxxxxxxx> >> --- >> arch/x86/kvm/x86.c | 33 ++++++++++++++++++--------------- >> 1 file changed, 18 insertions(+), 15 deletions(-) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 953e692..c0209eb 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -5232,55 +5232,58 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) >> bool req_int_win = !irqchip_in_kernel(vcpu->kvm) && >> vcpu->run->request_interrupt_window; >> bool req_immediate_exit = 0; >> + ulong reqs; >> >> if (unlikely(req_int_win)) >> kvm_make_request(KVM_REQ_EVENT, vcpu); >> >> if (vcpu->requests) { >> - if (kvm_check_request(KVM_REQ_MMU_RELOAD, vcpu)) >> + reqs = xchg(&vcpu->requests, 0UL); >> + >> + if (test_bit(KVM_REQ_MMU_RELOAD, &reqs)) >> kvm_mmu_unload(vcpu); >> - if (kvm_check_request(KVM_REQ_MIGRATE_TIMER, vcpu)) >> + if (test_bit(KVM_REQ_MIGRATE_TIMER, &reqs)) >> __kvm_migrate_timers(vcpu); >> - if (kvm_check_request(KVM_REQ_CLOCK_UPDATE, vcpu)) { >> + if (test_bit(KVM_REQ_CLOCK_UPDATE, &reqs)) { >> r = kvm_guest_time_update(vcpu); >> if (unlikely(r)) >> goto out; >> } > > Bailing out loses requests in "reqs". Whoops, good catch. > > Caching the requests makes the following type of sequence behave strangely > > req = xchg(&vcpu->requests); > if request is set > request handler > ... > set REQ_EVENT > ... > > prepare for guest entry > vcpu->requests set > bail I don't really mind that. But I do want to reduce the overhead of a request, they're not that rare in normal workloads. How about for_each_set_bit(req, &vcpu->requests, BITS_PER_LONG) { clear_bit(bit, &vcpu->requests); r = request_handlers[bit](vcpu); if (r) goto out; } ? That makes for O(1) handling since usually we only have one request set (KVM_REQ_EVENT). We'll make that the last one to avoid the scenario above. -- error compiling committee.c: too many arguments to function -- 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