Re: [patch 0/4] move irq protection role to separate lock v2

Avi Kivity
2009-05-21T07:27:00Z

Christian Bornträger wrote:
>> @@ -2053,6 +2054,9 @@ static long kvm_vm_ioctl(struct file *fi
>>
>> if (kvm->mm != current->mm)
>> return -EIO;
>> +
>> + mutex_lock(&kvm->vm_ioctl_lock);
>> +
>> switch (ioctl) {
>> case KVM_CREATE_VCPU:
>> r = kvm_vm_ioctl_create_vcpu(kvm, arg);
>> @@ -2228,6 +2232,7 @@ static long kvm_vm_ioctl(struct file *fi
>> r = kvm_arch_vm_ioctl(filp, ioctl, arg);
>> }
>> out:
>> + mutex_unlock(&kvm->vm_ioctl_lock);
>> return r;
>> }
>>
>
> The thing that looks worrysome is that the s390 version of kvm_arch_vm_ioctl
> has KVM_S390_INTERRUPT. This allows userspace to inject interrupts - which
> would be serialized. The thing is, that external interrupts and I/O interrupts
> are floating - which means they can arrive on all cpus. This is somewhat of a
> fast path.
> On the other hand, kvm_s390_inject_vm already takes the kvm->lock to protect
> agains hotplug. With this patch we might be able to remove the kvm->lock in
> kvm_s390_inject_vm - that would reduce the impact.
>
> This needs more thinking on our side.
>

x86 actually shares the same problem. KVM_IRQ_LINE interrupts may
arrive at any vcpu. Furthermore, with irqfd interrupts may be injected
from userspace (the vm process or other processes) or from the kernel
(assigned device, kernel virtio-net device). So we have the same
motivation to drop this lock and replace it by rcu for the fast paths.

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