[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V3)
On Thu, Nov 19, 2009 at 09:19:05AM -0600, Adam Litke wrote:
Rusty and Anthony,
If I've addressed all outstanding issues, please consider this patch for
inclusion. Thanks.
Changes since V2:
- Increase stat field size to 64 bits
- Report all sizes in kb (not pages)
- Drop anon_pages stat and fix endianness conversion
Changes since V1:
- Use a virtqueue instead of the device config space
When using ballooning to manage overcommitted memory on a host, a system for
guests to communicate their memory usage to the host can provide information
that will minimize the impact of ballooning on the guests. The current method
employs a daemon running in each guest that communicates memory statistics to
a
host daemon at a specified time interval. The host daemon aggregates this
information and inflates and/or deflates balloons according to the level of
host memory pressure. This approach is effective but overly complex since a
daemon must be installed inside each guest and coordinated to communicate with
the host. A simpler approach is to collect memory statistics in the virtio
balloon driver and communicate them directly to the hypervisor.
This patch enables the guest-side support by adding stats collection and
reporting to the virtio balloon driver.
Signed-off-by: Adam Litke a...@us.ibm.com
Cc: Rusty Russell ru...@rustcorp.com.au
Cc: Anthony Liguori anth...@codemonkey.ws
Cc: virtualizat...@lists.linux-foundation.org
Cc: linux-ker...@vger.kernel.org
that's pretty clean. some comments: I wrote them while off-line, and now
that I was going to send it out, I see that there's some overlap with
what Rusty wrote. Still, here it is:
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 200c22f..ebc9d39 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -29,7 +29,7 @@
struct virtio_balloon
{
struct virtio_device *vdev;
- struct virtqueue *inflate_vq, *deflate_vq;
+ struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
/* Where the ballooning thread waits for config to change. */
wait_queue_head_t config_change;
@@ -50,6 +50,9 @@ struct virtio_balloon
/* The array of pfns we tell the Host about. */
unsigned int num_pfns;
u32 pfns[256];
+
+ /* Memory statistics */
+ struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
};
static struct virtio_device_id id_table[] = {
@@ -155,6 +158,57 @@ static void leak_balloon(struct virtio_balloon *vb,
size_t num)
}
}
+static inline void update_stat(struct virtio_balloon *vb, int idx,
+ __le16 tag, __le64 val)
+{
+ BUG_ON(idx = VIRTIO_BALLOON_S_NR);
+ vb-stats[idx].tag = cpu_to_le16(tag);
+ vb-stats[idx].val = cpu_to_le64(val);
you should do le16_to_cpu etc on __le values.
Try running this patch through sparce checker.
+}
+
+#define K(x) ((x) (PAGE_SHIFT - 10))
can't this overflow?
also, won't it be simpler to just report memory is
in bytes, then just x * PAGE_SIZE instead of hairy constants?
+static void update_balloon_stats(struct virtio_balloon *vb)
+{
+ unsigned long events[NR_VM_EVENT_ITEMS];
that's about 1/2K worth of stack space on a 64 bit machine.
better keep it as part of struct virtio_balloon.
+ struct sysinfo i;
+ int idx = 0;
+
+ all_vm_events(events);
+ si_meminfo(i);
+
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN, K(events[PSWPIN]));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_OUT, K(events[PSWPOUT]));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_MAJFLT, events[PGMAJFAULT]);
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_MINFLT, events[PGFAULT]);
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE, K(i.freeram));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMTOT, K(i.totalram));
+}
+
+/*
+ * While most virtqueues communicate guest-initiated requests to the
hypervisor,
+ * the stats queue operates in reverse. The driver initializes the virtqueue
+ * with a single buffer. From that point forward, all conversations consist
of
+ * a hypervisor request (a call to this function) which directs us to refill
+ * the virtqueue with a fresh stats buffer.
+ */
+static void stats_ack(struct virtqueue *vq)
+{
+ struct virtio_balloon *vb;
+ unsigned int len;
+ struct scatterlist sg;
+
+ vb = vq-vq_ops-get_buf(vq, len);
+ if (!vb)
+ return;
+
+ update_balloon_stats(vb);
+
+ sg_init_one(sg, vb-stats, sizeof(vb-stats));
+ if (vq-vq_ops-add_buf(vq, sg, 1, 0, vb) 0)
+ BUG();
+ vq-vq_ops-kick(vq);
+}
+
static void virtballoon_changed(struct virtio_device *vdev)
{
struct virtio_balloon *vb = vdev-priv;
@@ -205,10 +259,10 @@ static int balloon(void *_vballoon)
static int virtballoon_probe(struct virtio_device *vdev)
{
struct virtio_balloon *vb;
- struct
Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V3)
On 11/23/2009 11:44 AM, Michael S. Tsirkin wrote:
On Thu, Nov 19, 2009 at 09:19:05AM -0600, Adam Litke wrote:
Rusty and Anthony,
If I've addressed all outstanding issues, please consider this patch for
inclusion. Thanks.
Changes since V2:
- Increase stat field size to 64 bits
- Report all sizes in kb (not pages)
- Drop anon_pages stat and fix endianness conversion
Changes since V1:
- Use a virtqueue instead of the device config space
When using ballooning to manage overcommitted memory on a host, a system for
guests to communicate their memory usage to the host can provide information
that will minimize the impact of ballooning on the guests. The current method
employs a daemon running in each guest that communicates memory statistics to a
host daemon at a specified time interval. The host daemon aggregates this
information and inflates and/or deflates balloons according to the level of
host memory pressure. This approach is effective but overly complex since a
daemon must be installed inside each guest and coordinated to communicate with
the host. A simpler approach is to collect memory statistics in the virtio
balloon driver and communicate them directly to the hypervisor.
This patch enables the guest-side support by adding stats collection and
reporting to the virtio balloon driver.
Signed-off-by: Adam Litkea...@us.ibm.com
Cc: Rusty Russellru...@rustcorp.com.au
Cc: Anthony Liguorianth...@codemonkey.ws
Cc: virtualizat...@lists.linux-foundation.org
Cc: linux-ker...@vger.kernel.org
that's pretty clean. some comments: I wrote them while off-line, and now
that I was going to send it out, I see that there's some overlap with
what Rusty wrote. Still, here it is:
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 200c22f..ebc9d39 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -29,7 +29,7 @@
struct virtio_balloon
{
struct virtio_device *vdev;
- struct virtqueue *inflate_vq, *deflate_vq;
+ struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
/* Where the ballooning thread waits for config to change. */
wait_queue_head_t config_change;
@@ -50,6 +50,9 @@ struct virtio_balloon
/* The array of pfns we tell the Host about. */
unsigned int num_pfns;
u32 pfns[256];
+
+ /* Memory statistics */
+ struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
};
static struct virtio_device_id id_table[] = {
@@ -155,6 +158,57 @@ static void leak_balloon(struct virtio_balloon *vb, size_t
num)
}
}
+static inline void update_stat(struct virtio_balloon *vb, int idx,
+ __le16 tag, __le64 val)
+{
+ BUG_ON(idx= VIRTIO_BALLOON_S_NR);
+ vb-stats[idx].tag = cpu_to_le16(tag);
+ vb-stats[idx].val = cpu_to_le64(val);
you should do le16_to_cpu etc on __le values.
Try running this patch through sparce checker.
+}
+
+#define K(x) ((x) (PAGE_SHIFT - 10))
can't this overflow?
also, won't it be simpler to just report memory is
in bytes, then just x * PAGE_SIZE instead of hairy constants?
+static void update_balloon_stats(struct virtio_balloon *vb)
+{
+ unsigned long events[NR_VM_EVENT_ITEMS];
that's about 1/2K worth of stack space on a 64 bit machine.
better keep it as part of struct virtio_balloon.
+ struct sysinfo i;
+ int idx = 0;
+
+ all_vm_events(events);
+ si_meminfo(i);
+
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN, K(events[PSWPIN]));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_OUT, K(events[PSWPOUT]));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_MAJFLT, events[PGMAJFAULT]);
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_MINFLT, events[PGFAULT]);
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE, K(i.freeram));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMTOT, K(i.totalram));
Finally I found some data from our M$ neighbors. This is from
http://www.microsoft.com/whdc/system/sysperf/Perf_tun_srv-R2.mspx
When running Windows in the child partition, you can use the following
performance counters within a child partition to identify whether the
child partition is experiencing memory pressure and is likely to perform
better with a higher VM memory size:
Memory – Standby Cache Reserve Bytes:
Sum of Standby Cache Reserve Bytes and Free and Zero Page List Bytes
should be 200 MB or more on systems with 1 GB, and 300 MB or more on
systems with 2 GB or more of visible RAM.
Memory – Free Zero Page List Bytes:
Sum of Standby Cache Reserve Bytes and Free and Zero Page List Bytes
should be 200 MB or more on systems with 1 GB, and 300 MB or more on
systems with 2 GB or more of visible RAM.
Memory – Pages Input/Sec:
Average over a 1-hour period is less than 10.
The biggest take out of it is that we may get #0-pages in windows.
It's valuable information for the host since KSM will collapse all of
them anyway, so there is
Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V3)
On 11/23/2009 01:00 PM, Dor Laor wrote:
On 11/23/2009 11:44 AM, Michael S. Tsirkin wrote:
On Thu, Nov 19, 2009 at 09:19:05AM -0600, Adam Litke wrote:
Rusty and Anthony,
If I've addressed all outstanding issues, please consider this patch
for
inclusion. Thanks.
Changes since V2:
- Increase stat field size to 64 bits
- Report all sizes in kb (not pages)
- Drop anon_pages stat and fix endianness conversion
Changes since V1:
- Use a virtqueue instead of the device config space
When using ballooning to manage overcommitted memory on a host, a
system for
guests to communicate their memory usage to the host can provide
information
that will minimize the impact of ballooning on the guests. The
current method
employs a daemon running in each guest that communicates memory
statistics to a
host daemon at a specified time interval. The host daemon
aggregates this
information and inflates and/or deflates balloons according to the
level of
host memory pressure. This approach is effective but overly complex
since a
daemon must be installed inside each guest and coordinated to
communicate with
the host. A simpler approach is to collect memory statistics in the
virtio
balloon driver and communicate them directly to the hypervisor.
This patch enables the guest-side support by adding stats collection
and
reporting to the virtio balloon driver.
Signed-off-by: Adam Litkea...@us.ibm.com
Cc: Rusty Russellru...@rustcorp.com.au
Cc: Anthony Liguorianth...@codemonkey.ws
Cc: virtualizat...@lists.linux-foundation.org
Cc: linux-ker...@vger.kernel.org
that's pretty clean. some comments: I wrote them while off-line, and now
that I was going to send it out, I see that there's some overlap with
what Rusty wrote. Still, here it is:
diff --git a/drivers/virtio/virtio_balloon.c
b/drivers/virtio/virtio_balloon.c
index 200c22f..ebc9d39 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -29,7 +29,7 @@
struct virtio_balloon
{
struct virtio_device *vdev;
-struct virtqueue *inflate_vq, *deflate_vq;
+struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
/* Where the ballooning thread waits for config to change. */
wait_queue_head_t config_change;
@@ -50,6 +50,9 @@ struct virtio_balloon
/* The array of pfns we tell the Host about. */
unsigned int num_pfns;
u32 pfns[256];
+
+/* Memory statistics */
+struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
};
static struct virtio_device_id id_table[] = {
@@ -155,6 +158,57 @@ static void leak_balloon(struct virtio_balloon
*vb, size_t num)
}
}
+static inline void update_stat(struct virtio_balloon *vb, int idx,
+__le16 tag, __le64 val)
+{
+BUG_ON(idx= VIRTIO_BALLOON_S_NR);
+vb-stats[idx].tag = cpu_to_le16(tag);
+vb-stats[idx].val = cpu_to_le64(val);
you should do le16_to_cpu etc on __le values.
Try running this patch through sparce checker.
+}
+
+#define K(x) ((x) (PAGE_SHIFT - 10))
can't this overflow?
also, won't it be simpler to just report memory is
in bytes, then just x * PAGE_SIZE instead of hairy constants?
+static void update_balloon_stats(struct virtio_balloon *vb)
+{
+unsigned long events[NR_VM_EVENT_ITEMS];
that's about 1/2K worth of stack space on a 64 bit machine.
better keep it as part of struct virtio_balloon.
+struct sysinfo i;
+int idx = 0;
+
+all_vm_events(events);
+si_meminfo(i);
+
+update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN,
K(events[PSWPIN]));
+update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_OUT,
K(events[PSWPOUT]));
+update_stat(vb, idx++, VIRTIO_BALLOON_S_MAJFLT,
events[PGMAJFAULT]);
+update_stat(vb, idx++, VIRTIO_BALLOON_S_MINFLT, events[PGFAULT]);
+update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE, K(i.freeram));
+update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMTOT, K(i.totalram));
Finally I found some data from our M$ neighbors. This is from
http://www.microsoft.com/whdc/system/sysperf/Perf_tun_srv-R2.mspx
When running Windows in the child partition, you can use the
following performance counters within a child partition to identify
whether the child partition is experiencing memory pressure and is
likely to perform better with a higher VM memory size:
Memory – Standby Cache Reserve Bytes:
Sum of Standby Cache Reserve Bytes and Free and Zero Page List Bytes
should be 200 MB or more on systems with 1 GB, and 300 MB or more on
systems with 2 GB or more of visible RAM.
Memory – Free Zero Page List Bytes:
Sum of Standby Cache Reserve Bytes and Free and Zero Page List Bytes
should be 200 MB or more on systems with 1 GB, and 300 MB or more on
systems with 2 GB or more of visible RAM.
Memory – Pages Input/Sec:
Average over a 1-hour period is less than 10.
The biggest take out of it is that we may get #0-pages in windows.
It's valuable information for the host since KSM will collapse all of
them anyway, so there is not point in
[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V3)
On 11/19/2009 05:19 PM, Adam Litke wrote:
Rusty and Anthony,
If I've addressed all outstanding issues, please consider this patch for
inclusion. Thanks.
+struct virtio_balloon_stat
+{
+ __le16 tag;
+ __le64 val;
+};
+
You're not doing endian conversion in the host?
--
error compiling committee.c: too many arguments to function
[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V3)
On Thu, 2009-11-19 at 17:22 +0200, Avi Kivity wrote:
On 11/19/2009 05:19 PM, Adam Litke wrote:
Rusty and Anthony,
If I've addressed all outstanding issues, please consider this patch for
inclusion. Thanks.
+struct virtio_balloon_stat
+{
+ __le16 tag;
+ __le64 val;
+};
+
You're not doing endian conversion in the host?
No. I was following by example. For the virtio_balloon, the existing
code is careful so that the guest always writes data in little endian.
--
Thanks,
Adam
[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V3)
On 11/19/2009 05:58 PM, Adam Litke wrote:
On Thu, 2009-11-19 at 17:22 +0200, Avi Kivity wrote:
On 11/19/2009 05:19 PM, Adam Litke wrote:
Rusty and Anthony,
If I've addressed all outstanding issues, please consider this patch for
inclusion. Thanks.
+struct virtio_balloon_stat
+{
+ __le16 tag;
+ __le64 val;
+};
+
You're not doing endian conversion in the host?
No. I was following by example. For the virtio_balloon, the existing
code is careful so that the guest always writes data in little endian.
I don't follow. If the guest is careful to write little-endian, surely
the host must be equally careful to read little-endian?
--
error compiling committee.c: too many arguments to function
[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V3)
On Thu, 2009-11-19 at 18:13 +0200, Avi Kivity wrote:
On 11/19/2009 05:58 PM, Adam Litke wrote:
On Thu, 2009-11-19 at 17:22 +0200, Avi Kivity wrote:
On 11/19/2009 05:19 PM, Adam Litke wrote:
Rusty and Anthony,
If I've addressed all outstanding issues, please consider this patch for
inclusion. Thanks.
+struct virtio_balloon_stat
+{
+ __le16 tag;
+ __le64 val;
+};
+
You're not doing endian conversion in the host?
No. I was following by example. For the virtio_balloon, the existing
code is careful so that the guest always writes data in little endian.
I don't follow. If the guest is careful to write little-endian, surely
the host must be equally careful to read little-endian?
That is true and, by my reading of the existing qemu virtio-balloon
device code, isn't virtio_balloon_set_config() on a big endian host
already broken?
--
Thanks,
Adam
[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V3)
On Fri, 20 Nov 2009 01:49:05 am Adam Litke wrote:
Rusty and Anthony,
If I've addressed all outstanding issues, please consider this patch for
inclusion. Thanks.
Changes since V2:
- Increase stat field size to 64 bits
- Report all sizes in kb (not pages)
Hi Adam,
Looks like we're very close. A few minor things:
Why k? Why not just do the simplest possible thing, and just report all
stats as straight numbers, now we have 64 bits.
- Drop anon_pages stat and fix endianness conversion
Please drop endianness conversion.
+struct virtio_balloon_stat
+{
+ __le16 tag;
+ __le64 val;
+};
Let's not introduce padding as well; __attribute__((packed)) here please.
Thanks,
Rusty.
