Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1340757 > unrolled thread

Re: [PATCH 2/2] virtio_balloon: export 'available' memory to balloon statistics

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] virtio_balloon: export 'available' memory to balloon  statistics "Michael S. Tsirkin" <mst@redhat.com> - 2016-02-23 16:20 +0100
    Re: [PATCH 2/2] virtio_balloon: export 'available' memory to balloon  statistics "Denis V. Lunev" <den@openvz.org> - 2016-02-23 16:30 +0100
      Re: [PATCH 2/2] virtio_balloon: export 'available' memory to balloon  statistics "Michael S. Tsirkin" <mst@redhat.com> - 2016-02-23 17:00 +0100
        Re: [PATCH 2/2] virtio_balloon: export 'available' memory to balloon  statistics "Denis V. Lunev" <den@openvz.org> - 2016-02-23 17:20 +0100

#1340757 — Re: [PATCH 2/2] virtio_balloon: export 'available' memory to balloon statistics

On Tue, Feb 16, 2016 at 06:50:52PM +0300, Denis V. Lunev wrote:
> From: Igor Redko <redkoi@virtuozzo.com>
> 
> Add a new field, VIRTIO_BALLOON_S_AVAIL, to virtio_balloon memory
> statistics protocol, corresponding to 'Available' in /proc/meminfo.
> 
> It indicates to the hypervisor how big the balloon can be inflated
> without pushing the guest system to swap.
> 
> Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
> Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Andrew Morton <akpm@linux-foundation.org>

Oops - I missed the fact that this affects host/guest ABI.

Can you please submit ABI update proposal to virtio tc?
Spec patch would be even better.

This is important to ensure there are no conflicts
with other features being developed in parallel.

> ---
>  drivers/virtio/virtio_balloon.c     | 6 ++++++
>  include/uapi/linux/virtio_balloon.h | 3 ++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 0c3691f..f2b77de 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -30,6 +30,7 @@
>  #include <linux/balloon_compaction.h>
>  #include <linux/oom.h>
>  #include <linux/wait.h>
> +#include <linux/mm.h>
>  
>  /*
>   * Balloon device works in 4K page units.  So each page is pointed to by
> @@ -229,10 +230,13 @@ static void update_balloon_stats(struct virtio_balloon *vb)
>  	unsigned long events[NR_VM_EVENT_ITEMS];
>  	struct sysinfo i;
>  	int idx = 0;
> +	long available;
>  
>  	all_vm_events(events);
>  	si_meminfo(&i);
>  
> +	available = si_mem_available();
> +
>  	update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN,
>  				pages_to_bytes(events[PSWPIN]));
>  	update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_OUT,
> @@ -243,6 +247,8 @@ static void update_balloon_stats(struct virtio_balloon *vb)
>  				pages_to_bytes(i.freeram));
>  	update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMTOT,
>  				pages_to_bytes(i.totalram));
> +	update_stat(vb, idx++, VIRTIO_BALLOON_S_AVAIL,
> +				pages_to_bytes(available));
>  }
>  
>  /*
> diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
> index d7f1cbc..343d7dd 100644
> --- a/include/uapi/linux/virtio_balloon.h
> +++ b/include/uapi/linux/virtio_balloon.h
> @@ -51,7 +51,8 @@ struct virtio_balloon_config {
>  #define VIRTIO_BALLOON_S_MINFLT   3   /* Number of minor faults */
>  #define VIRTIO_BALLOON_S_MEMFREE  4   /* Total amount of free memory */
>  #define VIRTIO_BALLOON_S_MEMTOT   5   /* Total amount of memory */
> -#define VIRTIO_BALLOON_S_NR       6
> +#define VIRTIO_BALLOON_S_AVAIL    6   /* Available memory as in /proc */
> +#define VIRTIO_BALLOON_S_NR       7
>  
>  /*
>   * Memory statistics structure.
> -- 
> 2.5.0

[toc] | [next] | [standalone]

#1340779

On 02/23/2016 06:10 PM, Michael S. Tsirkin wrote:
> On Tue, Feb 16, 2016 at 06:50:52PM +0300, Denis V. Lunev wrote:
>> From: Igor Redko <redkoi@virtuozzo.com>
>>
>> Add a new field, VIRTIO_BALLOON_S_AVAIL, to virtio_balloon memory
>> statistics protocol, corresponding to 'Available' in /proc/meminfo.
>>
>> It indicates to the hypervisor how big the balloon can be inflated
>> without pushing the guest system to swap.
>>
>> Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
>> Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> CC: Andrew Morton <akpm@linux-foundation.org>
> Oops - I missed the fact that this affects host/guest ABI.
>
> Can you please submit ABI update proposal to virtio tc?
> Spec patch would be even better.
>
> This is important to ensure there are no conflicts
> with other features being developed in parallel.

hmmm

 From my point of view ABI remains untouched.
The guest can send any amount of <tag>;<value>
pairs and unknown tags are properly ignored
by the host.

That is why I think that this change is safe.

[toc] | [prev] | [next] | [standalone]

#1340808

On Tue, Feb 23, 2016 at 06:26:47PM +0300, Denis V. Lunev wrote:
> On 02/23/2016 06:10 PM, Michael S. Tsirkin wrote:
> >On Tue, Feb 16, 2016 at 06:50:52PM +0300, Denis V. Lunev wrote:
> >>From: Igor Redko <redkoi@virtuozzo.com>
> >>
> >>Add a new field, VIRTIO_BALLOON_S_AVAIL, to virtio_balloon memory
> >>statistics protocol, corresponding to 'Available' in /proc/meminfo.
> >>
> >>It indicates to the hypervisor how big the balloon can be inflated
> >>without pushing the guest system to swap.
> >>
> >>Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
> >>Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
> >>Signed-off-by: Denis V. Lunev <den@openvz.org>
> >>CC: Michael S. Tsirkin <mst@redhat.com>
> >>CC: Andrew Morton <akpm@linux-foundation.org>
> >Oops - I missed the fact that this affects host/guest ABI.
> >
> >Can you please submit ABI update proposal to virtio tc?
> >Spec patch would be even better.
> >
> >This is important to ensure there are no conflicts
> >with other features being developed in parallel.
> 
> hmmm
> 
> From my point of view ABI remains untouched.

Anything exposed by guest to host is ABI.
Once we add stuff there, we never can remove it
as some host might rely on it.

> The guest can send any amount of <tag>;<value>
> pairs and unknown tags are properly ignored
> by the host.
> 
> That is why I think that this change is safe.

What happens if someone uses the tag you
used for VIRTIO_BALLOON_S_AVAIL, for some
other purpose?
Any tools using VIRTIO_BALLOON_S_AVAIL will be confused.

Really, it's not hard to get a tag number from virtio TC,
so please just do this.

-- 
MST

[toc] | [prev] | [next] | [standalone]

#1340835

On 02/23/2016 06:53 PM, Michael S. Tsirkin wrote:
> On Tue, Feb 23, 2016 at 06:26:47PM +0300, Denis V. Lunev wrote:
>> On 02/23/2016 06:10 PM, Michael S. Tsirkin wrote:
>>> On Tue, Feb 16, 2016 at 06:50:52PM +0300, Denis V. Lunev wrote:
>>>> From: Igor Redko <redkoi@virtuozzo.com>
>>>>
>>>> Add a new field, VIRTIO_BALLOON_S_AVAIL, to virtio_balloon memory
>>>> statistics protocol, corresponding to 'Available' in /proc/meminfo.
>>>>
>>>> It indicates to the hypervisor how big the balloon can be inflated
>>>> without pushing the guest system to swap.
>>>>
>>>> Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
>>>> Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>>> CC: Michael S. Tsirkin <mst@redhat.com>
>>>> CC: Andrew Morton <akpm@linux-foundation.org>
>>> Oops - I missed the fact that this affects host/guest ABI.
>>>
>>> Can you please submit ABI update proposal to virtio tc?
>>> Spec patch would be even better.
>>>
>>> This is important to ensure there are no conflicts
>>> with other features being developed in parallel.
>> hmmm
>>
>>  From my point of view ABI remains untouched.
> Anything exposed by guest to host is ABI.
> Once we add stuff there, we never can remove it
> as some host might rely on it.
>
>> The guest can send any amount of <tag>;<value>
>> pairs and unknown tags are properly ignored
>> by the host.
>>
>> That is why I think that this change is safe.
> What happens if someone uses the tag you
> used for VIRTIO_BALLOON_S_AVAIL, for some
> other purpose?
> Any tools using VIRTIO_BALLOON_S_AVAIL will be confused.
actually this constant resides in QEMU only,
values are reported above using JSON and
string tags.

> Really, it's not hard to get a tag number from virtio TC,
> so please just do this.
>
ok. So do you propose to negotiate maximum allowed
tag to send at the driver start time?

we will have to guard this exchange with proper flag
in feature space then. This could be done but from my
point of view this looks like serious over-complication.
Do we have somebody who can judge?

Den

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web