From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [patch 0/4] move irq protection role to separate lock v2
Date: Thu, 21 May 2009 11:32:03 -0300
Message-ID: <20090521143203.GA3358@amt.cnet>
References: <4A1413C3.4020606@redhat.com> <20090520184841.954066003@localhost.localdomain> <20090521045015.GA1104@amt.cnet> <200905210855.53470.borntrae@de.ibm.com> <4A150241.8070408@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: Christian =?iso-8859-1?Q?Borntr=E4ger?= <borntrae@de.ibm.com>,
	Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>,
	Carsten Otte <cotte@de.ibm.com>, kvm@vger.kernel.org,
	Christian Borntraeger <borntraeger@de.ibm.com>
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:40998 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753836AbZEUOci (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 21 May 2009 10:32:38 -0400
Content-Disposition: inline
In-Reply-To: <4A150241.8070408@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Thu, May 21, 2009 at 10:26:57AM +0300, Avi Kivity wrote:
> Christian Borntr=E4ger wrote:
>>> @@ -2053,6 +2054,9 @@ static long kvm_vm_ioctl(struct file *fi
>>>
>>>  	if (kvm->mm !=3D current->mm)
>>>  		return -EIO;
>>> +
>>> +	mutex_lock(&kvm->vm_ioctl_lock);
>>> +
>>>  	switch (ioctl) {
>>>  	case KVM_CREATE_VCPU:
>>>  		r =3D kvm_vm_ioctl_create_vcpu(kvm, arg);
>>> @@ -2228,6 +2232,7 @@ static long kvm_vm_ioctl(struct file *fi
>>>  		r =3D kvm_arch_vm_ioctl(filp, ioctl, arg);
>>>  	}
>>>  out:
>>> +	mutex_unlock(&kvm->vm_ioctl_lock);
>>>  	return r;
>>>  }
>>>    =20
>>
>> The thing that looks worrysome is that the s390 version of=20
>> kvm_arch_vm_ioctl has KVM_S390_INTERRUPT. This allows userspace to=20
>> inject interrupts - which would be serialized. The thing is, that=20
>> external interrupts and I/O interrupts are floating - which means th=
ey=20
>> 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=
=20
>> protect agains hotplug. With this patch we might be able to remove t=
he=20
>> kvm->lock in kvm_s390_inject_vm - that would reduce the impact.=20
>>
>> This needs more thinking on our side.
>>  =20
>
> x86 actually shares the same problem.  KVM_IRQ_LINE interrupts may =20
> arrive at any vcpu.  Furthermore, with irqfd interrupts may be inject=
ed =20
> from userspace (the vm process or other processes) or from the kernel=
 =20
> (assigned device, kernel virtio-net device).  So we have the same =20
> motivation to drop this lock and replace it by rcu for the fast paths=
=2E

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?