From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH 3/3] kvm-s390: streamline memslot handling - rebased
Date: Mon, 8 Jun 2009 21:56:32 -0300
Message-ID: <20090609005632.GA21096@amt.cnet>
References: <1243952771-32428-1-git-send-email-ehrhardt@linux.vnet.ibm.com> <1243952771-32428-4-git-send-email-ehrhardt@linux.vnet.ibm.com> <20090605205312.GA13471@amt.cnet> <4A2CED2E.6030904@linux.vnet.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: kvm@vger.kernel.org, avi@redhat.com, borntraeger@de.ibm.com,
	cotte@de.ibm.com, heiko.carstens@de.ibm.com, schwidefsky@de.ibm.com
To: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:35195 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753981AbZFIA5W (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 8 Jun 2009 20:57:22 -0400
Content-Disposition: inline
In-Reply-To: <4A2CED2E.6030904@linux.vnet.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Jun 08, 2009 at 12:51:26PM +0200, Christian Ehrhardt wrote:
>>>  Index: kvm/virt/kvm/kvm_main.c
>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>> --- kvm.orig/virt/kvm/kvm_main.c
>>> +++ kvm/virt/kvm/kvm_main.c
>>> @@ -1682,6 +1682,10 @@ static int kvm_vcpu_release(struct inode
>>>  {
>>>  	struct kvm_vcpu *vcpu =3D filp->private_data;
>>>  +	clear_bit(KVM_REQ_MMU_RELOAD, &vcpu->requests);
>>> +	smp_mb__after_clear_bit();
>>> +	wake_up_bit(&vcpu->requests, KVM_REQ_MMU_RELOAD);
>>> +
>>>    =20
>>
>> And this should be generic? Say if other architectures want to make =
use=20
>> of a similar wait infrastructure. Talk is cheap.
>>  =20
> Clear bit and wake up on release doesn't hurt any architecture, but i=
t =20
> is at a good place fine for those using the mechanism to ensure clean=
ing =20
> up outstanding things when closing a vcpu fd.
> I thought its not worth to add kvm_ARCH_vcpu_release for it while I =20
> could do so if we want it separated.

Yeah, was frustated for lack of more useful comments so decided to
nitpick on something.

> (continued below)
>> Anyway, yeah, the set request / wait mechanism you implement here is
>> quite similar to the idea mentioned earlier that could be used for x=
86.
>>
>> Just get rid of this explicit KVM_REQ_MMU_RELOAD knowledge in
>> arch-independent code please (if you want to see this merged).
>>  =20
> I agree to lift the wait part to other archs later if needed, but as =
=20
> mentioned above I could move this to arch code to the cost of one arc=
h =20
> hook more. But as also mentioned it doesn't really hurt. I agree that=
 it =20
> does not need to be KVM_REQ_MMU_RELOAD specific, we could just =20
> walk/clear/wake all bits on that vcpu->requests variable.
> Would that be generic enough in your opinion ?

Don't know.

Avi?

>> Later it can all be lifted off to arch independent code.
>>  =20
> True for the wait part which can evolve in our arch code until it is=20
> ripe to get cross arch merged.
>
> --=20
>
> Gr=FCsse / regards, Christian Ehrhardt
> IBM Linux Technology Center, Open Virtualization=20