[patch 2/4] KVM: move coalesced_mmio locking to its own device
Move coalesced_mmio locking to its own device, instead of relying on
kvm-lock.
Signed-off-by: Marcelo Tosatti mtosa...@redhat.com
Index: kvm/virt/kvm/coalesced_mmio.c
===
--- kvm.orig/virt/kvm/coalesced_mmio.c
+++ kvm/virt/kvm/coalesced_mmio.c
@@ -31,10 +31,6 @@ 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
- */
-
/* Are we able to batch it ? */
/* last is the first free entry
@@ -70,7 +66,7 @@ static void coalesced_mmio_write(struct
struct kvm_coalesced_mmio_dev *dev = to_mmio(this);
struct kvm_coalesced_mmio_ring *ring = dev-kvm-coalesced_mmio_ring;
- /* kvm-lock must be taken by caller before call to in_range()*/
+ spin_lock(dev-lock);
/* copy data in first free entry of the ring */
@@ -79,6 +75,7 @@ static void coalesced_mmio_write(struct
memcpy(ring-coalesced_mmio[ring-last].data, val, len);
smp_wmb();
ring-last = (ring-last + 1) % KVM_COALESCED_MMIO_MAX;
+ spin_unlock(dev-lock);
}
static void coalesced_mmio_destructor(struct kvm_io_device *this)
@@ -101,6 +98,7 @@ int kvm_coalesced_mmio_init(struct kvm *
dev = kzalloc(sizeof(struct kvm_coalesced_mmio_dev), GFP_KERNEL);
if (!dev)
return -ENOMEM;
+ spin_lock_init(dev-lock);
kvm_iodevice_init(dev-dev, coalesced_mmio_ops);
dev-kvm = kvm;
kvm-coalesced_mmio_dev = dev;
Index: kvm/virt/kvm/coalesced_mmio.h
===
--- kvm.orig/virt/kvm/coalesced_mmio.h
+++ kvm/virt/kvm/coalesced_mmio.h
@@ -12,6 +12,7 @@
struct kvm_coalesced_mmio_dev {
struct kvm_io_device dev;
struct kvm *kvm;
+ spinlock_t lock;
int nb_zones;
struct kvm_coalesced_mmio_zone zone[KVM_COALESCED_MMIO_ZONE_MAX];
};
--
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
Marcelo Tosatti wrote: On Sun, May 31, 2009 at 03:14:36PM +0300, Avi Kivity wrote: Marcelo Tosatti wrote: Move coalesced_mmio locking to its own device, instead of relying on kvm-lock. Signed-off-by: Marcelo Tosatti mtosa...@redhat.com Index: kvm-irqlock/virt/kvm/coalesced_mmio.c === --- kvm-irqlock.orig/virt/kvm/coalesced_mmio.c +++ kvm-irqlock/virt/kvm/coalesced_mmio.c @@ -26,9 +26,7 @@ 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 - */ + spin_lock(dev-lock); This unbalanced locking is still very displeasing. At a minimum you need a sparse annotation to indicate it. But I think it really indicates a problem with the io_device API. Potential solutions: - fold in_range() into -write and -read. Make those functions responsible for both determining whether they can handle the range and performing the I/O. - have a separate rwlock for the device list. IMO the problem is the coalesced_mmio device. The unbalanced locking is a result of the abuse of the in_range() and read/write() methods. Okay, the penny has dropped. I understand now. Normally you'd expect parallel accesses to in_range() to be allowed, since its just checking whether (aha) the access is in range, returning a pointer to the device if positive. Now read/write() are the ones who need serialization, since they touch the device internal state. coalesced_mmio abuses in_range() to do more things than it should. Ideally we should fix coalesced_mmio, but i'm not going to do that now (sorry, not confident in changing it without seeing go through intense torture testing). It's not trivial since it's userspace that clears the ring, and we can't wait on userspace. That said, is sparse annotation enough the convince you? Let me have a look at fixing coalesced_mmio first. We might allow -write to fail, causing a fallback to userspace. Or we could fail if n_avail MAX_VCPUS, so even the worst-case race leaves us one entry. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/4] KVM: move coalesced_mmio locking to its own device
Move coalesced_mmio locking to its own device, instead of relying on
kvm-lock.
Signed-off-by: Marcelo Tosatti mtosa...@redhat.com
Index: kvm-irqlock/virt/kvm/coalesced_mmio.c
===
--- kvm-irqlock.orig/virt/kvm/coalesced_mmio.c
+++ kvm-irqlock/virt/kvm/coalesced_mmio.c
@@ -26,9 +26,7 @@ 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
- */
+ spin_lock(dev-lock);
/* Are we able to batch it ? */
@@ -41,7 +39,7 @@ static int coalesced_mmio_in_range(struc
KVM_COALESCED_MMIO_MAX;
if (next == dev-kvm-coalesced_mmio_ring-first) {
/* full */
- return 0;
+ goto out_denied;
}
/* is it in a batchable area ? */
@@ -57,6 +55,8 @@ static int coalesced_mmio_in_range(struc
addr + len = zone-addr + zone-size)
return 1;
}
+out_denied:
+ spin_unlock(dev-lock);
return 0;
}
@@ -67,8 +67,6 @@ static void coalesced_mmio_write(struct
(struct kvm_coalesced_mmio_dev*)this-private;
struct kvm_coalesced_mmio_ring *ring = dev-kvm-coalesced_mmio_ring;
- /* kvm-lock must be taken by caller before call to in_range()*/
-
/* copy data in first free entry of the ring */
ring-coalesced_mmio[ring-last].phys_addr = addr;
@@ -76,6 +74,7 @@ static void coalesced_mmio_write(struct
memcpy(ring-coalesced_mmio[ring-last].data, val, len);
smp_wmb();
ring-last = (ring-last + 1) % KVM_COALESCED_MMIO_MAX;
+ spin_unlock(dev-lock);
}
static void coalesced_mmio_destructor(struct kvm_io_device *this)
@@ -90,6 +89,8 @@ int kvm_coalesced_mmio_init(struct kvm *
dev = kzalloc(sizeof(struct kvm_coalesced_mmio_dev), GFP_KERNEL);
if (!dev)
return -ENOMEM;
+ spin_lock_init(dev-lock);
+
dev-dev.write = coalesced_mmio_write;
dev-dev.in_range = coalesced_mmio_in_range;
dev-dev.destructor = coalesced_mmio_destructor;
Index: kvm-irqlock/virt/kvm/coalesced_mmio.h
===
--- kvm-irqlock.orig/virt/kvm/coalesced_mmio.h
+++ kvm-irqlock/virt/kvm/coalesced_mmio.h
@@ -12,6 +12,7 @@
struct kvm_coalesced_mmio_dev {
struct kvm_io_device dev;
struct kvm *kvm;
+ spinlock_t lock;
int nb_zones;
struct kvm_coalesced_mmio_zone zone[KVM_COALESCED_MMIO_ZONE_MAX];
};
--
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
Marcelo Tosatti wrote: === --- kvm-irqlock.orig/virt/kvm/coalesced_mmio.c +++ kvm-irqlock/virt/kvm/coalesced_mmio.c @@ -26,9 +26,12 @@ 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 - */ + /* +* Some other vcpu might be batching data into the ring, +* fallback to userspace. Ordering not our problem. +*/ + if (!atomic_add_unless(dev-in_use, 1, 1)) + return 0; Ordering with simultaneous writes is indeed not our problem, but the ring may contain ordered writes (even by the same vcpu!) You mean writes by a vcpu should maintain ordering even when that vcpu migrates to a different pcpu? No. Writes by a single vcpu need to preserve their ordering relative to each other. If a write by vcpu A completes before a write by vcpu B starts, then they need to be ordered, since the vcpus may have synchronized in between. Hm, I don't thimk you broke these rules. The smp_wmb in coalesced_write guarantees that. Suggest our own lock here. in_use is basically a homemade lock, better to use the factory made ones which come with a warranty. The first submission had a mutex but was considered unorthodox. Although i agree with your argument made then, i don't see a way around that. Some pseudocode please? Why not use slots_lock to protect the entire iodevice list (rcu one day), and an internal spinlock for coalesced mmio? -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
On Tue, May 26, 2009 at 02:24:33PM +0300, Avi Kivity wrote: Marcelo Tosatti wrote: === --- kvm-irqlock.orig/virt/kvm/coalesced_mmio.c +++ kvm-irqlock/virt/kvm/coalesced_mmio.c @@ -26,9 +26,12 @@ 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 - */ + /* + * Some other vcpu might be batching data into the ring, + * fallback to userspace. Ordering not our problem. + */ + if (!atomic_add_unless(dev-in_use, 1, 1)) + return 0; Ordering with simultaneous writes is indeed not our problem, but the ring may contain ordered writes (even by the same vcpu!) You mean writes by a vcpu should maintain ordering even when that vcpu migrates to a different pcpu? No. Writes by a single vcpu need to preserve their ordering relative to each other. If a write by vcpu A completes before a write by vcpu B starts, then they need to be ordered, since the vcpus may have synchronized in between. Hm, I don't thimk you broke these rules. The smp_wmb in coalesced_write guarantees that. Suggest our own lock here. in_use is basically a homemade lock, better to use the factory made ones which come with a warranty. The first submission had a mutex but was considered unorthodox. Although i agree with your argument made then, i don't see a way around that. Some pseudocode please? Why not use slots_lock to protect the entire iodevice list (rcu one day), and an internal spinlock for coalesced mmio? Don't like using slots_lock to protect the entire iodevice list, its reverse progress in my opinion. The PIO/MMIO device lists are data structures used in hot path, they are not directly related to the memslots, and therefore deserve a separate lock. Whenever slots_lock becomes an issue, you'll have to entangle the mess, so better start doing it now. Sure can switch to a internal spinlock for coalesced_mmio, but breaking out if spin_trylock fails. You're OK with that? -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
Marcelo Tosatti wrote: Why not use slots_lock to protect the entire iodevice list (rcu one day), and an internal spinlock for coalesced mmio? Don't like using slots_lock to protect the entire iodevice list, its reverse progress in my opinion. The PIO/MMIO device lists are data structures used in hot path, they are not directly related to the memslots, and therefore deserve a separate lock. Whenever slots_lock becomes an issue, you'll have to entangle the mess, so better start doing it now. It's a reader/writer lock, so you'll never have contention. I'm okay with a new lock though. Sure can switch to a internal spinlock for coalesced_mmio, but breaking out if spin_trylock fails. You're OK with that? Why not do a normal spin_lock()? It's not like you'll wait years for the store to complete. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
On Sun, May 24, 2009 at 05:04:52PM +0300, Avi Kivity wrote: Marcelo Tosatti wrote: Get rid of kvm-lock dependency on coalesced_mmio methods. Use an atomic variable instead to guarantee only one vcpu is batching data into the ring at a given time. Signed-off-by: Marcelo Tosatti mtosa...@redhat.com Index: kvm-irqlock/virt/kvm/coalesced_mmio.c === --- kvm-irqlock.orig/virt/kvm/coalesced_mmio.c +++ kvm-irqlock/virt/kvm/coalesced_mmio.c @@ -26,9 +26,12 @@ 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 - */ +/* + * Some other vcpu might be batching data into the ring, + * fallback to userspace. Ordering not our problem. + */ +if (!atomic_add_unless(dev-in_use, 1, 1)) +return 0; Ordering with simultaneous writes is indeed not our problem, but the ring may contain ordered writes (even by the same vcpu!) You mean writes by a vcpu should maintain ordering even when that vcpu migrates to a different pcpu? The smp_wmb in coalesced_write guarantees that. Suggest our own lock here. in_use is basically a homemade lock, better to use the factory made ones which come with a warranty. The first submission had a mutex but was considered unorthodox. Although i agree with your argument made then, i don't see a way around that. Some pseudocode please? Agree with the suggestion on patch 3, will fix the commentary on kvm_host.h. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
Marcelo Tosatti wrote: Get rid of kvm-lock dependency on coalesced_mmio methods. Use an atomic variable instead to guarantee only one vcpu is batching data into the ring at a given time. Signed-off-by: Marcelo Tosatti mtosa...@redhat.com Index: kvm-irqlock/virt/kvm/coalesced_mmio.c === --- kvm-irqlock.orig/virt/kvm/coalesced_mmio.c +++ kvm-irqlock/virt/kvm/coalesced_mmio.c @@ -26,9 +26,12 @@ 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 - */ + /* +* Some other vcpu might be batching data into the ring, +* fallback to userspace. Ordering not our problem. +*/ + if (!atomic_add_unless(dev-in_use, 1, 1)) + return 0; Ordering with simultaneous writes is indeed not our problem, but the ring may contain ordered writes (even by the same vcpu!) Suggest our own lock here. in_use is basically a homemade lock, better to use the factory made ones which come with a warranty. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
Marcelo Tosatti wrote:
Signed-off-by: Marcelo Tosatti mtosa...@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?
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
On Wed, May 20, 2009 at 03:06:26PM +0300, Avi Kivity wrote:
Marcelo Tosatti wrote:
Signed-off-by: Marcelo Tosatti mtosa...@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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
Marcelo Tosatti wrote: 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. Yes it's correct but we'll get an endless stream of patches to 'fix' it because it is so unorthodox. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
On Wed, May 20, 2009 at 05:29:23PM +0300, Avi Kivity wrote: Marcelo Tosatti wrote: 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. Yes it's correct but we'll get an endless stream of patches to 'fix' it because it is so unorthodox. Does it have to guarantee any kind of ordering in case of parallel writes by distincting vcpus? This is what it does now (so if a vcpu arrives first, the second will wait until the first is finished processing). I suppose that is the responsability of the guest (if it does MMIO writes to a device in parallel it'll screwup in real HW too). Because in such case, you can drop the mutex and protect only the kvm data. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
Marcelo Tosatti wrote: Yes it's correct but we'll get an endless stream of patches to 'fix' it because it is so unorthodox. Does it have to guarantee any kind of ordering in case of parallel writes by distincting vcpus? This is what it does now (so if a vcpu arrives first, the second will wait until the first is finished processing). No. I suppose that is the responsability of the guest (if it does MMIO writes to a device in parallel it'll screwup in real HW too). Yes. Because in such case, you can drop the mutex and protect only the kvm data. One has to be before the other. The order doesn't matter, but both have to be there. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device
On Wed, May 20, 2009 at 12:13:03PM -0300, Marcelo Tosatti wrote: On Wed, May 20, 2009 at 05:29:23PM +0300, Avi Kivity wrote: Marcelo Tosatti wrote: 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. Yes it's correct but we'll get an endless stream of patches to 'fix' it because it is so unorthodox. Does it have to guarantee any kind of ordering in case of parallel writes by distincting vcpus? This is what it does now (so if a vcpu arrives first, the second will wait until the first is finished processing). I suppose that is the responsability of the guest (if it does MMIO writes to a device in parallel it'll screwup in real HW too). Because in such case, you can drop the mutex and protect only the kvm data. If you want it to provide ordering (as in process the MMIO writes, by multiple vcpus, in the order they happen), you need a mutex or spinlock. And in that case, I don't see any other way around this given the way -in_range / -read/-write work. Even if you change the order in which the full mmio buffer check and coalesced-allowed-in-this-range happen you still need a spinlock/mutex in this unorthodox way. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/4] KVM: move coalesced_mmio locking to its own device
Get rid of kvm-lock dependency on coalesced_mmio methods. Use an
atomic variable instead to guarantee only one vcpu is batching
data into the ring at a given time.
Signed-off-by: Marcelo Tosatti mtosa...@redhat.com
Index: kvm-irqlock/virt/kvm/coalesced_mmio.c
===
--- kvm-irqlock.orig/virt/kvm/coalesced_mmio.c
+++ kvm-irqlock/virt/kvm/coalesced_mmio.c
@@ -26,9 +26,12 @@ 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
- */
+ /*
+* Some other vcpu might be batching data into the ring,
+* fallback to userspace. Ordering not our problem.
+*/
+ if (!atomic_add_unless(dev-in_use, 1, 1))
+ return 0;
/* Are we able to batch it ? */
@@ -41,7 +44,7 @@ static int coalesced_mmio_in_range(struc
KVM_COALESCED_MMIO_MAX;
if (next == dev-kvm-coalesced_mmio_ring-first) {
/* full */
- return 0;
+ goto out_denied;
}
/* is it in a batchable area ? */
@@ -57,6 +60,8 @@ static int coalesced_mmio_in_range(struc
addr + len = zone-addr + zone-size)
return 1;
}
+out_denied:
+ atomic_set(dev-in_use, 0);
return 0;
}
@@ -67,15 +72,14 @@ static void coalesced_mmio_write(struct
(struct kvm_coalesced_mmio_dev*)this-private;
struct kvm_coalesced_mmio_ring *ring = dev-kvm-coalesced_mmio_ring;
- /* kvm-lock must be taken by caller before call to in_range()*/
-
/* copy data in first free entry of the ring */
ring-coalesced_mmio[ring-last].phys_addr = addr;
ring-coalesced_mmio[ring-last].len = len;
memcpy(ring-coalesced_mmio[ring-last].data, val, len);
- smp_wmb();
ring-last = (ring-last + 1) % KVM_COALESCED_MMIO_MAX;
+ smp_wmb();
+ atomic_set(dev-in_use, 0);
}
static void coalesced_mmio_destructor(struct kvm_io_device *this)
@@ -90,6 +94,8 @@ int kvm_coalesced_mmio_init(struct kvm *
dev = kzalloc(sizeof(struct kvm_coalesced_mmio_dev), GFP_KERNEL);
if (!dev)
return -ENOMEM;
+ atomic_set(dev-in_use, 0);
+
dev-dev.write = coalesced_mmio_write;
dev-dev.in_range = coalesced_mmio_in_range;
dev-dev.destructor = coalesced_mmio_destructor;
Index: kvm-irqlock/virt/kvm/coalesced_mmio.h
===
--- kvm-irqlock.orig/virt/kvm/coalesced_mmio.h
+++ kvm-irqlock/virt/kvm/coalesced_mmio.h
@@ -12,6 +12,7 @@
struct kvm_coalesced_mmio_dev {
struct kvm_io_device dev;
struct kvm *kvm;
+ atomic_t in_use;
int nb_zones;
struct kvm_coalesced_mmio_zone zone[KVM_COALESCED_MMIO_ZONE_MAX];
};
--
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/4] KVM: move coalesced_mmio locking to its own device
Signed-off-by: Marcelo Tosatti mtosa...@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;
}
@@ -67,8 +70,6 @@ static void coalesced_mmio_write(struct
(struct kvm_coalesced_mmio_dev*)this-private;
struct kvm_coalesced_mmio_ring *ring = dev-kvm-coalesced_mmio_ring;
- /* kvm-lock must be taken by caller before call to in_range()*/
-
/* copy data in first free entry of the ring */
ring-coalesced_mmio[ring-last].phys_addr = addr;
@@ -76,6 +77,7 @@ static void coalesced_mmio_write(struct
memcpy(ring-coalesced_mmio[ring-last].data, val, len);
smp_wmb();
ring-last = (ring-last + 1) % KVM_COALESCED_MMIO_MAX;
+ mutex_unlock(dev-lock);
}
static void coalesced_mmio_destructor(struct kvm_io_device *this)
@@ -90,6 +92,8 @@ int kvm_coalesced_mmio_init(struct kvm *
dev = kzalloc(sizeof(struct kvm_coalesced_mmio_dev), GFP_KERNEL);
if (!dev)
return -ENOMEM;
+ mutex_init(dev-lock);
+
dev-dev.write = coalesced_mmio_write;
dev-dev.in_range = coalesced_mmio_in_range;
dev-dev.destructor = coalesced_mmio_destructor;
@@ -109,16 +113,16 @@ int kvm_vm_ioctl_register_coalesced_mmio
if (dev == NULL)
return -EINVAL;
- mutex_lock(kvm-lock);
+ mutex_lock(dev-lock);
if (dev-nb_zones = KVM_COALESCED_MMIO_ZONE_MAX) {
- mutex_unlock(kvm-lock);
+ mutex_unlock(dev-lock);
return -ENOBUFS;
}
dev-zone[dev-nb_zones] = *zone;
dev-nb_zones++;
- mutex_unlock(kvm-lock);
+ mutex_unlock(dev-lock);
return 0;
}
@@ -132,7 +136,7 @@ int kvm_vm_ioctl_unregister_coalesced_mm
if (dev == NULL)
return -EINVAL;
- mutex_lock(kvm-lock);
+ mutex_lock(dev-lock);
i = dev-nb_zones;
while(i) {
@@ -150,7 +154,7 @@ int kvm_vm_ioctl_unregister_coalesced_mm
i--;
}
- mutex_unlock(kvm-lock);
+ mutex_unlock(dev-lock);
return 0;
}
Index: kvm/virt/kvm/coalesced_mmio.h
===
--- kvm.orig/virt/kvm/coalesced_mmio.h
+++ kvm/virt/kvm/coalesced_mmio.h
@@ -12,6 +12,7 @@
struct kvm_coalesced_mmio_dev {
struct kvm_io_device dev;
struct kvm *kvm;
+ struct mutex lock;
int nb_zones;
struct kvm_coalesced_mmio_zone zone[KVM_COALESCED_MMIO_ZONE_MAX];
};
--
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
