Re: [patch 3/4] KVM: introduce irq_lock, use it to protect ioapic

[Avi Kivity <avi@redhat.com>]

Marcelo Tosatti wrote:
> Subject says it all.
>   

I hate those changelogs.  I guess Subject never reviews code.

You might put some evidence that we're suffering from contention here 
(I'm very willing to believe it, but hard evidence is better).

> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> Index: kvm-irqlock/include/linux/kvm_host.h
> ===================================================================
> --- kvm-irqlock.orig/include/linux/kvm_host.h
> +++ kvm-irqlock/include/linux/kvm_host.h
> @@ -123,7 +123,6 @@ struct kvm_kernel_irq_routing_entry {
>  };
>  
>  struct kvm {
> -	struct mutex lock; /* protects the vcpus array and APIC accesses */
>  	spinlock_t mmu_lock;
>  	struct rw_semaphore slots_lock;
>  	struct mm_struct *mm; /* userspace tied to this vm */
> @@ -132,6 +131,12 @@ struct kvm {
>  					KVM_PRIVATE_MEM_SLOTS];
>  	struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
>  	struct list_head vm_list;
> +	struct mutex lock; /*
> +			    * - protects mmio_bus, pio_bus.
> +			    * - protects a few concurrent ioctl's (FIXME).
> +			    * - protects concurrent create_vcpu, but
>   
List data structures, not operations.

> +			    *   kvm->vcpus walkers do it locklessly (FIXME).
>   

I think we're fine?  maybe add an smp_mb() between cpu creation and 
assignment into the vcpu array?


> +			    */
>  	struct kvm_io_bus mmio_bus;
>  	struct kvm_io_bus pio_bus;
>  	struct list_head irqfds;
> @@ -143,6 +148,7 @@ struct kvm {
>  	struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
>  #endif
>  
> +	struct mutex irq_lock; /* protects high level irq logic, ioapic */
>   

List the data structures protected please.

-- 
error compiling committee.c: too many arguments to function