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

Avi Kivity
2009-05-24T14:10:08Z

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