From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steve Rutherford Subject: Re: [RFC PATCH 3/4] KVM: x86: Add EOI exit bitmap inference Date: Thu, 14 May 2015 19:38:27 -0700 Message-ID: <20150515023827.GC21835@google.com> References: <1431481652-27268-1-git-send-email-srutherford@google.com> <1431481652-27268-3-git-send-email-srutherford@google.com> <5552EB35.2070806@siemens.com> <555305A5.8060601@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Jan Kiszka , kvm@vger.kernel.org, ahonig@google.com To: Paolo Bonzini Return-path: Received: from mail-ig0-f169.google.com ([209.85.213.169]:36158 "EHLO mail-ig0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1422918AbbEOCib (ORCPT ); Thu, 14 May 2015 22:38:31 -0400 Received: by igbpi8 with SMTP id pi8so189729443igb.1 for ; Thu, 14 May 2015 19:38:30 -0700 (PDT) Content-Disposition: inline In-Reply-To: <555305A5.8060601@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, May 13, 2015 at 10:04:53AM +0200, Paolo Bonzini wrote: > > > On 13/05/2015 08:12, Jan Kiszka wrote: > >> +void kvm_scan_ioapic_routes(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap) > >> +{ > >> + struct kvm *kvm = vcpu->kvm; > >> + struct kvm_kernel_irq_routing_entry *entry; > >> + struct kvm_irq_routing_table *table; > >> + u32 i, nr_rt_entries; > >> + > >> + mutex_lock(&kvm->irq_lock); > > This only needs irq_srcu protection, not irq_lock, so the lookup cost > becomes much smaller (all CPUs can proceed in parallel). I've updated the next iteration to include this. > > You would need to put an smp_mb here, to ensure that irq_routing is read > after KVM_SCAN_IOAPIC is cleared. You can introduce > smb_mb__after_srcu_read_lock in order to elide it. > > The matching memory barrier would be a smp_mb__before_atomic in > kvm_make_scan_ioapic_request. Wait, what could happen if KVM_SCAN_IOAPIC is cleared after irq_routing is read? I'm imagining the following, but I'm not sure it makes sense. After reading irq_routing, another routing update could roll through (which would be followed the setting of KVM_SCAN_IOAPIC), both of which could be followed by the reordered clearing of KVM_SCAN_IOAPIC. This would lead to only one scan, which would use the stale value for kvm->irq_routing. The reading of kvm->irq_routing (and the reads in srcu_read_lock) should be able to be reordered back to before the clearing of the bit in vcpu->requests (but after the read of vcpu->requests), because reads can be reordered with unrelated writes, but not with other reads. If a routing update came through on another cpu during this window, the issue above could happen. Adding a memory barrier (but not an wmb or an rmb) before reading irq_routing (ideally not in the srcu critical section) should prevent this from happening? Sorry if this is long winded. Memory ordering always feels subtle. Steve