Re: virtio: Add memory statistics reporting to the balloon driver (V4)

2009-12-01 Thread Avi Kivity
On 12/01/2009 05:05 PM, Adam Litke wrote:
> On Tue, 2009-12-01 at 16:56 +0200, Avi Kivity wrote:
>
>> What about a spec update?  Did that happen and I just missed it?
>>  
> Yes it did.  I forgot to update this patch leader note with the link:
> http://ozlabs.org/~rusty/virtio-spec/virtio-spec-0.8.2.pdf
>

Great, thanks.


-- 
error compiling committee.c: too many arguments to function

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver (V4)

2009-12-01 Thread Adam Litke
On Tue, 2009-12-01 at 16:56 +0200, Avi Kivity wrote:
> What about a spec update?  Did that happen and I just missed it?

Yes it did.  I forgot to update this patch leader note with the link:
http://ozlabs.org/~rusty/virtio-spec/virtio-spec-0.8.2.pdf


-- 
Thanks,
Adam

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver (V4)

2009-12-01 Thread Avi Kivity
On 11/30/2009 06:14 PM, Adam Litke wrote:
> Changes since V3:
>   - Do not do endian conversions as they will be done in the host
>   - Report stats that reference a quantity of memory in bytes
>   - Minor coding style updates
>
> 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.
>
>

What about a spec update?  Did that happen and I just missed it?

-- 
error compiling committee.c: too many arguments to function

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver (V4)

2009-12-01 Thread Adam Litke
Thanks Rusty.

Acked-by: Adam Litke 

On Tue, 2009-12-01 at 12:54 +1030, Rusty Russell wrote:
> On Tue, 1 Dec 2009 02:44:15 am Adam Litke wrote:
> > Changes since V3:
> 
> OK, I applied this.  And here's the fixes I applied afterwards (to save YA
> round trip: please ack and I'll fold them together)
> 
> virtio: balloon fixes
> 
> 1) Tag and val args to update_stat() are no longer __le.
> 2) pages_to_bytes() should promote to u64 so we don't truncate.
> 3) Fix two checkpatch.pl warnings.
> 
> Signed-off-by: Rusty Russell 
> ---
>  drivers/virtio/virtio_balloon.c |8 +---
>  include/linux/virtio_balloon.h  |3 +--
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -158,14 +158,15 @@ static void leak_balloon(struct virtio_b
>  }
> 
>  static inline void update_stat(struct virtio_balloon *vb, int idx,
> - __le16 tag, __le64 val)
> +u16 tag, u64 val)
>  {
>   BUG_ON(idx >= VIRTIO_BALLOON_S_NR);
>   vb->stats[idx].tag = tag;
>   vb->stats[idx].val = val;
>  }
> 
> -#define pages_to_bytes(x) ((x) << PAGE_SHIFT)
> +#define pages_to_bytes(x) ((u64)(x) << PAGE_SHIFT)
> +
>  static void update_balloon_stats(struct virtio_balloon *vb)
>  {
>   unsigned long events[NR_VM_EVENT_ITEMS];
> @@ -296,7 +297,8 @@ static int virtballoon_probe(struct virt
>* use it to signal us later.
>*/
>   sg_init_one(&sg, vb->stats, sizeof vb->stats);
> - if (vb->stats_vq->vq_ops->add_buf(vb->stats_vq, &sg, 1, 0, vb) 
> < 0)
> + if (vb->stats_vq->vq_ops->add_buf(vb->stats_vq,
> +   &sg, 1, 0, vb) < 0)
>   BUG();
>   vb->stats_vq->vq_ops->kick(vb->stats_vq);
>   }
> diff --git a/include/linux/virtio_balloon.h b/include/linux/virtio_balloon.h
> --- a/include/linux/virtio_balloon.h
> +++ b/include/linux/virtio_balloon.h
> @@ -28,8 +28,7 @@ struct virtio_balloon_config
>  #define VIRTIO_BALLOON_S_MEMTOT   5   /* Total amount of memory */
>  #define VIRTIO_BALLOON_S_NR   6
> 
> -struct virtio_balloon_stat
> -{
> +struct virtio_balloon_stat {
>   u16 tag;
>   u64 val;
>  } __attribute__((packed));
> 
> 


-- 
Thanks,
Adam

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver (V4)

2009-11-30 Thread Rusty Russell
On Tue, 1 Dec 2009 02:44:15 am Adam Litke wrote:
> Changes since V3:

OK, I applied this.  And here's the fixes I applied afterwards (to save YA
round trip: please ack and I'll fold them together)

virtio: balloon fixes

1) Tag and val args to update_stat() are no longer __le.
2) pages_to_bytes() should promote to u64 so we don't truncate.
3) Fix two checkpatch.pl warnings.

Signed-off-by: Rusty Russell 
---
 drivers/virtio/virtio_balloon.c |8 +---
 include/linux/virtio_balloon.h  |3 +--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -158,14 +158,15 @@ static void leak_balloon(struct virtio_b
 }
 
 static inline void update_stat(struct virtio_balloon *vb, int idx,
-   __le16 tag, __le64 val)
+  u16 tag, u64 val)
 {
BUG_ON(idx >= VIRTIO_BALLOON_S_NR);
vb->stats[idx].tag = tag;
vb->stats[idx].val = val;
 }
 
-#define pages_to_bytes(x) ((x) << PAGE_SHIFT)
+#define pages_to_bytes(x) ((u64)(x) << PAGE_SHIFT)
+
 static void update_balloon_stats(struct virtio_balloon *vb)
 {
unsigned long events[NR_VM_EVENT_ITEMS];
@@ -296,7 +297,8 @@ static int virtballoon_probe(struct virt
 * use it to signal us later.
 */
sg_init_one(&sg, vb->stats, sizeof vb->stats);
-   if (vb->stats_vq->vq_ops->add_buf(vb->stats_vq, &sg, 1, 0, vb) 
< 0)
+   if (vb->stats_vq->vq_ops->add_buf(vb->stats_vq,
+ &sg, 1, 0, vb) < 0)
BUG();
vb->stats_vq->vq_ops->kick(vb->stats_vq);
}
diff --git a/include/linux/virtio_balloon.h b/include/linux/virtio_balloon.h
--- a/include/linux/virtio_balloon.h
+++ b/include/linux/virtio_balloon.h
@@ -28,8 +28,7 @@ struct virtio_balloon_config
 #define VIRTIO_BALLOON_S_MEMTOT   5   /* Total amount of memory */
 #define VIRTIO_BALLOON_S_NR   6
 
-struct virtio_balloon_stat
-{
+struct virtio_balloon_stat {
u16 tag;
u64 val;
 } __attribute__((packed));


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


virtio: Add memory statistics reporting to the balloon driver (V4)

2009-11-30 Thread Adam Litke
Changes since V3:
 - Do not do endian conversions as they will be done in the host
 - Report stats that reference a quantity of memory in bytes
 - Minor coding style updates

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 
Cc: Rusty Russell 
Cc: Anthony Liguori 
Cc: virtualization@lists.linux-foundation.org
Cc: linux-ker...@vger.kernel.org

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 200c22f..f4c8cd8 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,61 @@ 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 = tag;
+   vb->stats[idx].val = val;
+}
+
+#define pages_to_bytes(x) ((x) << PAGE_SHIFT)
+static void update_balloon_stats(struct virtio_balloon *vb)
+{
+   unsigned long events[NR_VM_EVENT_ITEMS];
+   struct sysinfo i;
+   int idx = 0;
+
+   all_vm_events(events);
+   si_meminfo(&i);
+
+   update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN,
+   pages_to_bytes(events[PSWPIN]));
+   update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_OUT,
+   pages_to_bytes(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,
+   pages_to_bytes(i.freeram));
+   update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMTOT,
+   pages_to_bytes(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 +263,10 @@ static int balloon(void *_vballoon)
 static int virtballoon_probe(struct virtio_device *vdev)
 {
struct virtio_balloon *vb;
-   struct virtqueue *vqs[2];
-   vq_callback_t *callbacks[] = { balloon_ack, balloon_ack };
-   const char *names[] = { "inflate", "deflate" };
-   int err;
+   struct virtqueue *vqs[3];
+   vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_ack };
+   const char *names[] = { "inflate", "deflate", "stats" };
+   int err, nvqs;
 
vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
if (!vb) {
@@ -221,13

Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V3)

2009-11-23 Thread Vadim Rozenfeld
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 Litke
>>> Cc: Rusty Russell
>>> Cc: Anthony Liguori
>>> Cc: virtualization@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 Z

Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V3)

2009-11-23 Thread Dor Laor
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 Litke
>> Cc: Rusty Russell
>> Cc: Anthony Liguori
>> Cc: virtualization@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 o

Re: virtio: Add memory statistics reporting to the balloon driver (V3)

2009-11-23 Thread Michael S. Tsirkin
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 
> Cc: Rusty Russell 
> Cc: Anthony Liguori 
> Cc: virtualization@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 

Re: virtio: Add memory statistics reporting to the balloon driver (V3)

2009-11-19 Thread Rusty Russell
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.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver (V3)

2009-11-19 Thread Adam Litke
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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver (V3)

2009-11-19 Thread Avi Kivity
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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver (V3)

2009-11-19 Thread Adam Litke
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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver (V3)

2009-11-19 Thread Avi Kivity
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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


virtio: Add memory statistics reporting to the balloon driver (V3)

2009-11-19 Thread Adam Litke
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 
Cc: Rusty Russell 
Cc: Anthony Liguori 
Cc: virtualization@lists.linux-foundation.org
Cc: linux-ker...@vger.kernel.org

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);
+}
+
+#define K(x) ((x) << (PAGE_SHIFT - 10))
+static void update_balloon_stats(struct virtio_balloon *vb)
+{
+   unsigned long events[NR_VM_EVENT_ITEMS];
+   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 virtqueue *vqs[2];
-   vq_callback_t *callbacks[] = { balloon_ack, balloon_ack };
-   const char *names[] = { "inflate", "deflate" };
-   int err;
+   struct virtqueue *vqs[3];
+   vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_ack };
+   const char *names[] = { "inflate", "deflate", "stats" };
+   int err, nvqs;
 
vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
if (!vb) {
@@ -221,13 +275,28 @@ static int virtballoon_probe(struct virtio_device *vdev)
init_waitqueue_head(&vb->config_change);
vb->vdev = vdev;
 
-   /* We expect two virtqueues. */
-   err = vdev->config->find_vqs(

Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V2)

2009-11-18 Thread Rusty Russell
On Thu, 19 Nov 2009 01:32:26 am Anthony Liguori wrote:
> Rusty Russell wrote:
> > The little-endian conversion of the balloon driver is a historical mistake
> > (no other driver does this).  Let's not extend it to the stats.
> 
> I think the mistake is that the other drivers don't do that.
> 
> We cheat in qemu and assume that the guest is always in a fixed 
> endianness but this is not always the case for all architectures.

Perhaps, but it's documented in the spec.  My assertion remains that to do
any virtualization you need to know what the guest endian is anyway, so
endian converts throughout the drivers just add pain for driver authors.

> I think making the interface u64 and byte based would be the best 
> solution.  Making assumptions about page size across guest and host is 
> another thing we should try to avoid.

Yep, just report the raw byte counts as u64.

Cheers,
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V2)

2009-11-18 Thread Jamie Lokier
Anthony Liguori wrote:
> Rusty Russell wrote:
> >The little-endian conversion of the balloon driver is a historical mistake
> >(no other driver does this).  Let's not extend it to the stats.
> 
> I think the mistake is that the other drivers don't do that.
> 
> We cheat in qemu and assume that the guest is always in a fixed 
> endianness but this is not always the case for all architectures.

If guests can have different endianness (reasonable on some CPUs where
it's switchable - some even have more than 2 options), then I guess
the *host* on those systems have different endianness too.

Is the host's endianness signalled to the guest anywhere, so that
guest drivers can do cpu_to_qemuhost32(), when someone eventually
finds that necessary?

-- Jamie
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V2)

2009-11-18 Thread Anthony Liguori
Rusty Russell wrote:
> On Wed, 18 Nov 2009 07:06:29 am Adam Litke wrote:
>   
>> virtio: Add memory statistics reporting to the balloon driver (V2)
>>
>> Changes since V1:
>>  - Use a virtqueue instead of the device config space
>> 
>
> Hi Adam,
>
> If Anthony's happy, I'm happy with this approach.
>
> Couple of minor points:
>
>   
>> +static inline void update_stat(struct virtio_balloon *vb, int idx,
>> +unsigned int tag, unsigned long val)
>> +{
>> +BUG_ON(idx >= VIRTIO_BALLOON_S_NR);
>> +vb->stats[idx].tag = tag;
>> +vb->stats[idx].val = cpu_to_le32(val);
>> +}
>> 
>
> The little-endian conversion of the balloon driver is a historical mistake
> (no other driver does this).  Let's not extend it to the stats.
>   

I think the mistake is that the other drivers don't do that.

We cheat in qemu and assume that the guest is always in a fixed 
endianness but this is not always the case for all architectures.

That said, since we make this mistake everywhere, I guess I understand 
the argument to have consistency and to just admit that we're broken 
here.  But this is where the endianness bits come from.

> Here you've done one and not the other, which is even worse.  (Sparse would
> have found this, I assume).
>   

Yup, that's definitely wrong.

>> +struct virtio_balloon_stat
>> +{
>> +__le16 tag;
>> +__le32 val;
>> +};
>> 
>
> Is 32 bits sufficient?  A big machine might get over 4bn faults, and certainly
> 4 TB of memory isn't that far away.
>   

I think making the interface u64 and byte based would be the best 
solution.  Making assumptions about page size across guest and host is 
another thing we should try to avoid.

Regards,

Anthony Liguori
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver (V2)

2009-11-17 Thread Rusty Russell
On Wed, 18 Nov 2009 07:06:29 am Adam Litke wrote:
> virtio: Add memory statistics reporting to the balloon driver (V2)
> 
> Changes since V1:
>  - Use a virtqueue instead of the device config space

Hi Adam,

If Anthony's happy, I'm happy with this approach.

Couple of minor points:

> +static inline void update_stat(struct virtio_balloon *vb, int idx,
> + unsigned int tag, unsigned long val)
> +{
> + BUG_ON(idx >= VIRTIO_BALLOON_S_NR);
> + vb->stats[idx].tag = tag;
> + vb->stats[idx].val = cpu_to_le32(val);
> +}

The little-endian conversion of the balloon driver is a historical mistake
(no other driver does this).  Let's not extend it to the stats.

Here you've done one and not the other, which is even worse.  (Sparse would
have found this, I assume).

> +struct virtio_balloon_stat
> +{
> + __le16 tag;
> + __le32 val;
> +};

Is 32 bits sufficient?  A big machine might get over 4bn faults, and certainly
4 TB of memory isn't that far away.

Thanks,
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


virtio: Add memory statistics reporting to the balloon driver (V2)

2009-11-17 Thread Adam Litke
virtio: Add memory statistics reporting to the balloon driver (V2)

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 
Cc: Rusty Russell 
Cc: Anthony Liguori 
Cc: virtualization@lists.linux-foundation.org
Cc: linux-ker...@vger.kernel.org

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 200c22f..59b9533 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,60 @@ static void leak_balloon(struct virtio_balloon *vb, size_t 
num)
}
 }
 
+static inline void update_stat(struct virtio_balloon *vb, int idx,
+   unsigned int tag, unsigned long val)
+{
+   BUG_ON(idx >= VIRTIO_BALLOON_S_NR);
+   vb->stats[idx].tag = tag;
+   vb->stats[idx].val = cpu_to_le32(val);
+}
+
+#define K(x) ((x) << (PAGE_SHIFT - 10))
+static void update_balloon_stats(struct virtio_balloon *vb)
+{
+   unsigned long events[NR_VM_EVENT_ITEMS];
+   struct sysinfo i;
+   unsigned long anon_pages;
+   int idx = 0;
+
+   all_vm_events(events);
+   si_meminfo(&i);
+   anon_pages = K(global_page_state(NR_ANON_PAGES));
+
+   update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN, events[PSWPIN]);
+   update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_OUT, events[PSWPOUT]);
+   update_stat(vb, idx++, VIRTIO_BALLOON_S_ANON, anon_pages);
+   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 +262,10 @@ static int balloon(void *_vballoon)
 static int virtballoon_probe(struct virtio_device *vdev)
 {
struct virtio_balloon *vb;
-   struct virtqueue *vqs[2];
-   vq_callback_t *callbacks[] = { balloon_ack, balloon_ack };
-   const char *names[] = { "inflate", "deflate" };
-   int err;
+   struct virtqueue *vqs[3];
+   vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_ack };
+   const char *names[] = { "inflate", "deflate", "stats" };
+   int err, nvqs;
 
vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
if (!vb) {
@@ -221,13 +278,28 @@ static int virtballoon_probe(struct virtio_device *vdev)
init_waitqueue_head(&vb->config_change);
vb-&g

Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-11 Thread Rusty Russell
On Thu, 12 Nov 2009 01:38:34 am Adam Litke wrote:
> > But it raises the question: what stats are generally useful cross-OS?  
> > Should
> > we be supplying numbers like "unused" (free) "instantly discardable" (ie.
> > clean), "discardable to disk" (ie. file-backed), "discardable to swap"
> > (ie. swap-backed) and "unswappable" instead?
> 
> While I see the virtue in presenting abstracted memory stats that seem
> more digestible in a virtualization context, I think we should keep the
> raw stats.  This concentrates the complexity in the host-side management
> daemon, and allows the host daemon to make better decisions (ie. by
> reacting to trends in individual statistics).  Different OSes (or
> different versions of the same OS), may also have different sets of
> statistics that will provide the answers that a management daemon needs.

OK, I see you made each one a separate feature bit, which does allow this
somewhat.  But you can't just change the meaning arbitrarily, all you can
do is refuse to supply some of them.  This is because virtio is an ABI,
but also it's plain sanity: run a new guest on an old host and get crazy
answers.

Two more questions:

I assume memtot should be equal to the initial memory granted to the guest
(perhaps reduced if the guest can't use all the memory for internal reasons)?

I'm not sure of the relevance to the host of the number of anonymous pages?
That's why I wondered if unswappable pages would be a better number to supply?

Thanks,
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-11 Thread Adam Litke
On Wed, 2009-11-11 at 13:13 +1030, Rusty Russell wrote:
> > It's not laziness, it's consistency.  How is actual different than free 
> > memory or any other stat?
> 
> Because it's a COLLECTION of stats.  For example, swap in should be < swap
> out.  Now, the current Linux implementation of all_vm_events() is non-atomic
> anyway, so maybe we can just document this as best-effort.  I'm saying that
> if it *is* a problem, I think we need a vq.

I can't see why we would care about the atomicity of the collection of
statistics.  Best-effort is good enough.  Any variance within the stats
will be overshadowed by the latency of the host-side management daemon.

> But it raises the question: what stats are generally useful cross-OS?  Should
> we be supplying numbers like "unused" (free) "instantly discardable" (ie.
> clean), "discardable to disk" (ie. file-backed), "discardable to swap"
> (ie. swap-backed) and "unswappable" instead?

While I see the virtue in presenting abstracted memory stats that seem
more digestible in a virtualization context, I think we should keep the
raw stats.  This concentrates the complexity in the host-side management
daemon, and allows the host daemon to make better decisions (ie. by
reacting to trends in individual statistics).  Different OSes (or
different versions of the same OS), may also have different sets of
statistics that will provide the answers that a management daemon needs.


-- 
Thanks,
Adam

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-11 Thread Avi Kivity
On 11/11/2009 03:26 PM, Adam Litke wrote:
> On Wed, 2009-11-11 at 10:12 +, Daniel P. Berrange wrote:
>
>> This all suggests that we should only update the stats from the guest
>> when something on the host actually asks for them by issuing the QEMU
>> monitor command. We don't want any kind of continuous polling of stats
>> at any frequency, if nothing is using these stats on the host.
>>  
> Agreed.  The next version of the patch will remove the timer completely.
> We'll wake up in response to config change notifications only.
>

A vq with its own interrupt would be much nicer.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-11 Thread Adam Litke
On Wed, 2009-11-11 at 10:12 +, Daniel P. Berrange wrote:
> This all suggests that we should only update the stats from the guest
> when something on the host actually asks for them by issuing the QEMU
> monitor command. We don't want any kind of continuous polling of stats
> at any frequency, if nothing is using these stats on the host.

Agreed.  The next version of the patch will remove the timer completely.
We'll wake up in response to config change notifications only.

-- 
Thanks,
Adam

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-11 Thread Daniel P. Berrange
On Wed, Nov 11, 2009 at 09:24:09AM +, Jamie Lokier wrote:
> Anthony Liguori wrote:
> > Avi Kivity wrote:
> > >On 11/10/2009 04:36 PM, Anthony Liguori wrote:
> > >>
> > >>>A stats vq might solve this more cleanly?
> > >>
> > >>actual and target are both really just stats.  Had we implemented 
> > >>those with a vq, I'd be inclined to agree with you but since they're 
> > >>implemented in the config space, it seems natural to extend the 
> > >>config space with other stats.
> > >>
> > >
> > >There is in fact a difference; actual and target are very rarely 
> > >updated, while the stats are updated very often.  Using a vq means a 
> > >constant number of exits per batch instead of one exit per statistic.  
> > >If the vq is host-driven, it also allows the host to control the 
> > >update frequency dynamically (i.e. stop polling when there is no 
> > >memory pressure).
> > 
> > I'm not terribly opposed to using a vq for this.  I would expect the 
> > stat update interval to be rather long (10s probably) but a vq works 
> > just as well.
> 
> If there's no memory pressure and no guest activity, you probably want
> the stat update to be as rare as possible to avoid wakeups.  Save
> power on laptops, that sort of thing.
> 
> If there's a host user interested in the state ("qemutop?"), you may
> want updates more often than 10s.

This all suggests that we should only update the stats from the guest
when something on the host actually asks for them by issuing the QEMU
monitor command. We don't want any kind of continuous polling of stats
at any frequency, if nothing is using these stats on the host.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-11 Thread Jamie Lokier
Anthony Liguori wrote:
> Avi Kivity wrote:
> >On 11/10/2009 04:36 PM, Anthony Liguori wrote:
> >>
> >>>A stats vq might solve this more cleanly?
> >>
> >>actual and target are both really just stats.  Had we implemented 
> >>those with a vq, I'd be inclined to agree with you but since they're 
> >>implemented in the config space, it seems natural to extend the 
> >>config space with other stats.
> >>
> >
> >There is in fact a difference; actual and target are very rarely 
> >updated, while the stats are updated very often.  Using a vq means a 
> >constant number of exits per batch instead of one exit per statistic.  
> >If the vq is host-driven, it also allows the host to control the 
> >update frequency dynamically (i.e. stop polling when there is no 
> >memory pressure).
> 
> I'm not terribly opposed to using a vq for this.  I would expect the 
> stat update interval to be rather long (10s probably) but a vq works 
> just as well.

If there's no memory pressure and no guest activity, you probably want
the stat update to be as rare as possible to avoid wakeups.  Save
power on laptops, that sort of thing.

If there's a host user interested in the state ("qemutop?"), you may
want updates more often than 10s.

-- Jamie
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-10 Thread Rusty Russell
On Wed, 11 Nov 2009 10:37:56 am Anthony Liguori wrote:
> Rusty Russell wrote:
> > You register an outbuf at initialization time.  The host hands it back when
> > it wants you to refill it with stats.
> 
> That's strangely backwards.  Guest send a stat buffer that's filled out, 
> host acks it when it wants another.  That doesn't seem bizarre to you?

Yep!  But that's a limitation of our brains, not the infrastructure ;)

Think of the stats as an infinite stream of data.  Read from it at your
leisure.  This is how, for example, console output works.

> > But the universe is remarkably indifferent to what we want.  Is it actually
> > sufficient or are we going to regret our laziness?
> 
> It's not laziness, it's consistency.  How is actual different than free 
> memory or any other stat?

Because it's a COLLECTION of stats.  For example, swap in should be < swap
out.  Now, the current Linux implementation of all_vm_events() is non-atomic
anyway, so maybe we can just document this as best-effort.  I'm saying that
if it *is* a problem, I think we need a vq.

But it raises the question: what stats are generally useful cross-OS?  Should
we be supplying numbers like "unused" (free) "instantly discardable" (ie.
clean), "discardable to disk" (ie. file-backed), "discardable to swap"
(ie. swap-backed) and "unswappable" instead?

(I just made those up, of course, but it seems like that would give a fair
indication of real memory pressure in any OS).

Thanks,
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-10 Thread Anthony Liguori
Rusty Russell wrote:
> On Wed, 11 Nov 2009 08:22:42 am Anthony Liguori wrote:
>   
>> Rusty Russell wrote:
>> 
>>> On Tue, 10 Nov 2009 03:02:06 am Adam Litke wrote:
>>>   
>>>   
 A simpler approach is to collect memory statistics in the virtio
 balloon driver and communicate them to the host via the device config 
 space.
 
 
>>> There are two issues I see with this.  First, there's an atomicity problem
>>> since you can't tell when the stats are consistent.  Second, polling is
>>> ugly.
>>>
>>> A stats vq might solve this more cleanly?
>>>   
>>>   
>> This turns out to not work so nicely.  You really need bidirectional 
>> communication.  You need to request that stats be collected and then you 
>> need to tell the hypervisor about the stats that were collected.  You 
>> don't need any real correlation between requests and stat reports either.
>> 
>
> You register an outbuf at initialization time.  The host hands it back when
> it wants you to refill it with stats.
>   

That's strangely backwards.  Guest send a stat buffer that's filled out, 
host acks it when it wants another.  That doesn't seem bizarre to you?

>> This really models how target/actual work and I think it suggests that 
>> we want to reuse that mechanism for the stats too.
>> 
>
> Sure, I want to.  You want to.  It's simple.
>
> But the universe is remarkably indifferent to what we want.  Is it actually
> sufficient or are we going to regret our laziness?
>   

It's not laziness, it's consistency.  How is actual different than free 
memory or any other stat?

> Cheers,
> Rusty.
>   


-- 
Regards,

Anthony Liguori

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-10 Thread Rusty Russell
On Wed, 11 Nov 2009 08:22:42 am Anthony Liguori wrote:
> Rusty Russell wrote:
> > On Tue, 10 Nov 2009 03:02:06 am Adam Litke wrote:
> >   
> >> A simpler approach is to collect memory statistics in the virtio
> >> balloon driver and communicate them to the host via the device config 
> >> space.
> >> 
> >
> > There are two issues I see with this.  First, there's an atomicity problem
> > since you can't tell when the stats are consistent.  Second, polling is
> > ugly.
> >
> > A stats vq might solve this more cleanly?
> >   
> 
> This turns out to not work so nicely.  You really need bidirectional 
> communication.  You need to request that stats be collected and then you 
> need to tell the hypervisor about the stats that were collected.  You 
> don't need any real correlation between requests and stat reports either.

You register an outbuf at initialization time.  The host hands it back when
it wants you to refill it with stats.

> This really models how target/actual work and I think it suggests that 
> we want to reuse that mechanism for the stats too.

Sure, I want to.  You want to.  It's simple.

But the universe is remarkably indifferent to what we want.  Is it actually
sufficient or are we going to regret our laziness?

Cheers,
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-10 Thread Rusty Russell
On Wed, 11 Nov 2009 01:06:14 am Anthony Liguori wrote:
> Rusty Russell wrote:
> > On Tue, 10 Nov 2009 03:02:06 am Adam Litke wrote:
> >   
> >> A simpler approach is to collect memory statistics in the virtio
> >> balloon driver and communicate them to the host via the device config 
> >> space.
> >> 
> >
> > There are two issues I see with this.  First, there's an atomicity problem
> > since you can't tell when the stats are consistent.
> 
> Actually, config writes always require notification from the guest to 
> the host.  This means the host knows when they config space is changed 
> so atomicity isn't a problem.

I think you missed my point: the stats are inter-related, so they should be
served together.

> In fact, if it were a problem, then the balloon driver would be 
> fundamentally broken because target and actual are stored in the config 
> space.

No, one is written by the host, the other the guest.  Still works.

> If you recall, we had this discussion originally wrt the balloon driver :-)

And I never did get around to the lguest implementation, which would have
seen if this really is an issue.

> >   Second, polling is ugly.
> 
> As opposed to?

As opposed to giving the stats whenever asked by the host.

> > A stats vq might solve this more cleanly?
> >   
> 
> actual and target are both really just stats.  Had we implemented those 
> with a vq, I'd be inclined to agree with you but since they're 
> implemented in the config space, it seems natural to extend the config 
> space with other stats.

It does, *if* we don't need accuracy.  Otherwise, it seems like we need
something else.

Cheers,
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-10 Thread Anthony Liguori
Rusty Russell wrote:
> On Tue, 10 Nov 2009 03:02:06 am Adam Litke wrote:
>   
>> A simpler approach is to collect memory statistics in the virtio
>> balloon driver and communicate them to the host via the device config space.
>> 
>
> There are two issues I see with this.  First, there's an atomicity problem
> since you can't tell when the stats are consistent.  Second, polling is
> ugly.
>
> A stats vq might solve this more cleanly?
>   

This turns out to not work so nicely.  You really need bidirectional 
communication.  You need to request that stats be collected and then you 
need to tell the hypervisor about the stats that were collected.  You 
don't need any real correlation between requests and stat reports either.

This really models how target/actual work and I think it suggests that 
we want to reuse that mechanism for the stats too.

> Rusty.
>   


-- 
Regards,

Anthony Liguori

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-10 Thread Anthony Liguori
Avi Kivity wrote:
> On 11/10/2009 04:36 PM, Anthony Liguori wrote:
>>
>>> A stats vq might solve this more cleanly?
>>
>> actual and target are both really just stats.  Had we implemented 
>> those with a vq, I'd be inclined to agree with you but since they're 
>> implemented in the config space, it seems natural to extend the 
>> config space with other stats.
>>
>
> There is in fact a difference; actual and target are very rarely 
> updated, while the stats are updated very often.  Using a vq means a 
> constant number of exits per batch instead of one exit per statistic.  
> If the vq is host-driven, it also allows the host to control the 
> update frequency dynamically (i.e. stop polling when there is no 
> memory pressure).

I'm not terribly opposed to using a vq for this.  I would expect the 
stat update interval to be rather long (10s probably) but a vq works 
just as well.

-- 
Regards,

Anthony Liguori

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-10 Thread Avi Kivity
On 11/10/2009 04:36 PM, Anthony Liguori wrote:
>
>> A stats vq might solve this more cleanly?
>
> actual and target are both really just stats.  Had we implemented 
> those with a vq, I'd be inclined to agree with you but since they're 
> implemented in the config space, it seems natural to extend the config 
> space with other stats.
>

There is in fact a difference; actual and target are very rarely 
updated, while the stats are updated very often.  Using a vq means a 
constant number of exits per batch instead of one exit per statistic.  
If the vq is host-driven, it also allows the host to control the update 
frequency dynamically (i.e. stop polling when there is no memory pressure).

-- 
error compiling committee.c: too many arguments to function

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-10 Thread Anthony Liguori
Rusty Russell wrote:
> On Tue, 10 Nov 2009 03:02:06 am Adam Litke wrote:
>   
>> A simpler approach is to collect memory statistics in the virtio
>> balloon driver and communicate them to the host via the device config space.
>> 
>
> There are two issues I see with this.  First, there's an atomicity problem
> since you can't tell when the stats are consistent.

Actually, config writes always require notification from the guest to 
the host.  This means the host knows when they config space is changed 
so atomicity isn't a problem.

In fact, if it were a problem, then the balloon driver would be 
fundamentally broken because target and actual are stored in the config 
space.

If you recall, we had this discussion originally wrt the balloon driver :-)

>   Second, polling is
> ugly.
>   

As opposed to?  The guest could set a timer and update the values 
periodically but that's even uglier because then the host cannot 
determine the update granularity.

> A stats vq might solve this more cleanly?
>   

actual and target are both really just stats.  Had we implemented those 
with a vq, I'd be inclined to agree with you but since they're 
implemented in the config space, it seems natural to extend the config 
space with other stats.

Regards,

Anthony Liguori
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-09 Thread Rusty Russell
On Tue, 10 Nov 2009 03:02:06 am Adam Litke wrote:
> A simpler approach is to collect memory statistics in the virtio
> balloon driver and communicate them to the host via the device config space.

There are two issues I see with this.  First, there's an atomicity problem
since you can't tell when the stats are consistent.  Second, polling is
ugly.

A stats vq might solve this more cleanly?
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


virtio: Add memory statistics reporting to the balloon driver

2009-11-09 Thread Adam Litke
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 to the host via the device config space.

This patch enables the guest-side support by adding stats collection and
reporting to the virtio balloon driver.

Comments?

Signed-off-by: Adam Litke 
Cc: Rusty Russell 
Cc: Anthony Liguori 
Cc: Avi Kivity 
Cc: virtualization@lists.linux-foundation.org
Cc: linux-ker...@vger.kernel.org

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 200c22f..0c9a9a1 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -180,6 +180,41 @@ static void update_balloon_size(struct virtio_balloon *vb)
  &actual, sizeof(actual));
 }
 
+static inline void update_stat(struct virtio_device *vdev, int feature,
+   unsigned long value, unsigned offset)
+{
+   __le32 __v = cpu_to_le32(value);
+   if (virtio_has_feature(vdev, feature))
+   vdev->config->set(vdev, offset, &__v, sizeof(__v));
+}
+
+#define K(x) ((x) << (PAGE_SHIFT - 10))
+static void update_balloon_stats(struct virtio_balloon *vb)
+{
+   unsigned long events[NR_VM_EVENT_ITEMS];
+   struct sysinfo i;
+   unsigned off = offsetof(struct virtio_balloon_config, stats);
+
+   all_vm_events(events);
+
+   update_stat(vb->vdev, VIRTIO_BALLOON_F_RPT_SWAP_IN, events[PSWPIN],
+   off + offsetof(struct virtio_balloon_stats, pswapin));
+   update_stat(vb->vdev, VIRTIO_BALLOON_F_RPT_SWAP_OUT, events[PSWPOUT],
+   off + offsetof(struct virtio_balloon_stats, pswapout));
+   update_stat(vb->vdev, VIRTIO_BALLOON_F_RPT_MAJFLT, events[PGMAJFAULT],
+   off + offsetof(struct virtio_balloon_stats, pgmajfault));
+   update_stat(vb->vdev, VIRTIO_BALLOON_F_RPT_MINFLT, events[PGFAULT],
+   off + offsetof(struct virtio_balloon_stats, pgminfault));
+   update_stat(vb->vdev, VIRTIO_BALLOON_F_RPT_ANON,
+   K(global_page_state(NR_ANON_PAGES)),
+   off + offsetof(struct virtio_balloon_stats, panon));
+   si_meminfo(&i);
+   update_stat(vb->vdev, VIRTIO_BALLOON_F_RPT_MEMFREE, K(i.freeram),
+   off + offsetof(struct virtio_balloon_stats, memfree));
+   update_stat(vb->vdev, VIRTIO_BALLOON_F_RPT_MEMTOT, K(i.totalram),
+   off + offsetof(struct virtio_balloon_stats, memtot));
+}
+
 static int balloon(void *_vballoon)
 {
struct virtio_balloon *vb = _vballoon;
@@ -189,15 +224,17 @@ static int balloon(void *_vballoon)
s64 diff;
 
try_to_freeze();
-   wait_event_interruptible(vb->config_change,
+   wait_event_interruptible_timeout(vb->config_change,
 (diff = towards_target(vb)) != 0
 || kthread_should_stop()
-|| freezing(current));
+|| freezing(current),
+VIRTIO_BALLOON_TIMEOUT);
if (diff > 0)
fill_balloon(vb, diff);
else if (diff < 0)
leak_balloon(vb, -diff);
update_balloon_size(vb);
+   update_balloon_stats(vb);
}
return 0;
 }
@@ -265,7 +302,12 @@ static void virtballoon_remove(struct virtio_device *vdev)
kfree(vb);
 }
 
-static unsigned int features[] = { VIRTIO_BALLOON_F_MUST_TELL_HOST };
+static unsigned int features[] = {
+   VIRTIO_BALLOON_F_MUST_TELL_HOST, VIRTIO_BALLOON_F_RPT_SWAP_IN,
+   VIRTIO_BALLOON_F_RPT_SWAP_OUT, VIRTIO_BALLOON_F_RPT_ANON,
+   VIRTIO_BALLOON_F_RPT_MAJFLT, VIRTIO_BALLOON_F_RPT_MINFLT,
+   VIRTIO_BALLOON_F_RPT_MEMFREE, VIRTIO_BALLOON_F_RPT_MEMTOT,
+};
 
 static struct virtio_driver virtio_balloon = {
.feature_table = features,
diff --git a/include/linux/virtio_balloon.h b/include/linux/virtio_balloon.h
index 09d7300..0bff4b8 100644
--- a/include/linux/virtio_balloon.h
+++ b/include/linux/virtio_balloon.h
@@ -6,15 +6,39 @@
 
 /* The feature bitmap for virtio balloon */
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST0 /* Tell before reclaiming 
pages */
+  /*

Re: virtio: Add memory statistics reporting to the balloon driver

2009-11-05 Thread Anthony Liguori
a...@linux.vnet.ibm.com wrote:
> Here are the corresponding changes to the Linux virtio driver...
>
>     virtio: Add memory statistics reporting to the balloon driver
> 
> 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 to the host via the device config 
> space.
> 
> This patch enables the guest-side support by adding stats collection and
> reporting to the virtio balloon driver.
> 
> Signed-off-by: Adam Litke 
>
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 3a43ebf..1029363 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -135,6 +135,7 @@ static int virtio_dev_probe(struct device *_d)
>   set_bit(i, dev->features);
>
>   dev->config->finalize_features(dev);
> + printk("virtio_dev_probe: final features = %lx\n", dev->features[0]);
>   

Looks like leftover debugging.

>   err = drv->probe(dev);
>   if (err)
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 200c22f..77cb953 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -180,6 +180,45 @@ static void update_balloon_size(struct virtio_balloon 
> *vb)
> &actual, sizeof(actual));
>  }
>
> +static inline void update_stat(struct virtio_device *vdev, int feature,
> + unsigned long value, unsigned offset)
> +{
> + if (virtio_has_feature(vdev, feature)) {
> + vdev->config->set(vdev, offset, &value, sizeof(value));
>   

I think this bit assumes a little endian guest.  We shouldn't make that 
assumption.

For virtio kernel patches, please CC the virtualization list and Rusty 
as he's the maintainer.  It wouldn't hurt to CC lkml either.

-- 
Regards,

Anthony Liguori

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization