On Wed, May 20, 2009 at 03:06:26PM +0300, Avi Kivity wrote: > Marcelo Tosatti wrote: >> Signed-off-by: Marcelo Tosatti <mtosatti@xxxxxxxxxx> >> >> 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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html