From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
Date: Wed, 20 May 2009 11:09:56 -0300
Message-ID: <20090520140956.GA3370@amt.cnet>
References: <20090518165601.747763120@localhost.localdomain> <20090518170855.346048603@localhost.localdomain> <4A13F242.7090404@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:51097 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753884AbZETOTT (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 20 May 2009 10:19:19 -0400
Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26])
	by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n4KEJLYI031191
	for <kvm@vger.kernel.org>; Wed, 20 May 2009 10:19:21 -0400
Content-Disposition: inline
In-Reply-To: <4A13F242.7090404@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, May 20, 2009 at 03:06:26PM +0300, Avi Kivity wrote:
> Marcelo Tosatti wrote:
>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>
>> Index: kvm/virt/kvm/coalesced_mmio.c
>> ===================================================================
>> --- kvm.orig/virt/kvm/coalesced_mmio.c
>> +++ kvm/virt/kvm/coalesced_mmio.c
>> @@ -26,9 +26,10 @@ static int coalesced_mmio_in_range(struc
>>  	if (!is_write)
>>  		return 0;
>>  -	/* kvm->lock is taken by the caller and must be not released before
>> -         * dev.read/write
>> -         */
>> +	/*
>> + 	 * dev->lock must not be released before coalesced_mmio_write.
>> +	 */
>> +	mutex_lock(&dev->lock);
>>   	/* Are we able to batch it ? */
>>  @@ -41,6 +42,7 @@ static int coalesced_mmio_in_range(struc
>>  							KVM_COALESCED_MMIO_MAX;
>>  	if (next == dev->kvm->coalesced_mmio_ring->first) {
>>  		/* full */
>> +		mutex_unlock(&dev->lock);
>>  		return 0;
>>  	}
>>  @@ -57,6 +59,7 @@ static int coalesced_mmio_in_range(struc
>>  		    addr + len <= zone->addr + zone->size)
>>  			return 1;
>>  	}
>> +	mutex_unlock(&dev->lock);
>>  	return 0;
>>  }
>>   
>
> So we have a function that takes a lock and conditionally releases it?

Yes, but it is correct: it will only return with the lock held in case
it returns 1, in which case its guaranteed ->write will be called (which
will unlock it).

It should check the range first and/or use some smarter synchronization,
but one thing at a time.