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

[Marcelo Tosatti <mtosatti@redhat.com>]

On Thu, May 21, 2009 at 10:26:57AM +0300, Avi Kivity wrote:
> 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.

OK, will use the lock to serialize individual ioctl commands that are
not performance sensitive and need serialization.

Any objection to v2 of the irq_lock patch?