Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device

Marcelo Tosatti
2009-05-20T14:19:19Z

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.