All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/2] balloon: add a feature bit to let Guest OS deflate virtio_balloon on OOM
@ 2014-11-27 11:45 Denis V. Lunev
  2014-11-27 11:45 ` [Qemu-devel] [PATCH 1/2] balloon: call qdev_alias_all_properties for proxy dev in balloon class init Denis V. Lunev
  2014-11-27 11:45 ` [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom Denis V. Lunev
  0 siblings, 2 replies; 26+ messages in thread
From: Denis V. Lunev @ 2014-11-27 11:45 UTC (permalink / raw)
  Cc: Denis V. Lunev, Michael S. Tsirkin, qemu-devel,
	Raushaniya Maksudova, Anthony Liguori

Excessive virtio_balloon inflation can cause invocation of OOM-killer,
when Linux is under severe memory pressure. Various mechanisms are
responsible for correct virtio_balloon memory management. Nevertheless it
is often the case that these control tools does not have enough time to
react on fast changing memory load. As a result OS runs out of memory and
invokes OOM-killer. The balancing of memory by use of the virtio balloon
should not cause the termination of processes while there are pages in the
balloon. Now there is no way for virtio balloon driver to free memory at
the last moment before some process get killed by OOM-killer.

This does not provide a security breach as balloon itself is running
inside Guest OS and is working in the cooperation with the host. Thus
some improvements from Guest side should be considered as normal.

To solve the problem, introduce a virtio_balloon callback which is
expected to be called from the oom notifier call chain in out_of_memory()
function. If virtio balloon could release some memory, it will make the
system to return and retry the allocation that forced the out of memory
killer to run.

This behavior should be enabled if and only if appropriate feature bit
is set on the device. It is off by default.

This functionality was recently merged into vanilla Linux (actually in
linux-next at the moment)

  commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
  Author: Raushaniya Maksudova <rmaksudova@parallels.com>
  Date:   Mon Nov 10 09:36:29 2014 +1030

This patch adds respective control bits into QEMU. It introduces
deflate-on-oom option for baloon device which do the trick.

Changes from v1:
- From: in patch 1 according to the original ownership
- feature processing in patch 2 as suggested by Michael. It could be done
  without additional field, but this will require to move the property
  level up, i.e. to PCI & CCW level.

Signed-off-by: Raushaniya Maksudova <rmaksudova@parallels.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Anthony Liguori <aliguori@amazon.com>
CC: Michael S. Tsirkin <mst@redhat.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH 1/2] balloon: call qdev_alias_all_properties for proxy dev in balloon class init
  2014-11-27 11:45 [Qemu-devel] [PATCH v2 0/2] balloon: add a feature bit to let Guest OS deflate virtio_balloon on OOM Denis V. Lunev
@ 2014-11-27 11:45 ` Denis V. Lunev
  2014-11-27 11:45 ` [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom Denis V. Lunev
  1 sibling, 0 replies; 26+ messages in thread
From: Denis V. Lunev @ 2014-11-27 11:45 UTC (permalink / raw)
  Cc: Raushaniya Maksudova, Michael S. Tsirkin, qemu-devel,
	Christian Borntraeger, Anthony Liguori, Denis V. Lunev

The idea is that all other virtio devices are calling this helper
to merge properties of the proxy device. This is the only difference
in between this helper and code in inside virtio_instance_init_common.
The patch should not cause any harm as property list in generic balloon
code is empty.

This also allows to avoid some dummy errors like fixed by this
    commit 91ba21208839643603e7f7fa5864723c3f371ebe
    Author: Gonglei <arei.gonglei@huawei.com>
    Date:   Tue Sep 30 14:10:35 2014 +0800
    virtio-balloon: fix virtio-balloon child refcount in transports

Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Raushaniya Maksudova <rmaksudova@parallels.com>
Revieved-by: Cornelia Huck <cornelia.huck@de.ibm.com>
CC: Christian Borntraeger <borntraeger@de.ibm.com>
CC: Anthony Liguori <aliguori@amazon.com>
CC: Michael S. Tsirkin <mst@redhat.com>
---
 hw/s390x/virtio-ccw.c  | 5 ++---
 hw/virtio/virtio-pci.c | 5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index ea236c9..82da894 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -899,9 +899,8 @@ static void balloon_ccw_stats_set_poll_interval(Object *obj, struct Visitor *v,
 static void virtio_ccw_balloon_instance_init(Object *obj)
 {
     VirtIOBalloonCcw *dev = VIRTIO_BALLOON_CCW(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_BALLOON);
     object_property_add(obj, "guest-stats", "guest statistics",
                         balloon_ccw_stats_get_all, NULL, NULL, dev, NULL);
 
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index dde1d73..745324b 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1316,9 +1316,8 @@ static void virtio_balloon_pci_class_init(ObjectClass *klass, void *data)
 static void virtio_balloon_pci_instance_init(Object *obj)
 {
     VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_BALLOON);
     object_property_add(obj, "guest-stats", "guest statistics",
                         balloon_pci_stats_get_all, NULL, NULL, dev,
                         NULL);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-27 11:45 [Qemu-devel] [PATCH v2 0/2] balloon: add a feature bit to let Guest OS deflate virtio_balloon on OOM Denis V. Lunev
  2014-11-27 11:45 ` [Qemu-devel] [PATCH 1/2] balloon: call qdev_alias_all_properties for proxy dev in balloon class init Denis V. Lunev
@ 2014-11-27 11:45 ` Denis V. Lunev
  2014-11-27 11:50   ` Andrey Korolyov
  2014-11-27 12:31   ` Michael S. Tsirkin
  1 sibling, 2 replies; 26+ messages in thread
From: Denis V. Lunev @ 2014-11-27 11:45 UTC (permalink / raw)
  Cc: Denis V. Lunev, Michael S. Tsirkin, qemu-devel,
	Raushaniya Maksudova, Anthony Liguori

Excessive virtio_balloon inflation can cause invocation of OOM-killer,
when Linux is under severe memory pressure. Various mechanisms are
responsible for correct virtio_balloon memory management. Nevertheless it
is often the case that these control tools does not have enough time to
react on fast changing memory load. As a result OS runs out of memory and
invokes OOM-killer. The balancing of memory by use of the virtio balloon
should not cause the termination of processes while there are pages in the
balloon. Now there is no way for virtio balloon driver to free memory at
the last moment before some process get killed by OOM-killer.

This does not provide a security breach as balloon itself is running
inside Guest OS and is working in the cooperation with the host. Thus
some improvements from Guest side should be considered as normal.

To solve the problem, introduce a virtio_balloon callback which is
expected to be called from the oom notifier call chain in out_of_memory()
function. If virtio balloon could release some memory, it will make the
system to return and retry the allocation that forced the out of memory
killer to run.

This behavior should be enabled if and only if appropriate feature bit
is set on the device. It is off by default.

This functionality was recently merged into vanilla Linux (actually in
linux-next at the moment)

  commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
  Author: Raushaniya Maksudova <rmaksudova@parallels.com>
  Date:   Mon Nov 10 09:36:29 2014 +1030

This patch adds respective control bits into QEMU. It introduces
deflate-on-oom option for baloon device which do the trick.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Raushaniya Maksudova <rmaksudova@parallels.com>
CC: Anthony Liguori <aliguori@amazon.com>
CC: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-balloon.c         | 6 ++++--
 include/hw/virtio/virtio-balloon.h | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 7bfbb75..4d043ce 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -305,8 +305,8 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
 
 static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
 {
-    f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
-    return f;
+    VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
+    return (f | VIRTIO_BALLOON_F_STATS_VQ) | dev->host_features;
 }
 
 static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
@@ -409,6 +409,8 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
 }
 
 static Property virtio_balloon_properties[] = {
+    DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
+                    VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index f863bfe..2e1ccd9 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -30,6 +30,7 @@
 /* The feature bitmap for virtio balloon */
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
 #define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
+#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
@@ -67,6 +68,7 @@ typedef struct VirtIOBalloon {
     QEMUTimer *stats_timer;
     int64_t stats_last_update;
     int64_t stats_poll_interval;
+    uint32_t host_features;
 } VirtIOBalloon;
 
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-27 11:45 ` [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom Denis V. Lunev
@ 2014-11-27 11:50   ` Andrey Korolyov
  2014-11-27 12:26     ` Denis V. Lunev
  2014-11-27 12:28     ` Michael S. Tsirkin
  2014-11-27 12:31   ` Michael S. Tsirkin
  1 sibling, 2 replies; 26+ messages in thread
From: Andrey Korolyov @ 2014-11-27 11:50 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: Anthony Liguori, qemu-devel, Raushaniya Maksudova, Michael S. Tsirkin

On Thu, Nov 27, 2014 at 2:45 PM, Denis V. Lunev <den@openvz.org> wrote:
> Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> when Linux is under severe memory pressure. Various mechanisms are
> responsible for correct virtio_balloon memory management. Nevertheless it
> is often the case that these control tools does not have enough time to
> react on fast changing memory load. As a result OS runs out of memory and
> invokes OOM-killer. The balancing of memory by use of the virtio balloon
> should not cause the termination of processes while there are pages in the
> balloon. Now there is no way for virtio balloon driver to free memory at
> the last moment before some process get killed by OOM-killer.
>
> This does not provide a security breach as balloon itself is running
> inside Guest OS and is working in the cooperation with the host. Thus
> some improvements from Guest side should be considered as normal.
>
> To solve the problem, introduce a virtio_balloon callback which is
> expected to be called from the oom notifier call chain in out_of_memory()
> function. If virtio balloon could release some memory, it will make the
> system to return and retry the allocation that forced the out of memory
> killer to run.
>
> This behavior should be enabled if and only if appropriate feature bit
> is set on the device. It is off by default.
>
> This functionality was recently merged into vanilla Linux (actually in
> linux-next at the moment)
>
>   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
>   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
>   Date:   Mon Nov 10 09:36:29 2014 +1030
>
> This patch adds respective control bits into QEMU. It introduces
> deflate-on-oom option for baloon device which do the trick.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Raushaniya Maksudova <rmaksudova@parallels.com>
> CC: Anthony Liguori <aliguori@amazon.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/virtio/virtio-balloon.c         | 6 ++++--
>  include/hw/virtio/virtio-balloon.h | 2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 7bfbb75..4d043ce 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -305,8 +305,8 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
>
>  static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
>  {
> -    f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
> -    return f;
> +    VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
> +    return (f | VIRTIO_BALLOON_F_STATS_VQ) | dev->host_features;
>  }
>
>  static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
> @@ -409,6 +409,8 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
>  }
>
>  static Property virtio_balloon_properties[] = {
> +    DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
> +                    VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> index f863bfe..2e1ccd9 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -30,6 +30,7 @@
>  /* The feature bitmap for virtio balloon */
>  #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
>  #define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
> +#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
>
>  /* Size of a PFN in the balloon interface. */
>  #define VIRTIO_BALLOON_PFN_SHIFT 12
> @@ -67,6 +68,7 @@ typedef struct VirtIOBalloon {
>      QEMUTimer *stats_timer;
>      int64_t stats_last_update;
>      int64_t stats_poll_interval;
> +    uint32_t host_features;
>  } VirtIOBalloon;
>
>  #endif
> --
> 1.9.1
>
>

Had you tried this with a system-wide OOM on a real workload? This
behavior can work perfectly with dedicated memory cgroups, but I`m
afraid it would be unusable when entire system stalls and waits for a
balloon deflation.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-27 11:50   ` Andrey Korolyov
@ 2014-11-27 12:26     ` Denis V. Lunev
  2014-11-27 12:28     ` Michael S. Tsirkin
  1 sibling, 0 replies; 26+ messages in thread
From: Denis V. Lunev @ 2014-11-27 12:26 UTC (permalink / raw)
  To: Andrey Korolyov, Denis V. Lunev
  Cc: Raushaniya Maksudova, qemu-devel, Anthony Liguori, Michael S. Tsirkin

On 27/11/14 14:50, Andrey Korolyov wrote:
> On Thu, Nov 27, 2014 at 2:45 PM, Denis V. Lunev <den@openvz.org> wrote:
>> Excessive virtio_balloon inflation can cause invocation of OOM-killer,
>> when Linux is under severe memory pressure. Various mechanisms are
>> responsible for correct virtio_balloon memory management. Nevertheless it
>> is often the case that these control tools does not have enough time to
>> react on fast changing memory load. As a result OS runs out of memory and
>> invokes OOM-killer. The balancing of memory by use of the virtio balloon
>> should not cause the termination of processes while there are pages in the
>> balloon. Now there is no way for virtio balloon driver to free memory at
>> the last moment before some process get killed by OOM-killer.
>>
>> This does not provide a security breach as balloon itself is running
>> inside Guest OS and is working in the cooperation with the host. Thus
>> some improvements from Guest side should be considered as normal.
>>
>> To solve the problem, introduce a virtio_balloon callback which is
>> expected to be called from the oom notifier call chain in out_of_memory()
>> function. If virtio balloon could release some memory, it will make the
>> system to return and retry the allocation that forced the out of memory
>> killer to run.
>>
>> This behavior should be enabled if and only if appropriate feature bit
>> is set on the device. It is off by default.
>>
>> This functionality was recently merged into vanilla Linux (actually in
>> linux-next at the moment)
>>
>>    commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
>>    Author: Raushaniya Maksudova <rmaksudova@parallels.com>
>>    Date:   Mon Nov 10 09:36:29 2014 +1030
>>
>> This patch adds respective control bits into QEMU. It introduces
>> deflate-on-oom option for baloon device which do the trick.
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Raushaniya Maksudova <rmaksudova@parallels.com>
>> CC: Anthony Liguori <aliguori@amazon.com>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>   hw/virtio/virtio-balloon.c         | 6 ++++--
>>   include/hw/virtio/virtio-balloon.h | 2 ++
>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index 7bfbb75..4d043ce 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -305,8 +305,8 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
>>
>>   static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
>>   {
>> -    f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
>> -    return f;
>> +    VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
>> +    return (f | VIRTIO_BALLOON_F_STATS_VQ) | dev->host_features;
>>   }
>>
>>   static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
>> @@ -409,6 +409,8 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
>>   }
>>
>>   static Property virtio_balloon_properties[] = {
>> +    DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
>> +                    VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
>>       DEFINE_PROP_END_OF_LIST(),
>>   };
>>
>> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
>> index f863bfe..2e1ccd9 100644
>> --- a/include/hw/virtio/virtio-balloon.h
>> +++ b/include/hw/virtio/virtio-balloon.h
>> @@ -30,6 +30,7 @@
>>   /* The feature bitmap for virtio balloon */
>>   #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
>>   #define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
>> +#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
>>
>>   /* Size of a PFN in the balloon interface. */
>>   #define VIRTIO_BALLOON_PFN_SHIFT 12
>> @@ -67,6 +68,7 @@ typedef struct VirtIOBalloon {
>>       QEMUTimer *stats_timer;
>>       int64_t stats_last_update;
>>       int64_t stats_poll_interval;
>> +    uint32_t host_features;
>>   } VirtIOBalloon;
>>
>>   #endif
>> --
>> 1.9.1
>>
>>
>
> Had you tried this with a system-wide OOM on a real workload? This
> behavior can work perfectly with dedicated memory cgroups, but I`m
> afraid it would be unusable when entire system stalls and waits for a
> balloon deflation.
>

we have tried this with test workloads only at the moment.
I think that this is a matter of setup. Yes, this setup probably
will result in host OOM. But host system has quite a lot of options
to toss host memory (including VMs memory) and the system will
survive longer. Host cgroup is also a good idea but in this
case (most probably) you will have entire qemu killed.

We could think on this in the following terms: OOM is guest
is equivalent to OOM in host from the point of critical
service interaction. Most likely guest OOM will the fattest
eater in guest which is the most critical one and this will
not be seen by host at all. If entire QEMU will be killed,
the VM could be restarted by the fault tolerance system
and even this restart could happen on the different node.
These are just simple speculations...

Anyway, this behavior is quite native from the point of guest
and is off by default.

I do not see much problem with it. Though this ability with a
proper guest-to-host feedback seems promising from the
management point of view.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-27 11:50   ` Andrey Korolyov
  2014-11-27 12:26     ` Denis V. Lunev
@ 2014-11-27 12:28     ` Michael S. Tsirkin
  2014-11-27 14:00       ` Andrey Korolyov
  1 sibling, 1 reply; 26+ messages in thread
From: Michael S. Tsirkin @ 2014-11-27 12:28 UTC (permalink / raw)
  To: Andrey Korolyov
  Cc: Denis V. Lunev, qemu-devel, Raushaniya Maksudova, Anthony Liguori

On Thu, Nov 27, 2014 at 03:50:11PM +0400, Andrey Korolyov wrote:
> On Thu, Nov 27, 2014 at 2:45 PM, Denis V. Lunev <den@openvz.org> wrote:
> > Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> > when Linux is under severe memory pressure. Various mechanisms are
> > responsible for correct virtio_balloon memory management. Nevertheless it
> > is often the case that these control tools does not have enough time to
> > react on fast changing memory load. As a result OS runs out of memory and
> > invokes OOM-killer. The balancing of memory by use of the virtio balloon
> > should not cause the termination of processes while there are pages in the
> > balloon. Now there is no way for virtio balloon driver to free memory at
> > the last moment before some process get killed by OOM-killer.
> >
> > This does not provide a security breach as balloon itself is running
> > inside Guest OS and is working in the cooperation with the host. Thus
> > some improvements from Guest side should be considered as normal.
> >
> > To solve the problem, introduce a virtio_balloon callback which is
> > expected to be called from the oom notifier call chain in out_of_memory()
> > function. If virtio balloon could release some memory, it will make the
> > system to return and retry the allocation that forced the out of memory
> > killer to run.
> >
> > This behavior should be enabled if and only if appropriate feature bit
> > is set on the device. It is off by default.
> >
> > This functionality was recently merged into vanilla Linux (actually in
> > linux-next at the moment)
> >
> >   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
> >   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
> >   Date:   Mon Nov 10 09:36:29 2014 +1030
> >
> > This patch adds respective control bits into QEMU. It introduces
> > deflate-on-oom option for baloon device which do the trick.
> >
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > CC: Raushaniya Maksudova <rmaksudova@parallels.com>
> > CC: Anthony Liguori <aliguori@amazon.com>
> > CC: Michael S. Tsirkin <mst@redhat.com>

...

> Had you tried this with a system-wide OOM on a real workload? This
> behavior can work perfectly with dedicated memory cgroups, but I`m
> afraid it would be unusable when entire system stalls and waits for a
> balloon deflation.

That's really a question about guest drivers though, isn't it?
So you aren't responding to correct patches, and aren't copying
the correct people.

-- 
MST

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-27 11:45 ` [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom Denis V. Lunev
  2014-11-27 11:50   ` Andrey Korolyov
@ 2014-11-27 12:31   ` Michael S. Tsirkin
  2014-11-27 12:34     ` Denis V. Lunev
  1 sibling, 1 reply; 26+ messages in thread
From: Michael S. Tsirkin @ 2014-11-27 12:31 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: qemu-devel, Raushaniya Maksudova, Anthony Liguori

On Thu, Nov 27, 2014 at 02:45:42PM +0300, Denis V. Lunev wrote:
> Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> when Linux is under severe memory pressure. Various mechanisms are
> responsible for correct virtio_balloon memory management. Nevertheless it
> is often the case that these control tools does not have enough time to
> react on fast changing memory load. As a result OS runs out of memory and
> invokes OOM-killer. The balancing of memory by use of the virtio balloon
> should not cause the termination of processes while there are pages in the
> balloon. Now there is no way for virtio balloon driver to free memory at
> the last moment before some process get killed by OOM-killer.
> 
> This does not provide a security breach as balloon itself is running
> inside Guest OS and is working in the cooperation with the host. Thus
> some improvements from Guest side should be considered as normal.
> 
> To solve the problem, introduce a virtio_balloon callback which is
> expected to be called from the oom notifier call chain in out_of_memory()
> function. If virtio balloon could release some memory, it will make the
> system to return and retry the allocation that forced the out of memory
> killer to run.
> 
> This behavior should be enabled if and only if appropriate feature bit
> is set on the device. It is off by default.
> 
> This functionality was recently merged into vanilla Linux (actually in
> linux-next at the moment)
> 
>   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
>   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
>   Date:   Mon Nov 10 09:36:29 2014 +1030
> 
> This patch adds respective control bits into QEMU. It introduces
> deflate-on-oom option for baloon device which do the trick.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Raushaniya Maksudova <rmaksudova@parallels.com>
> CC: Anthony Liguori <aliguori@amazon.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/virtio/virtio-balloon.c         | 6 ++++--
>  include/hw/virtio/virtio-balloon.h | 2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 7bfbb75..4d043ce 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -305,8 +305,8 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
>  
>  static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
>  {
> -    f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
> -    return f;
> +    VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
> +    return (f | VIRTIO_BALLOON_F_STATS_VQ) | dev->host_features;

This looks very strange.
You use | on the bit number?

Also, no need for () above.

>  }
>  
>  static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
> @@ -409,6 +409,8 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
>  }
>  
>  static Property virtio_balloon_properties[] = {
> +    DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
> +                    VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> index f863bfe..2e1ccd9 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -30,6 +30,7 @@
>  /* The feature bitmap for virtio balloon */
>  #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
>  #define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
> +#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
>  
>  /* Size of a PFN in the balloon interface. */
>  #define VIRTIO_BALLOON_PFN_SHIFT 12
> @@ -67,6 +68,7 @@ typedef struct VirtIOBalloon {
>      QEMUTimer *stats_timer;
>      int64_t stats_last_update;
>      int64_t stats_poll_interval;
> +    uint32_t host_features;
>  } VirtIOBalloon;
>  
>  #endif
> -- 
> 1.9.1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-27 12:31   ` Michael S. Tsirkin
@ 2014-11-27 12:34     ` Denis V. Lunev
  0 siblings, 0 replies; 26+ messages in thread
From: Denis V. Lunev @ 2014-11-27 12:34 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, Raushaniya Maksudova, Anthony Liguori

On 27/11/14 15:31, Michael S. Tsirkin wrote:
> On Thu, Nov 27, 2014 at 02:45:42PM +0300, Denis V. Lunev wrote:
>> Excessive virtio_balloon inflation can cause invocation of OOM-killer,
>> when Linux is under severe memory pressure. Various mechanisms are
>> responsible for correct virtio_balloon memory management. Nevertheless it
>> is often the case that these control tools does not have enough time to
>> react on fast changing memory load. As a result OS runs out of memory and
>> invokes OOM-killer. The balancing of memory by use of the virtio balloon
>> should not cause the termination of processes while there are pages in the
>> balloon. Now there is no way for virtio balloon driver to free memory at
>> the last moment before some process get killed by OOM-killer.
>>
>> This does not provide a security breach as balloon itself is running
>> inside Guest OS and is working in the cooperation with the host. Thus
>> some improvements from Guest side should be considered as normal.
>>
>> To solve the problem, introduce a virtio_balloon callback which is
>> expected to be called from the oom notifier call chain in out_of_memory()
>> function. If virtio balloon could release some memory, it will make the
>> system to return and retry the allocation that forced the out of memory
>> killer to run.
>>
>> This behavior should be enabled if and only if appropriate feature bit
>> is set on the device. It is off by default.
>>
>> This functionality was recently merged into vanilla Linux (actually in
>> linux-next at the moment)
>>
>>    commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
>>    Author: Raushaniya Maksudova <rmaksudova@parallels.com>
>>    Date:   Mon Nov 10 09:36:29 2014 +1030
>>
>> This patch adds respective control bits into QEMU. It introduces
>> deflate-on-oom option for baloon device which do the trick.
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Raushaniya Maksudova <rmaksudova@parallels.com>
>> CC: Anthony Liguori <aliguori@amazon.com>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>   hw/virtio/virtio-balloon.c         | 6 ++++--
>>   include/hw/virtio/virtio-balloon.h | 2 ++
>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index 7bfbb75..4d043ce 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -305,8 +305,8 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
>>   
>>   static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
>>   {
>> -    f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
>> -    return f;
>> +    VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
>> +    return (f | VIRTIO_BALLOON_F_STATS_VQ) | dev->host_features;
> This looks very strange.
> You use | on the bit number?
>
> Also, no need for () above.
uuu, my bad :(

sure you are right...


>>   }
>>   
>>   static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
>> @@ -409,6 +409,8 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
>>   }
>>   
>>   static Property virtio_balloon_properties[] = {
>> +    DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
>> +                    VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
>>       DEFINE_PROP_END_OF_LIST(),
>>   };
>>   
>> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
>> index f863bfe..2e1ccd9 100644
>> --- a/include/hw/virtio/virtio-balloon.h
>> +++ b/include/hw/virtio/virtio-balloon.h
>> @@ -30,6 +30,7 @@
>>   /* The feature bitmap for virtio balloon */
>>   #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
>>   #define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
>> +#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
>>   
>>   /* Size of a PFN in the balloon interface. */
>>   #define VIRTIO_BALLOON_PFN_SHIFT 12
>> @@ -67,6 +68,7 @@ typedef struct VirtIOBalloon {
>>       QEMUTimer *stats_timer;
>>       int64_t stats_last_update;
>>       int64_t stats_poll_interval;
>> +    uint32_t host_features;
>>   } VirtIOBalloon;
>>   
>>   #endif
>> -- 
>> 1.9.1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-27 12:28     ` Michael S. Tsirkin
@ 2014-11-27 14:00       ` Andrey Korolyov
  2014-11-27 21:49         ` Michael S. Tsirkin
  0 siblings, 1 reply; 26+ messages in thread
From: Andrey Korolyov @ 2014-11-27 14:00 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Denis V. Lunev, qemu-devel, Raushaniya Maksudova, Anthony Liguori

On Thu, Nov 27, 2014 at 3:28 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Nov 27, 2014 at 03:50:11PM +0400, Andrey Korolyov wrote:
>> On Thu, Nov 27, 2014 at 2:45 PM, Denis V. Lunev <den@openvz.org> wrote:
>> > Excessive virtio_balloon inflation can cause invocation of OOM-killer,
>> > when Linux is under severe memory pressure. Various mechanisms are
>> > responsible for correct virtio_balloon memory management. Nevertheless it
>> > is often the case that these control tools does not have enough time to
>> > react on fast changing memory load. As a result OS runs out of memory and
>> > invokes OOM-killer. The balancing of memory by use of the virtio balloon
>> > should not cause the termination of processes while there are pages in the
>> > balloon. Now there is no way for virtio balloon driver to free memory at
>> > the last moment before some process get killed by OOM-killer.
>> >
>> > This does not provide a security breach as balloon itself is running
>> > inside Guest OS and is working in the cooperation with the host. Thus
>> > some improvements from Guest side should be considered as normal.
>> >
>> > To solve the problem, introduce a virtio_balloon callback which is
>> > expected to be called from the oom notifier call chain in out_of_memory()
>> > function. If virtio balloon could release some memory, it will make the
>> > system to return and retry the allocation that forced the out of memory
>> > killer to run.
>> >
>> > This behavior should be enabled if and only if appropriate feature bit
>> > is set on the device. It is off by default.
>> >
>> > This functionality was recently merged into vanilla Linux (actually in
>> > linux-next at the moment)
>> >
>> >   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
>> >   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
>> >   Date:   Mon Nov 10 09:36:29 2014 +1030
>> >
>> > This patch adds respective control bits into QEMU. It introduces
>> > deflate-on-oom option for baloon device which do the trick.
>> >
>> > Signed-off-by: Denis V. Lunev <den@openvz.org>
>> > CC: Raushaniya Maksudova <rmaksudova@parallels.com>
>> > CC: Anthony Liguori <aliguori@amazon.com>
>> > CC: Michael S. Tsirkin <mst@redhat.com>
>
> ...
>
>> Had you tried this with a system-wide OOM on a real workload? This
>> behavior can work perfectly with dedicated memory cgroups, but I`m
>> afraid it would be unusable when entire system stalls and waits for a
>> balloon deflation.
>
> That's really a question about guest drivers though, isn't it?
> So you aren't responding to correct patches, and aren't copying
> the correct people.
>
> --
> MST

Not entirely, it is a question about host-guest interaction in such a
case. If we will wait for a balloon deflation while OOM condition
exists at the 'root' cg controller level, for a certain settings it
may probably lead to the host unresponsiveness. As for OOM event in a
dedicated cgroup with strictly defined set of processes inside, it
should way more safe. In other words, even such kind of guest-host
interaction can be considered as a potential threat for a host
security, as return from a try of balloon defiation may take too much
time and some other host processes can be stuck effectively. I am
using delayed OOM loop via userspace application, reaching simular
goals, but it is using dedicated cgroups explicitly. Please correct me
if I am wrong in my suggestions.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-27 14:00       ` Andrey Korolyov
@ 2014-11-27 21:49         ` Michael S. Tsirkin
  2014-11-27 22:13           ` Andrey Korolyov
  0 siblings, 1 reply; 26+ messages in thread
From: Michael S. Tsirkin @ 2014-11-27 21:49 UTC (permalink / raw)
  To: Andrey Korolyov; +Cc: Denis V. Lunev, qemu-devel, Raushaniya Maksudova

On Thu, Nov 27, 2014 at 06:00:36PM +0400, Andrey Korolyov wrote:
> On Thu, Nov 27, 2014 at 3:28 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Thu, Nov 27, 2014 at 03:50:11PM +0400, Andrey Korolyov wrote:
> >> On Thu, Nov 27, 2014 at 2:45 PM, Denis V. Lunev <den@openvz.org> wrote:
> >> > Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> >> > when Linux is under severe memory pressure. Various mechanisms are
> >> > responsible for correct virtio_balloon memory management. Nevertheless it
> >> > is often the case that these control tools does not have enough time to
> >> > react on fast changing memory load. As a result OS runs out of memory and
> >> > invokes OOM-killer. The balancing of memory by use of the virtio balloon
> >> > should not cause the termination of processes while there are pages in the
> >> > balloon. Now there is no way for virtio balloon driver to free memory at
> >> > the last moment before some process get killed by OOM-killer.
> >> >
> >> > This does not provide a security breach as balloon itself is running
> >> > inside Guest OS and is working in the cooperation with the host. Thus
> >> > some improvements from Guest side should be considered as normal.
> >> >
> >> > To solve the problem, introduce a virtio_balloon callback which is
> >> > expected to be called from the oom notifier call chain in out_of_memory()
> >> > function. If virtio balloon could release some memory, it will make the
> >> > system to return and retry the allocation that forced the out of memory
> >> > killer to run.
> >> >
> >> > This behavior should be enabled if and only if appropriate feature bit
> >> > is set on the device. It is off by default.
> >> >
> >> > This functionality was recently merged into vanilla Linux (actually in
> >> > linux-next at the moment)
> >> >
> >> >   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
> >> >   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
> >> >   Date:   Mon Nov 10 09:36:29 2014 +1030
> >> >
> >> > This patch adds respective control bits into QEMU. It introduces
> >> > deflate-on-oom option for baloon device which do the trick.
> >> >
> >> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> >> > CC: Raushaniya Maksudova <rmaksudova@parallels.com>
> >> > CC: Anthony Liguori <aliguori@amazon.com>
> >> > CC: Michael S. Tsirkin <mst@redhat.com>
> >
> > ...
> >
> >> Had you tried this with a system-wide OOM on a real workload? This
> >> behavior can work perfectly with dedicated memory cgroups, but I`m
> >> afraid it would be unusable when entire system stalls and waits for a
> >> balloon deflation.
> >
> > That's really a question about guest drivers though, isn't it?
> > So you aren't responding to correct patches, and aren't copying
> > the correct people.
> >
> > --
> > MST
> 
> Not entirely, it is a question about host-guest interaction in such a
> case. If we will wait for a balloon deflation while OOM condition
> exists at the 'root' cg controller level, for a certain settings it
> may probably lead to the host unresponsiveness. As for OOM event in a
> dedicated cgroup with strictly defined set of processes inside, it
> should way more safe. In other words, even such kind of guest-host
> interaction can be considered as a potential threat for a host
> security, as return from a try of balloon defiation may take too much
> time and some other host processes can be stuck effectively. I am
> using delayed OOM loop via userspace application, reaching simular
> goals, but it is using dedicated cgroups explicitly. Please correct me
> if I am wrong in my suggestions.

ATM balloon is cooperative anyway:
If guest deflating balloon leads to host OOM, you
have misconfigured your host, or you have trusted
guests.

We could change this: unmap pages from guest memory on
inflate, map them back on inflate.



-- 
MST

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-27 21:49         ` Michael S. Tsirkin
@ 2014-11-27 22:13           ` Andrey Korolyov
  0 siblings, 0 replies; 26+ messages in thread
From: Andrey Korolyov @ 2014-11-27 22:13 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Denis V. Lunev, qemu-devel, Raushaniya Maksudova

On Fri, Nov 28, 2014 at 12:49 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Nov 27, 2014 at 06:00:36PM +0400, Andrey Korolyov wrote:
>> On Thu, Nov 27, 2014 at 3:28 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Thu, Nov 27, 2014 at 03:50:11PM +0400, Andrey Korolyov wrote:
>> >> On Thu, Nov 27, 2014 at 2:45 PM, Denis V. Lunev <den@openvz.org> wrote:
>> >> > Excessive virtio_balloon inflation can cause invocation of OOM-killer,
>> >> > when Linux is under severe memory pressure. Various mechanisms are
>> >> > responsible for correct virtio_balloon memory management. Nevertheless it
>> >> > is often the case that these control tools does not have enough time to
>> >> > react on fast changing memory load. As a result OS runs out of memory and
>> >> > invokes OOM-killer. The balancing of memory by use of the virtio balloon
>> >> > should not cause the termination of processes while there are pages in the
>> >> > balloon. Now there is no way for virtio balloon driver to free memory at
>> >> > the last moment before some process get killed by OOM-killer.
>> >> >
>> >> > This does not provide a security breach as balloon itself is running
>> >> > inside Guest OS and is working in the cooperation with the host. Thus
>> >> > some improvements from Guest side should be considered as normal.
>> >> >
>> >> > To solve the problem, introduce a virtio_balloon callback which is
>> >> > expected to be called from the oom notifier call chain in out_of_memory()
>> >> > function. If virtio balloon could release some memory, it will make the
>> >> > system to return and retry the allocation that forced the out of memory
>> >> > killer to run.
>> >> >
>> >> > This behavior should be enabled if and only if appropriate feature bit
>> >> > is set on the device. It is off by default.
>> >> >
>> >> > This functionality was recently merged into vanilla Linux (actually in
>> >> > linux-next at the moment)
>> >> >
>> >> >   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
>> >> >   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
>> >> >   Date:   Mon Nov 10 09:36:29 2014 +1030
>> >> >
>> >> > This patch adds respective control bits into QEMU. It introduces
>> >> > deflate-on-oom option for baloon device which do the trick.
>> >> >
>> >> > Signed-off-by: Denis V. Lunev <den@openvz.org>
>> >> > CC: Raushaniya Maksudova <rmaksudova@parallels.com>
>> >> > CC: Anthony Liguori <aliguori@amazon.com>
>> >> > CC: Michael S. Tsirkin <mst@redhat.com>
>> >
>> > ...
>> >
>> >> Had you tried this with a system-wide OOM on a real workload? This
>> >> behavior can work perfectly with dedicated memory cgroups, but I`m
>> >> afraid it would be unusable when entire system stalls and waits for a
>> >> balloon deflation.
>> >
>> > That's really a question about guest drivers though, isn't it?
>> > So you aren't responding to correct patches, and aren't copying
>> > the correct people.
>> >
>> > --
>> > MST
>>
>> Not entirely, it is a question about host-guest interaction in such a
>> case. If we will wait for a balloon deflation while OOM condition
>> exists at the 'root' cg controller level, for a certain settings it
>> may probably lead to the host unresponsiveness. As for OOM event in a
>> dedicated cgroup with strictly defined set of processes inside, it
>> should way more safe. In other words, even such kind of guest-host
>> interaction can be considered as a potential threat for a host
>> security, as return from a try of balloon defiation may take too much
>> time and some other host processes can be stuck effectively. I am
>> using delayed OOM loop via userspace application, reaching simular
>> goals, but it is using dedicated cgroups explicitly. Please correct me
>> if I am wrong in my suggestions.
>
> ATM balloon is cooperative anyway:
> If guest deflating balloon leads to host OOM, you
> have misconfigured your host, or you have trusted
> guests.
>
> We could change this: unmap pages from guest memory on
> inflate, map them back on inflate.
>
>

// sorry for bad grammar in a previous message, was distracted at a time

Yes, exactly, I meant just a regular (probably untrusted) guest in a
previous message, which can either behave badly or its driver may not
respond timely (for this case I have zero knowledge on how delay
increase of the return from OOM handler will affect hypervisor, if no
separate control groups are set and memory pressure is high enough,
but I do not expect anything good).

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2015-06-08 14:54             ` James Bottomley
@ 2015-06-08 15:24               ` Michael S. Tsirkin
  0 siblings, 0 replies; 26+ messages in thread
From: Michael S. Tsirkin @ 2015-06-08 15:24 UTC (permalink / raw)
  To: James Bottomley
  Cc: Denis V. Lunev, qemu-devel, Raushaniya Maksudova, Anthony Liguori

On Mon, Jun 08, 2015 at 07:54:42AM -0700, James Bottomley wrote:
> On Mon, 2015-05-04 at 12:47 +0300, Denis V. Lunev wrote:
> > On 01/04/15 13:18, Michael S. Tsirkin wrote:
> > > On Wed, Apr 01, 2015 at 12:51:42PM +0300, James Bottomley wrote:
> > >> On Wed, 2015-04-01 at 11:50 +0200, Michael S. Tsirkin wrote:
> > >>> On Wed, Apr 01, 2015 at 12:44:28PM +0300, James Bottomley wrote:
> > >>>> On Fri, 2015-02-27 at 09:57 +0300, Denis V. Lunev wrote:
> > >>>>> Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> > >>>>> when Linux is under severe memory pressure. Various mechanisms are
> > >>>>> responsible for correct virtio_balloon memory management. Nevertheless it
> > >>>>> is often the case that these control tools does not have enough time to
> > >>>>> react on fast changing memory load. As a result OS runs out of memory and
> > >>>>> invokes OOM-killer. The balancing of memory by use of the virtio balloon
> > >>>>> should not cause the termination of processes while there are pages in the
> > >>>>> balloon. Now there is no way for virtio balloon driver to free memory at
> > >>>>> the last moment before some process get killed by OOM-killer.
> > >>>>>
> > >>>>> This does not provide a security breach as balloon itself is running
> > >>>>> inside Guest OS and is working in the cooperation with the host. Thus
> > >>>>> some improvements from Guest side should be considered as normal.
> > >>>>>
> > >>>>> To solve the problem, introduce a virtio_balloon callback which is
> > >>>>> expected to be called from the oom notifier call chain in out_of_memory()
> > >>>>> function. If virtio balloon could release some memory, it will make the
> > >>>>> system return and retry the allocation that forced the out of memory
> > >>>>> killer to run.
> > >>>>>
> > >>>>> This behavior should be enabled if and only if appropriate feature bit
> > >>>>> is set on the device. It is off by default.
> > >>>>>
> > >>>>> This functionality was recently merged into vanilla Linux.
> > >>>>>
> > >>>>>    commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
> > >>>>>    Author: Raushaniya Maksudova <rmaksudova@parallels.com>
> > >>>>>    Date:   Mon Nov 10 09:36:29 2014 +1030
> > >>>>>
> > >>>>> This patch adds respective control bits into QEMU. It introduces
> > >>>>> deflate-on-oom option for balloon device which does the trick.
> > >>>> What's the status on this, please?  It's been over a month since this
> > >>>> was posted with no further review feedback, so I think it's ready.
> > >>>> Getting this into qemu is blocking our next step which would be adding
> > >>>> the feature bit to the virtio spec.
> > >>>>
> > >>>> James
> > >>> This was posted after soft feature freeze for 2.3, so it'll have to go
> > >>> into 2.4.  I don't see why would this block your work on the spec: you
> > >>> should make progress on this meanwhile.
> > >> I can do that ... I just thought the spec was trailing edge, so I was
> > >> waiting to have the patch accepted, which confirms the implementation.
> > >> I didn't want to write it into the spec and have the actual
> > >> implementation changed by review later.
> > >>
> > >> James
> > >>
> > > It's up to you really, I would just like to point out two things:
> > > - spec process is a long one, assuming we accept a spec change,
> > >    we go though a public review period, multiple votes etc.
> > >    About half a year to release a spec revision with
> > >    new features.
> > >    So time enough to make minor changes.
> > > - oasis process works like this (roughly):
> > > 	spec is written
> > > 	spec goes through a public review process
> > > 	community standard is published
> > > 	3 implementations are reported
> > > 	spec becomes an oasis standard
> > >    so implementations aren't required at early stages
> > 2.3 is done, 2.4 window is opened....
> > 
> > The patch is applicable for both
> > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git
> > and vanilla qemu.
> > 
> > How can we proceed?
> 
> The spec update supporting this feature is published for review:
> 
> https://www.oasis-open.org/committees/download.php/55709/virtio-v1.0-csprd04.zip
> 
> It's probably a good idea to have the implementation there as well.  Do
> we need to resend these patches?
> 
> James
> 

Yes, please do.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2015-05-04  9:47           ` Denis V. Lunev
@ 2015-06-08 14:54             ` James Bottomley
  2015-06-08 15:24               ` Michael S. Tsirkin
  0 siblings, 1 reply; 26+ messages in thread
From: James Bottomley @ 2015-06-08 14:54 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: Anthony Liguori, qemu-devel, Raushaniya Maksudova, Michael S. Tsirkin

On Mon, 2015-05-04 at 12:47 +0300, Denis V. Lunev wrote:
> On 01/04/15 13:18, Michael S. Tsirkin wrote:
> > On Wed, Apr 01, 2015 at 12:51:42PM +0300, James Bottomley wrote:
> >> On Wed, 2015-04-01 at 11:50 +0200, Michael S. Tsirkin wrote:
> >>> On Wed, Apr 01, 2015 at 12:44:28PM +0300, James Bottomley wrote:
> >>>> On Fri, 2015-02-27 at 09:57 +0300, Denis V. Lunev wrote:
> >>>>> Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> >>>>> when Linux is under severe memory pressure. Various mechanisms are
> >>>>> responsible for correct virtio_balloon memory management. Nevertheless it
> >>>>> is often the case that these control tools does not have enough time to
> >>>>> react on fast changing memory load. As a result OS runs out of memory and
> >>>>> invokes OOM-killer. The balancing of memory by use of the virtio balloon
> >>>>> should not cause the termination of processes while there are pages in the
> >>>>> balloon. Now there is no way for virtio balloon driver to free memory at
> >>>>> the last moment before some process get killed by OOM-killer.
> >>>>>
> >>>>> This does not provide a security breach as balloon itself is running
> >>>>> inside Guest OS and is working in the cooperation with the host. Thus
> >>>>> some improvements from Guest side should be considered as normal.
> >>>>>
> >>>>> To solve the problem, introduce a virtio_balloon callback which is
> >>>>> expected to be called from the oom notifier call chain in out_of_memory()
> >>>>> function. If virtio balloon could release some memory, it will make the
> >>>>> system return and retry the allocation that forced the out of memory
> >>>>> killer to run.
> >>>>>
> >>>>> This behavior should be enabled if and only if appropriate feature bit
> >>>>> is set on the device. It is off by default.
> >>>>>
> >>>>> This functionality was recently merged into vanilla Linux.
> >>>>>
> >>>>>    commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
> >>>>>    Author: Raushaniya Maksudova <rmaksudova@parallels.com>
> >>>>>    Date:   Mon Nov 10 09:36:29 2014 +1030
> >>>>>
> >>>>> This patch adds respective control bits into QEMU. It introduces
> >>>>> deflate-on-oom option for balloon device which does the trick.
> >>>> What's the status on this, please?  It's been over a month since this
> >>>> was posted with no further review feedback, so I think it's ready.
> >>>> Getting this into qemu is blocking our next step which would be adding
> >>>> the feature bit to the virtio spec.
> >>>>
> >>>> James
> >>> This was posted after soft feature freeze for 2.3, so it'll have to go
> >>> into 2.4.  I don't see why would this block your work on the spec: you
> >>> should make progress on this meanwhile.
> >> I can do that ... I just thought the spec was trailing edge, so I was
> >> waiting to have the patch accepted, which confirms the implementation.
> >> I didn't want to write it into the spec and have the actual
> >> implementation changed by review later.
> >>
> >> James
> >>
> > It's up to you really, I would just like to point out two things:
> > - spec process is a long one, assuming we accept a spec change,
> >    we go though a public review period, multiple votes etc.
> >    About half a year to release a spec revision with
> >    new features.
> >    So time enough to make minor changes.
> > - oasis process works like this (roughly):
> > 	spec is written
> > 	spec goes through a public review process
> > 	community standard is published
> > 	3 implementations are reported
> > 	spec becomes an oasis standard
> >    so implementations aren't required at early stages
> 2.3 is done, 2.4 window is opened....
> 
> The patch is applicable for both
> git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git
> and vanilla qemu.
> 
> How can we proceed?

The spec update supporting this feature is published for review:

https://www.oasis-open.org/committees/download.php/55709/virtio-v1.0-csprd04.zip

It's probably a good idea to have the implementation there as well.  Do
we need to resend these patches?

James

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2015-04-01 10:18         ` Michael S. Tsirkin
@ 2015-05-04  9:47           ` Denis V. Lunev
  2015-06-08 14:54             ` James Bottomley
  0 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2015-05-04  9:47 UTC (permalink / raw)
  To: Michael S. Tsirkin, James Bottomley
  Cc: qemu-devel, Raushaniya Maksudova, Anthony Liguori

On 01/04/15 13:18, Michael S. Tsirkin wrote:
> On Wed, Apr 01, 2015 at 12:51:42PM +0300, James Bottomley wrote:
>> On Wed, 2015-04-01 at 11:50 +0200, Michael S. Tsirkin wrote:
>>> On Wed, Apr 01, 2015 at 12:44:28PM +0300, James Bottomley wrote:
>>>> On Fri, 2015-02-27 at 09:57 +0300, Denis V. Lunev wrote:
>>>>> Excessive virtio_balloon inflation can cause invocation of OOM-killer,
>>>>> when Linux is under severe memory pressure. Various mechanisms are
>>>>> responsible for correct virtio_balloon memory management. Nevertheless it
>>>>> is often the case that these control tools does not have enough time to
>>>>> react on fast changing memory load. As a result OS runs out of memory and
>>>>> invokes OOM-killer. The balancing of memory by use of the virtio balloon
>>>>> should not cause the termination of processes while there are pages in the
>>>>> balloon. Now there is no way for virtio balloon driver to free memory at
>>>>> the last moment before some process get killed by OOM-killer.
>>>>>
>>>>> This does not provide a security breach as balloon itself is running
>>>>> inside Guest OS and is working in the cooperation with the host. Thus
>>>>> some improvements from Guest side should be considered as normal.
>>>>>
>>>>> To solve the problem, introduce a virtio_balloon callback which is
>>>>> expected to be called from the oom notifier call chain in out_of_memory()
>>>>> function. If virtio balloon could release some memory, it will make the
>>>>> system return and retry the allocation that forced the out of memory
>>>>> killer to run.
>>>>>
>>>>> This behavior should be enabled if and only if appropriate feature bit
>>>>> is set on the device. It is off by default.
>>>>>
>>>>> This functionality was recently merged into vanilla Linux.
>>>>>
>>>>>    commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
>>>>>    Author: Raushaniya Maksudova <rmaksudova@parallels.com>
>>>>>    Date:   Mon Nov 10 09:36:29 2014 +1030
>>>>>
>>>>> This patch adds respective control bits into QEMU. It introduces
>>>>> deflate-on-oom option for balloon device which does the trick.
>>>> What's the status on this, please?  It's been over a month since this
>>>> was posted with no further review feedback, so I think it's ready.
>>>> Getting this into qemu is blocking our next step which would be adding
>>>> the feature bit to the virtio spec.
>>>>
>>>> James
>>> This was posted after soft feature freeze for 2.3, so it'll have to go
>>> into 2.4.  I don't see why would this block your work on the spec: you
>>> should make progress on this meanwhile.
>> I can do that ... I just thought the spec was trailing edge, so I was
>> waiting to have the patch accepted, which confirms the implementation.
>> I didn't want to write it into the spec and have the actual
>> implementation changed by review later.
>>
>> James
>>
> It's up to you really, I would just like to point out two things:
> - spec process is a long one, assuming we accept a spec change,
>    we go though a public review period, multiple votes etc.
>    About half a year to release a spec revision with
>    new features.
>    So time enough to make minor changes.
> - oasis process works like this (roughly):
> 	spec is written
> 	spec goes through a public review process
> 	community standard is published
> 	3 implementations are reported
> 	spec becomes an oasis standard
>    so implementations aren't required at early stages
2.3 is done, 2.4 window is opened....

The patch is applicable for both
git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git
and vanilla qemu.

How can we proceed?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2015-04-01  9:51       ` James Bottomley
@ 2015-04-01 10:18         ` Michael S. Tsirkin
  2015-05-04  9:47           ` Denis V. Lunev
  0 siblings, 1 reply; 26+ messages in thread
From: Michael S. Tsirkin @ 2015-04-01 10:18 UTC (permalink / raw)
  To: James Bottomley
  Cc: Denis V. Lunev, qemu-devel, Raushaniya Maksudova, Anthony Liguori

On Wed, Apr 01, 2015 at 12:51:42PM +0300, James Bottomley wrote:
> On Wed, 2015-04-01 at 11:50 +0200, Michael S. Tsirkin wrote:
> > On Wed, Apr 01, 2015 at 12:44:28PM +0300, James Bottomley wrote:
> > > On Fri, 2015-02-27 at 09:57 +0300, Denis V. Lunev wrote:
> > > > Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> > > > when Linux is under severe memory pressure. Various mechanisms are
> > > > responsible for correct virtio_balloon memory management. Nevertheless it
> > > > is often the case that these control tools does not have enough time to
> > > > react on fast changing memory load. As a result OS runs out of memory and
> > > > invokes OOM-killer. The balancing of memory by use of the virtio balloon
> > > > should not cause the termination of processes while there are pages in the
> > > > balloon. Now there is no way for virtio balloon driver to free memory at
> > > > the last moment before some process get killed by OOM-killer.
> > > > 
> > > > This does not provide a security breach as balloon itself is running
> > > > inside Guest OS and is working in the cooperation with the host. Thus
> > > > some improvements from Guest side should be considered as normal.
> > > > 
> > > > To solve the problem, introduce a virtio_balloon callback which is
> > > > expected to be called from the oom notifier call chain in out_of_memory()
> > > > function. If virtio balloon could release some memory, it will make the
> > > > system return and retry the allocation that forced the out of memory
> > > > killer to run.
> > > > 
> > > > This behavior should be enabled if and only if appropriate feature bit
> > > > is set on the device. It is off by default.
> > > > 
> > > > This functionality was recently merged into vanilla Linux.
> > > > 
> > > >   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
> > > >   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
> > > >   Date:   Mon Nov 10 09:36:29 2014 +1030
> > > > 
> > > > This patch adds respective control bits into QEMU. It introduces
> > > > deflate-on-oom option for balloon device which does the trick.
> > > 
> > > What's the status on this, please?  It's been over a month since this
> > > was posted with no further review feedback, so I think it's ready.
> > > Getting this into qemu is blocking our next step which would be adding
> > > the feature bit to the virtio spec.
> > > 
> > > James
> > 
> > This was posted after soft feature freeze for 2.3, so it'll have to go
> > into 2.4.  I don't see why would this block your work on the spec: you
> > should make progress on this meanwhile.
> 
> I can do that ... I just thought the spec was trailing edge, so I was
> waiting to have the patch accepted, which confirms the implementation.
> I didn't want to write it into the spec and have the actual
> implementation changed by review later.
> 
> James
> 

It's up to you really, I would just like to point out two things:
- spec process is a long one, assuming we accept a spec change,
  we go though a public review period, multiple votes etc.
  About half a year to release a spec revision with
  new features.
  So time enough to make minor changes.
- oasis process works like this (roughly):
	spec is written
	spec goes through a public review process
	community standard is published
	3 implementations are reported
	spec becomes an oasis standard
  so implementations aren't required at early stages
-- 
MST

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2015-04-01  9:50     ` Michael S. Tsirkin
@ 2015-04-01  9:51       ` James Bottomley
  2015-04-01 10:18         ` Michael S. Tsirkin
  0 siblings, 1 reply; 26+ messages in thread
From: James Bottomley @ 2015-04-01  9:51 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Denis V. Lunev, qemu-devel, Raushaniya Maksudova, Anthony Liguori

On Wed, 2015-04-01 at 11:50 +0200, Michael S. Tsirkin wrote:
> On Wed, Apr 01, 2015 at 12:44:28PM +0300, James Bottomley wrote:
> > On Fri, 2015-02-27 at 09:57 +0300, Denis V. Lunev wrote:
> > > Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> > > when Linux is under severe memory pressure. Various mechanisms are
> > > responsible for correct virtio_balloon memory management. Nevertheless it
> > > is often the case that these control tools does not have enough time to
> > > react on fast changing memory load. As a result OS runs out of memory and
> > > invokes OOM-killer. The balancing of memory by use of the virtio balloon
> > > should not cause the termination of processes while there are pages in the
> > > balloon. Now there is no way for virtio balloon driver to free memory at
> > > the last moment before some process get killed by OOM-killer.
> > > 
> > > This does not provide a security breach as balloon itself is running
> > > inside Guest OS and is working in the cooperation with the host. Thus
> > > some improvements from Guest side should be considered as normal.
> > > 
> > > To solve the problem, introduce a virtio_balloon callback which is
> > > expected to be called from the oom notifier call chain in out_of_memory()
> > > function. If virtio balloon could release some memory, it will make the
> > > system return and retry the allocation that forced the out of memory
> > > killer to run.
> > > 
> > > This behavior should be enabled if and only if appropriate feature bit
> > > is set on the device. It is off by default.
> > > 
> > > This functionality was recently merged into vanilla Linux.
> > > 
> > >   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
> > >   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
> > >   Date:   Mon Nov 10 09:36:29 2014 +1030
> > > 
> > > This patch adds respective control bits into QEMU. It introduces
> > > deflate-on-oom option for balloon device which does the trick.
> > 
> > What's the status on this, please?  It's been over a month since this
> > was posted with no further review feedback, so I think it's ready.
> > Getting this into qemu is blocking our next step which would be adding
> > the feature bit to the virtio spec.
> > 
> > James
> 
> This was posted after soft feature freeze for 2.3, so it'll have to go
> into 2.4.  I don't see why would this block your work on the spec: you
> should make progress on this meanwhile.

I can do that ... I just thought the spec was trailing edge, so I was
waiting to have the patch accepted, which confirms the implementation.
I didn't want to write it into the spec and have the actual
implementation changed by review later.

James

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2015-04-01  9:44   ` James Bottomley
@ 2015-04-01  9:50     ` Michael S. Tsirkin
  2015-04-01  9:51       ` James Bottomley
  0 siblings, 1 reply; 26+ messages in thread
From: Michael S. Tsirkin @ 2015-04-01  9:50 UTC (permalink / raw)
  To: James Bottomley
  Cc: Denis V. Lunev, qemu-devel, Raushaniya Maksudova, Anthony Liguori

On Wed, Apr 01, 2015 at 12:44:28PM +0300, James Bottomley wrote:
> On Fri, 2015-02-27 at 09:57 +0300, Denis V. Lunev wrote:
> > Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> > when Linux is under severe memory pressure. Various mechanisms are
> > responsible for correct virtio_balloon memory management. Nevertheless it
> > is often the case that these control tools does not have enough time to
> > react on fast changing memory load. As a result OS runs out of memory and
> > invokes OOM-killer. The balancing of memory by use of the virtio balloon
> > should not cause the termination of processes while there are pages in the
> > balloon. Now there is no way for virtio balloon driver to free memory at
> > the last moment before some process get killed by OOM-killer.
> > 
> > This does not provide a security breach as balloon itself is running
> > inside Guest OS and is working in the cooperation with the host. Thus
> > some improvements from Guest side should be considered as normal.
> > 
> > To solve the problem, introduce a virtio_balloon callback which is
> > expected to be called from the oom notifier call chain in out_of_memory()
> > function. If virtio balloon could release some memory, it will make the
> > system return and retry the allocation that forced the out of memory
> > killer to run.
> > 
> > This behavior should be enabled if and only if appropriate feature bit
> > is set on the device. It is off by default.
> > 
> > This functionality was recently merged into vanilla Linux.
> > 
> >   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
> >   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
> >   Date:   Mon Nov 10 09:36:29 2014 +1030
> > 
> > This patch adds respective control bits into QEMU. It introduces
> > deflate-on-oom option for balloon device which does the trick.
> 
> What's the status on this, please?  It's been over a month since this
> was posted with no further review feedback, so I think it's ready.
> Getting this into qemu is blocking our next step which would be adding
> the feature bit to the virtio spec.
> 
> James

This was posted after soft feature freeze for 2.3, so it'll have to go
into 2.4.  I don't see why would this block your work on the spec: you
should make progress on this meanwhile.

-- 
MST

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2015-02-27  6:57 ` [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom Denis V. Lunev
@ 2015-04-01  9:44   ` James Bottomley
  2015-04-01  9:50     ` Michael S. Tsirkin
  0 siblings, 1 reply; 26+ messages in thread
From: James Bottomley @ 2015-04-01  9:44 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: Michael S. Tsirkin, qemu-devel, Raushaniya Maksudova, Anthony Liguori

On Fri, 2015-02-27 at 09:57 +0300, Denis V. Lunev wrote:
> Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> when Linux is under severe memory pressure. Various mechanisms are
> responsible for correct virtio_balloon memory management. Nevertheless it
> is often the case that these control tools does not have enough time to
> react on fast changing memory load. As a result OS runs out of memory and
> invokes OOM-killer. The balancing of memory by use of the virtio balloon
> should not cause the termination of processes while there are pages in the
> balloon. Now there is no way for virtio balloon driver to free memory at
> the last moment before some process get killed by OOM-killer.
> 
> This does not provide a security breach as balloon itself is running
> inside Guest OS and is working in the cooperation with the host. Thus
> some improvements from Guest side should be considered as normal.
> 
> To solve the problem, introduce a virtio_balloon callback which is
> expected to be called from the oom notifier call chain in out_of_memory()
> function. If virtio balloon could release some memory, it will make the
> system return and retry the allocation that forced the out of memory
> killer to run.
> 
> This behavior should be enabled if and only if appropriate feature bit
> is set on the device. It is off by default.
> 
> This functionality was recently merged into vanilla Linux.
> 
>   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
>   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
>   Date:   Mon Nov 10 09:36:29 2014 +1030
> 
> This patch adds respective control bits into QEMU. It introduces
> deflate-on-oom option for balloon device which does the trick.

What's the status on this, please?  It's been over a month since this
was posted with no further review feedback, so I think it's ready.
Getting this into qemu is blocking our next step which would be adding
the feature bit to the virtio spec.

James

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2015-02-27  6:57 [Qemu-devel] [PATCH v5 0/2] balloon: add a feature bit to let Guest OS deflate Denis V. Lunev
@ 2015-02-27  6:57 ` Denis V. Lunev
  2015-04-01  9:44   ` James Bottomley
  0 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2015-02-27  6:57 UTC (permalink / raw)
  Cc: Denis V. Lunev, Michael S. Tsirkin, qemu-devel,
	Raushaniya Maksudova, Anthony Liguori

Excessive virtio_balloon inflation can cause invocation of OOM-killer,
when Linux is under severe memory pressure. Various mechanisms are
responsible for correct virtio_balloon memory management. Nevertheless it
is often the case that these control tools does not have enough time to
react on fast changing memory load. As a result OS runs out of memory and
invokes OOM-killer. The balancing of memory by use of the virtio balloon
should not cause the termination of processes while there are pages in the
balloon. Now there is no way for virtio balloon driver to free memory at
the last moment before some process get killed by OOM-killer.

This does not provide a security breach as balloon itself is running
inside Guest OS and is working in the cooperation with the host. Thus
some improvements from Guest side should be considered as normal.

To solve the problem, introduce a virtio_balloon callback which is
expected to be called from the oom notifier call chain in out_of_memory()
function. If virtio balloon could release some memory, it will make the
system return and retry the allocation that forced the out of memory
killer to run.

This behavior should be enabled if and only if appropriate feature bit
is set on the device. It is off by default.

This functionality was recently merged into vanilla Linux.

  commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
  Author: Raushaniya Maksudova <rmaksudova@parallels.com>
  Date:   Mon Nov 10 09:36:29 2014 +1030

This patch adds respective control bits into QEMU. It introduces
deflate-on-oom option for balloon device which does the trick.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Raushaniya Maksudova <rmaksudova@parallels.com>
CC: Anthony Liguori <aliguori@amazon.com>
CC: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-balloon.c         | 6 ++++--
 include/hw/virtio/virtio-balloon.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 21e449a..cbc5f7f 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -305,8 +305,8 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
 
 static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
 {
-    f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
-    return f;
+    VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
+    return f | (1u << VIRTIO_BALLOON_F_STATS_VQ) | dev->host_features;
 }
 
 static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
@@ -409,6 +409,8 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
 }
 
 static Property virtio_balloon_properties[] = {
+    DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
+                    VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index 4ab8f54..7f49b1f 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -36,6 +36,7 @@ typedef struct VirtIOBalloon {
     QEMUTimer *stats_timer;
     int64_t stats_last_update;
     int64_t stats_poll_interval;
+    uint32_t host_features;
 } VirtIOBalloon;
 
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2015-02-26 17:39 ` [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom Denis V. Lunev
@ 2015-02-26 20:39   ` Eric Blake
  0 siblings, 0 replies; 26+ messages in thread
From: Eric Blake @ 2015-02-26 20:39 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: Anthony Liguori, qemu-devel, Raushaniya Maksudova, Michael S. Tsirkin

[-- Attachment #1: Type: text/plain, Size: 1982 bytes --]

On 02/26/2015 10:39 AM, Denis V. Lunev wrote:
> Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> when Linux is under severe memory pressure. Various mechanisms are
> responsible for correct virtio_balloon memory management. Nevertheless it
> is often the case that these control tools does not have enough time to
> react on fast changing memory load. As a result OS runs out of memory and
> invokes OOM-killer. The balancing of memory by use of the virtio balloon
> should not cause the termination of processes while there are pages in the
> balloon. Now there is no way for virtio balloon driver to free memory at
> the last moment before some process get killed by OOM-killer.
> 
> This does not provide a security breach as balloon itself is running
> inside Guest OS and is working in the cooperation with the host. Thus
> some improvements from Guest side should be considered as normal.
> 
> To solve the problem, introduce a virtio_balloon callback which is
> expected to be called from the oom notifier call chain in out_of_memory()
> function. If virtio balloon could release some memory, it will make the
> system to return and retry the allocation that forced the out of memory

s/to return/return/

> killer to run.
> 
> This behavior should be enabled if and only if appropriate feature bit
> is set on the device. It is off by default.
> 
> This functionality was recently merged into vanilla Linux.
> 
>   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
>   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
>   Date:   Mon Nov 10 09:36:29 2014 +1030
> 
> This patch adds respective control bits into QEMU. It introduces
> deflate-on-oom option for baloon device which do the trick.

s/baloon/balloon/
s/do/does/

I'll leave the content review to someone more familiar with the code.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2015-02-26 17:39 [Qemu-devel] [PATCH v4 0/2] balloon: add a feature bit to let Guest OS deflate Denis V. Lunev
@ 2015-02-26 17:39 ` Denis V. Lunev
  2015-02-26 20:39   ` Eric Blake
  0 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2015-02-26 17:39 UTC (permalink / raw)
  Cc: Denis V. Lunev, Michael S. Tsirkin, qemu-devel,
	Raushaniya Maksudova, Anthony Liguori

Excessive virtio_balloon inflation can cause invocation of OOM-killer,
when Linux is under severe memory pressure. Various mechanisms are
responsible for correct virtio_balloon memory management. Nevertheless it
is often the case that these control tools does not have enough time to
react on fast changing memory load. As a result OS runs out of memory and
invokes OOM-killer. The balancing of memory by use of the virtio balloon
should not cause the termination of processes while there are pages in the
balloon. Now there is no way for virtio balloon driver to free memory at
the last moment before some process get killed by OOM-killer.

This does not provide a security breach as balloon itself is running
inside Guest OS and is working in the cooperation with the host. Thus
some improvements from Guest side should be considered as normal.

To solve the problem, introduce a virtio_balloon callback which is
expected to be called from the oom notifier call chain in out_of_memory()
function. If virtio balloon could release some memory, it will make the
system to return and retry the allocation that forced the out of memory
killer to run.

This behavior should be enabled if and only if appropriate feature bit
is set on the device. It is off by default.

This functionality was recently merged into vanilla Linux.

  commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
  Author: Raushaniya Maksudova <rmaksudova@parallels.com>
  Date:   Mon Nov 10 09:36:29 2014 +1030

This patch adds respective control bits into QEMU. It introduces
deflate-on-oom option for baloon device which do the trick.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Raushaniya Maksudova <rmaksudova@parallels.com>
CC: Anthony Liguori <aliguori@amazon.com>
CC: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-balloon.c         | 6 ++++--
 include/hw/virtio/virtio-balloon.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 21e449a..cbc5f7f 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -305,8 +305,8 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
 
 static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
 {
-    f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
-    return f;
+    VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
+    return f | (1u << VIRTIO_BALLOON_F_STATS_VQ) | dev->host_features;
 }
 
 static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
@@ -409,6 +409,8 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
 }
 
 static Property virtio_balloon_properties[] = {
+    DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
+                    VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index 4ab8f54..7f49b1f 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -36,6 +36,7 @@ typedef struct VirtIOBalloon {
     QEMUTimer *stats_timer;
     int64_t stats_last_update;
     int64_t stats_poll_interval;
+    uint32_t host_features;
 } VirtIOBalloon;
 
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-27 12:44 [Qemu-devel] [PATCH v3 0/2] balloon: add a feature bit to let Guest OS deflate virtio_balloon on OOM Denis V. Lunev
@ 2014-11-27 12:44 ` Denis V. Lunev
  0 siblings, 0 replies; 26+ messages in thread
From: Denis V. Lunev @ 2014-11-27 12:44 UTC (permalink / raw)
  Cc: Denis V. Lunev, Michael S. Tsirkin, qemu-devel,
	Raushaniya Maksudova, Anthony Liguori

Excessive virtio_balloon inflation can cause invocation of OOM-killer,
when Linux is under severe memory pressure. Various mechanisms are
responsible for correct virtio_balloon memory management. Nevertheless it
is often the case that these control tools does not have enough time to
react on fast changing memory load. As a result OS runs out of memory and
invokes OOM-killer. The balancing of memory by use of the virtio balloon
should not cause the termination of processes while there are pages in the
balloon. Now there is no way for virtio balloon driver to free memory at
the last moment before some process get killed by OOM-killer.

This does not provide a security breach as balloon itself is running
inside Guest OS and is working in the cooperation with the host. Thus
some improvements from Guest side should be considered as normal.

To solve the problem, introduce a virtio_balloon callback which is
expected to be called from the oom notifier call chain in out_of_memory()
function. If virtio balloon could release some memory, it will make the
system to return and retry the allocation that forced the out of memory
killer to run.

This behavior should be enabled if and only if appropriate feature bit
is set on the device. It is off by default.

This functionality was recently merged into vanilla Linux (actually in
linux-next at the moment)

  commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
  Author: Raushaniya Maksudova <rmaksudova@parallels.com>
  Date:   Mon Nov 10 09:36:29 2014 +1030

This patch adds respective control bits into QEMU. It introduces
deflate-on-oom option for baloon device which do the trick.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Raushaniya Maksudova <rmaksudova@parallels.com>
CC: Anthony Liguori <aliguori@amazon.com>
CC: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-balloon.c         | 6 ++++--
 include/hw/virtio/virtio-balloon.h | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 7bfbb75..a54d026 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -305,8 +305,8 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
 
 static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
 {
-    f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
-    return f;
+    VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
+    return f | (1u << VIRTIO_BALLOON_F_STATS_VQ) | dev->host_features;
 }
 
 static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
@@ -409,6 +409,8 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
 }
 
 static Property virtio_balloon_properties[] = {
+    DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
+                    VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index f863bfe..2e1ccd9 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -30,6 +30,7 @@
 /* The feature bitmap for virtio balloon */
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
 #define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
+#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
@@ -67,6 +68,7 @@ typedef struct VirtIOBalloon {
     QEMUTimer *stats_timer;
     int64_t stats_last_update;
     int64_t stats_poll_interval;
+    uint32_t host_features;
 } VirtIOBalloon;
 
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-27 11:04     ` Denis V. Lunev
@ 2014-11-27 11:08       ` Michael S. Tsirkin
  0 siblings, 0 replies; 26+ messages in thread
From: Michael S. Tsirkin @ 2014-11-27 11:08 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: qemu-devel, Raushaniya Maksudova, Anthony Liguori

On Thu, Nov 27, 2014 at 02:04:55PM +0300, Denis V. Lunev wrote:
> On 26/11/14 14:16, Michael S. Tsirkin wrote:
> >On Wed, Nov 26, 2014 at 01:11:25PM +0300, Denis V. Lunev wrote:
> >>From: Raushaniya Maksudova <rmaksudova@parallels.com>
> >>
> >>Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> >>when Linux is under severe memory pressure. Various mechanisms are
> >>responsible for correct virtio_balloon memory management. Nevertheless it
> >>is often the case that these control tools does not have enough time to
> >>react on fast changing memory load. As a result OS runs out of memory and
> >>invokes OOM-killer. The balancing of memory by use of the virtio balloon
> >>should not cause the termination of processes while there are pages in the
> >>balloon. Now there is no way for virtio balloon driver to free memory at
> >>the last moment before some process get killed by OOM-killer.
> >>
> >>This does not provide a security breach as balloon itself is running
> >>inside Guest OS and is working in the cooperation with the host. Thus
> >>some improvements from Guest side should be considered as normal.
> >>
> >>To solve the problem, introduce a virtio_balloon callback which is
> >>expected to be called from the oom notifier call chain in out_of_memory()
> >>function. If virtio balloon could release some memory, it will make the
> >>system to return and retry the allocation that forced the out of memory
> >>killer to run.
> >>
> >>This behavior should be enabled if and only if appropriate feature bit
> >>is set on the device. It is off by default.
> >>
> >>This functionality was recently merged into vanilla Linux (actually in
> >>linux-next at the moment)
> >>
> >>   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
> >>   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
> >>   Date:   Mon Nov 10 09:36:29 2014 +1030
> >>
> >>This patch adds respective control bits into QEMU. It introduces
> >>deflate-on-oom option for baloon device which do the trick.
> >>
> >>Signed-off-by: Raushaniya Maksudova <rmaksudova@parallels.com>
> >>Signed-off-by: Denis V. Lunev <den@openvz.org>
> >>CC: Anthony Liguori <aliguori@amazon.com>
> >>CC: Michael S. Tsirkin <mst@redhat.com>
> >>---
> >>  hw/virtio/virtio-balloon.c         | 7 +++++++
> >>  include/hw/virtio/virtio-balloon.h | 2 ++
> >>  qemu-options.hx                    | 6 +++++-
> >>  3 files changed, 14 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> >>index 7bfbb75..9d145fa 100644
> >>--- a/hw/virtio/virtio-balloon.c
> >>+++ b/hw/virtio/virtio-balloon.c
> >>@@ -305,7 +305,12 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
> >>  static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
> >>  {
> >>+    VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
> >>      f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
> >>+    if (dev->deflate_on_oom) {
> >>+        f |= (1 << VIRTIO_BALLOON_F_DEFLATE_ON_OOM);
> >>+    }
> >>+
> >>      return f;
> >>  }
> >>@@ -409,6 +414,7 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
> >>  }
> >>  static Property virtio_balloon_properties[] = {
> >>+    DEFINE_PROP_BOOL("deflate-on-oom", VirtIOBalloon, deflate_on_oom, false),
> >>      DEFINE_PROP_END_OF_LIST(),
> >>  };
> >>diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> >>index f863bfe..45cc55a 100644
> >>--- a/include/hw/virtio/virtio-balloon.h
> >>+++ b/include/hw/virtio/virtio-balloon.h
> >>@@ -30,6 +30,7 @@
> >>  /* The feature bitmap for virtio balloon */
> >>  #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
> >>  #define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
> >>+#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
> >>  /* Size of a PFN in the balloon interface. */
> >>  #define VIRTIO_BALLOON_PFN_SHIFT 12
> >>@@ -67,6 +68,7 @@ typedef struct VirtIOBalloon {
> >>      QEMUTimer *stats_timer;
> >>      int64_t stats_last_update;
> >>      int64_t stats_poll_interval;
> >>+    bool deflate_on_oom;
> >>  } VirtIOBalloon;
> >>  #endif
> >You don't need an extra bool, and open-coding.
> >Do it same as we do for other features please,
> >set bit in feature mask directly.
> there is no host_features placeholder at this level.
> 
> We could add it and propagate proper bit in get_features
> callback like
>    return f | dev->host_features
> or we have to move this stuff a little bit up into
> CCW & PCI code like done in virtscsi with
>   #define DEFINE_VIRTIO_SCSI_FEATURES(_state, _feature_field)
> 
> The first approach keeps bits in the same place,
> the second follows current approach. If you prefer unification way,
> I think that other devices could be re-written this way.
> 
> Any opinion?

Balloon is the weird one out here.
Look at how feature bits are defined for e.g.
DEFINE_PROP_BIT("any_layout", _state, _field, VIRTIO_F_ANY_LAYOUT, true),

you should do something similar.


> >>diff --git a/qemu-options.hx b/qemu-options.hx
> >>index da9851d..14ede0b 100644
> >>--- a/qemu-options.hx
> >>+++ b/qemu-options.hx
> >>@@ -324,7 +324,8 @@ ETEXI
> >>  DEF("balloon", HAS_ARG, QEMU_OPTION_balloon,
> >>      "-balloon none   disable balloon device\n"
> >>      "-balloon virtio[,addr=str]\n"
> >>-    "                enable virtio balloon device (default)\n", QEMU_ARCH_ALL)
> >>+    "                enable virtio balloon device (default)\n"
> >>+    "               [,deflate-on-oom=on|off]\n", QEMU_ARCH_ALL)
> >>  STEXI
> >>  @item -balloon none
> >>  @findex -balloon
> >>@@ -332,6 +333,9 @@ Disable balloon device.
> >>  @item -balloon virtio[,addr=@var{addr}]
> >>  Enable virtio balloon device (default), optionally with PCI address
> >>  @var{addr}.
> >>+@item -balloon virtio[,deflate-on-oom=@var{deflate-on-oom}]
> >>+@var{deflate-on-oom} is "on" or "off" and enables whether to let Guest OS
> >>+to deflate virtio balloon on OOM. Default is off.
> >>  ETEXI
> >>  DEF("device", HAS_ARG, QEMU_OPTION_device,
> >Please don't add stuff to legacy -balloon.
> >New -device is enough, you don't need to touch qemu-options.hx
> >for it.
> >
> sure

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-26 11:16   ` Michael S. Tsirkin
@ 2014-11-27 11:04     ` Denis V. Lunev
  2014-11-27 11:08       ` Michael S. Tsirkin
  0 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2014-11-27 11:04 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, Raushaniya Maksudova, Anthony Liguori

On 26/11/14 14:16, Michael S. Tsirkin wrote:
> On Wed, Nov 26, 2014 at 01:11:25PM +0300, Denis V. Lunev wrote:
>> From: Raushaniya Maksudova <rmaksudova@parallels.com>
>>
>> Excessive virtio_balloon inflation can cause invocation of OOM-killer,
>> when Linux is under severe memory pressure. Various mechanisms are
>> responsible for correct virtio_balloon memory management. Nevertheless it
>> is often the case that these control tools does not have enough time to
>> react on fast changing memory load. As a result OS runs out of memory and
>> invokes OOM-killer. The balancing of memory by use of the virtio balloon
>> should not cause the termination of processes while there are pages in the
>> balloon. Now there is no way for virtio balloon driver to free memory at
>> the last moment before some process get killed by OOM-killer.
>>
>> This does not provide a security breach as balloon itself is running
>> inside Guest OS and is working in the cooperation with the host. Thus
>> some improvements from Guest side should be considered as normal.
>>
>> To solve the problem, introduce a virtio_balloon callback which is
>> expected to be called from the oom notifier call chain in out_of_memory()
>> function. If virtio balloon could release some memory, it will make the
>> system to return and retry the allocation that forced the out of memory
>> killer to run.
>>
>> This behavior should be enabled if and only if appropriate feature bit
>> is set on the device. It is off by default.
>>
>> This functionality was recently merged into vanilla Linux (actually in
>> linux-next at the moment)
>>
>>    commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
>>    Author: Raushaniya Maksudova <rmaksudova@parallels.com>
>>    Date:   Mon Nov 10 09:36:29 2014 +1030
>>
>> This patch adds respective control bits into QEMU. It introduces
>> deflate-on-oom option for baloon device which do the trick.
>>
>> Signed-off-by: Raushaniya Maksudova <rmaksudova@parallels.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Anthony Liguori <aliguori@amazon.com>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>   hw/virtio/virtio-balloon.c         | 7 +++++++
>>   include/hw/virtio/virtio-balloon.h | 2 ++
>>   qemu-options.hx                    | 6 +++++-
>>   3 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index 7bfbb75..9d145fa 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -305,7 +305,12 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
>>   
>>   static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
>>   {
>> +    VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
>>       f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
>> +    if (dev->deflate_on_oom) {
>> +        f |= (1 << VIRTIO_BALLOON_F_DEFLATE_ON_OOM);
>> +    }
>> +
>>       return f;
>>   }
>>   
>> @@ -409,6 +414,7 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
>>   }
>>   
>>   static Property virtio_balloon_properties[] = {
>> +    DEFINE_PROP_BOOL("deflate-on-oom", VirtIOBalloon, deflate_on_oom, false),
>>       DEFINE_PROP_END_OF_LIST(),
>>   };
>>   
>> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
>> index f863bfe..45cc55a 100644
>> --- a/include/hw/virtio/virtio-balloon.h
>> +++ b/include/hw/virtio/virtio-balloon.h
>> @@ -30,6 +30,7 @@
>>   /* The feature bitmap for virtio balloon */
>>   #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
>>   #define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
>> +#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
>>   
>>   /* Size of a PFN in the balloon interface. */
>>   #define VIRTIO_BALLOON_PFN_SHIFT 12
>> @@ -67,6 +68,7 @@ typedef struct VirtIOBalloon {
>>       QEMUTimer *stats_timer;
>>       int64_t stats_last_update;
>>       int64_t stats_poll_interval;
>> +    bool deflate_on_oom;
>>   } VirtIOBalloon;
>>   
>>   #endif
> You don't need an extra bool, and open-coding.
> Do it same as we do for other features please,
> set bit in feature mask directly.
there is no host_features placeholder at this level.

We could add it and propagate proper bit in get_features
callback like
    return f | dev->host_features
or we have to move this stuff a little bit up into
CCW & PCI code like done in virtscsi with
   #define DEFINE_VIRTIO_SCSI_FEATURES(_state, _feature_field)

The first approach keeps bits in the same place,
the second follows current approach. If you prefer unification way,
I think that other devices could be re-written this way.

Any opinion?

>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index da9851d..14ede0b 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -324,7 +324,8 @@ ETEXI
>>   DEF("balloon", HAS_ARG, QEMU_OPTION_balloon,
>>       "-balloon none   disable balloon device\n"
>>       "-balloon virtio[,addr=str]\n"
>> -    "                enable virtio balloon device (default)\n", QEMU_ARCH_ALL)
>> +    "                enable virtio balloon device (default)\n"
>> +    "               [,deflate-on-oom=on|off]\n", QEMU_ARCH_ALL)
>>   STEXI
>>   @item -balloon none
>>   @findex -balloon
>> @@ -332,6 +333,9 @@ Disable balloon device.
>>   @item -balloon virtio[,addr=@var{addr}]
>>   Enable virtio balloon device (default), optionally with PCI address
>>   @var{addr}.
>> +@item -balloon virtio[,deflate-on-oom=@var{deflate-on-oom}]
>> +@var{deflate-on-oom} is "on" or "off" and enables whether to let Guest OS
>> +to deflate virtio balloon on OOM. Default is off.
>>   ETEXI
>>   
>>   DEF("device", HAS_ARG, QEMU_OPTION_device,
> Please don't add stuff to legacy -balloon.
> New -device is enough, you don't need to touch qemu-options.hx
> for it.
>
sure

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-26 10:11 ` [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom Denis V. Lunev
@ 2014-11-26 11:16   ` Michael S. Tsirkin
  2014-11-27 11:04     ` Denis V. Lunev
  0 siblings, 1 reply; 26+ messages in thread
From: Michael S. Tsirkin @ 2014-11-26 11:16 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: qemu-devel, Raushaniya Maksudova, Anthony Liguori

On Wed, Nov 26, 2014 at 01:11:25PM +0300, Denis V. Lunev wrote:
> From: Raushaniya Maksudova <rmaksudova@parallels.com>
> 
> Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> when Linux is under severe memory pressure. Various mechanisms are
> responsible for correct virtio_balloon memory management. Nevertheless it
> is often the case that these control tools does not have enough time to
> react on fast changing memory load. As a result OS runs out of memory and
> invokes OOM-killer. The balancing of memory by use of the virtio balloon
> should not cause the termination of processes while there are pages in the
> balloon. Now there is no way for virtio balloon driver to free memory at
> the last moment before some process get killed by OOM-killer.
> 
> This does not provide a security breach as balloon itself is running
> inside Guest OS and is working in the cooperation with the host. Thus
> some improvements from Guest side should be considered as normal.
> 
> To solve the problem, introduce a virtio_balloon callback which is
> expected to be called from the oom notifier call chain in out_of_memory()
> function. If virtio balloon could release some memory, it will make the
> system to return and retry the allocation that forced the out of memory
> killer to run.
> 
> This behavior should be enabled if and only if appropriate feature bit
> is set on the device. It is off by default.
> 
> This functionality was recently merged into vanilla Linux (actually in
> linux-next at the moment)
> 
>   commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
>   Author: Raushaniya Maksudova <rmaksudova@parallels.com>
>   Date:   Mon Nov 10 09:36:29 2014 +1030
> 
> This patch adds respective control bits into QEMU. It introduces
> deflate-on-oom option for baloon device which do the trick.
> 
> Signed-off-by: Raushaniya Maksudova <rmaksudova@parallels.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Anthony Liguori <aliguori@amazon.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/virtio/virtio-balloon.c         | 7 +++++++
>  include/hw/virtio/virtio-balloon.h | 2 ++
>  qemu-options.hx                    | 6 +++++-
>  3 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 7bfbb75..9d145fa 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -305,7 +305,12 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
>  
>  static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
>  {
> +    VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
>      f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
> +    if (dev->deflate_on_oom) {
> +        f |= (1 << VIRTIO_BALLOON_F_DEFLATE_ON_OOM);
> +    }
> +
>      return f;
>  }
>  
> @@ -409,6 +414,7 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
>  }
>  
>  static Property virtio_balloon_properties[] = {
> +    DEFINE_PROP_BOOL("deflate-on-oom", VirtIOBalloon, deflate_on_oom, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> index f863bfe..45cc55a 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -30,6 +30,7 @@
>  /* The feature bitmap for virtio balloon */
>  #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
>  #define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
> +#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
>  
>  /* Size of a PFN in the balloon interface. */
>  #define VIRTIO_BALLOON_PFN_SHIFT 12
> @@ -67,6 +68,7 @@ typedef struct VirtIOBalloon {
>      QEMUTimer *stats_timer;
>      int64_t stats_last_update;
>      int64_t stats_poll_interval;
> +    bool deflate_on_oom;
>  } VirtIOBalloon;
>  
>  #endif

You don't need an extra bool, and open-coding.
Do it same as we do for other features please,
set bit in feature mask directly.

> diff --git a/qemu-options.hx b/qemu-options.hx
> index da9851d..14ede0b 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -324,7 +324,8 @@ ETEXI
>  DEF("balloon", HAS_ARG, QEMU_OPTION_balloon,
>      "-balloon none   disable balloon device\n"
>      "-balloon virtio[,addr=str]\n"
> -    "                enable virtio balloon device (default)\n", QEMU_ARCH_ALL)
> +    "                enable virtio balloon device (default)\n"
> +    "               [,deflate-on-oom=on|off]\n", QEMU_ARCH_ALL)
>  STEXI
>  @item -balloon none
>  @findex -balloon
> @@ -332,6 +333,9 @@ Disable balloon device.
>  @item -balloon virtio[,addr=@var{addr}]
>  Enable virtio balloon device (default), optionally with PCI address
>  @var{addr}.
> +@item -balloon virtio[,deflate-on-oom=@var{deflate-on-oom}]
> +@var{deflate-on-oom} is "on" or "off" and enables whether to let Guest OS
> +to deflate virtio balloon on OOM. Default is off.
>  ETEXI
>  
>  DEF("device", HAS_ARG, QEMU_OPTION_device,

Please don't add stuff to legacy -balloon.
New -device is enough, you don't need to touch qemu-options.hx
for it.

> -- 
> 1.9.1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom
  2014-11-26 10:11 [Qemu-devel] [PATCH 0/2] balloon: add a feature bit to let Guest OS deflate virtio_balloon on OOM Denis V. Lunev
@ 2014-11-26 10:11 ` Denis V. Lunev
  2014-11-26 11:16   ` Michael S. Tsirkin
  0 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2014-11-26 10:11 UTC (permalink / raw)
  Cc: Denis V. Lunev, Michael S. Tsirkin, qemu-devel,
	Raushaniya Maksudova, Anthony Liguori

From: Raushaniya Maksudova <rmaksudova@parallels.com>

Excessive virtio_balloon inflation can cause invocation of OOM-killer,
when Linux is under severe memory pressure. Various mechanisms are
responsible for correct virtio_balloon memory management. Nevertheless it
is often the case that these control tools does not have enough time to
react on fast changing memory load. As a result OS runs out of memory and
invokes OOM-killer. The balancing of memory by use of the virtio balloon
should not cause the termination of processes while there are pages in the
balloon. Now there is no way for virtio balloon driver to free memory at
the last moment before some process get killed by OOM-killer.

This does not provide a security breach as balloon itself is running
inside Guest OS and is working in the cooperation with the host. Thus
some improvements from Guest side should be considered as normal.

To solve the problem, introduce a virtio_balloon callback which is
expected to be called from the oom notifier call chain in out_of_memory()
function. If virtio balloon could release some memory, it will make the
system to return and retry the allocation that forced the out of memory
killer to run.

This behavior should be enabled if and only if appropriate feature bit
is set on the device. It is off by default.

This functionality was recently merged into vanilla Linux (actually in
linux-next at the moment)

  commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
  Author: Raushaniya Maksudova <rmaksudova@parallels.com>
  Date:   Mon Nov 10 09:36:29 2014 +1030

This patch adds respective control bits into QEMU. It introduces
deflate-on-oom option for baloon device which do the trick.

Signed-off-by: Raushaniya Maksudova <rmaksudova@parallels.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Anthony Liguori <aliguori@amazon.com>
CC: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-balloon.c         | 7 +++++++
 include/hw/virtio/virtio-balloon.h | 2 ++
 qemu-options.hx                    | 6 +++++-
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 7bfbb75..9d145fa 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -305,7 +305,12 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
 
 static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
 {
+    VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
     f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
+    if (dev->deflate_on_oom) {
+        f |= (1 << VIRTIO_BALLOON_F_DEFLATE_ON_OOM);
+    }
+
     return f;
 }
 
@@ -409,6 +414,7 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
 }
 
 static Property virtio_balloon_properties[] = {
+    DEFINE_PROP_BOOL("deflate-on-oom", VirtIOBalloon, deflate_on_oom, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index f863bfe..45cc55a 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -30,6 +30,7 @@
 /* The feature bitmap for virtio balloon */
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
 #define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
+#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
@@ -67,6 +68,7 @@ typedef struct VirtIOBalloon {
     QEMUTimer *stats_timer;
     int64_t stats_last_update;
     int64_t stats_poll_interval;
+    bool deflate_on_oom;
 } VirtIOBalloon;
 
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index da9851d..14ede0b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -324,7 +324,8 @@ ETEXI
 DEF("balloon", HAS_ARG, QEMU_OPTION_balloon,
     "-balloon none   disable balloon device\n"
     "-balloon virtio[,addr=str]\n"
-    "                enable virtio balloon device (default)\n", QEMU_ARCH_ALL)
+    "                enable virtio balloon device (default)\n"
+    "               [,deflate-on-oom=on|off]\n", QEMU_ARCH_ALL)
 STEXI
 @item -balloon none
 @findex -balloon
@@ -332,6 +333,9 @@ Disable balloon device.
 @item -balloon virtio[,addr=@var{addr}]
 Enable virtio balloon device (default), optionally with PCI address
 @var{addr}.
+@item -balloon virtio[,deflate-on-oom=@var{deflate-on-oom}]
+@var{deflate-on-oom} is "on" or "off" and enables whether to let Guest OS
+to deflate virtio balloon on OOM. Default is off.
 ETEXI
 
 DEF("device", HAS_ARG, QEMU_OPTION_device,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2015-06-08 15:24 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-27 11:45 [Qemu-devel] [PATCH v2 0/2] balloon: add a feature bit to let Guest OS deflate virtio_balloon on OOM Denis V. Lunev
2014-11-27 11:45 ` [Qemu-devel] [PATCH 1/2] balloon: call qdev_alias_all_properties for proxy dev in balloon class init Denis V. Lunev
2014-11-27 11:45 ` [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom Denis V. Lunev
2014-11-27 11:50   ` Andrey Korolyov
2014-11-27 12:26     ` Denis V. Lunev
2014-11-27 12:28     ` Michael S. Tsirkin
2014-11-27 14:00       ` Andrey Korolyov
2014-11-27 21:49         ` Michael S. Tsirkin
2014-11-27 22:13           ` Andrey Korolyov
2014-11-27 12:31   ` Michael S. Tsirkin
2014-11-27 12:34     ` Denis V. Lunev
  -- strict thread matches above, loose matches on Subject: below --
2015-02-27  6:57 [Qemu-devel] [PATCH v5 0/2] balloon: add a feature bit to let Guest OS deflate Denis V. Lunev
2015-02-27  6:57 ` [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom Denis V. Lunev
2015-04-01  9:44   ` James Bottomley
2015-04-01  9:50     ` Michael S. Tsirkin
2015-04-01  9:51       ` James Bottomley
2015-04-01 10:18         ` Michael S. Tsirkin
2015-05-04  9:47           ` Denis V. Lunev
2015-06-08 14:54             ` James Bottomley
2015-06-08 15:24               ` Michael S. Tsirkin
2015-02-26 17:39 [Qemu-devel] [PATCH v4 0/2] balloon: add a feature bit to let Guest OS deflate Denis V. Lunev
2015-02-26 17:39 ` [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom Denis V. Lunev
2015-02-26 20:39   ` Eric Blake
2014-11-27 12:44 [Qemu-devel] [PATCH v3 0/2] balloon: add a feature bit to let Guest OS deflate virtio_balloon on OOM Denis V. Lunev
2014-11-27 12:44 ` [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom Denis V. Lunev
2014-11-26 10:11 [Qemu-devel] [PATCH 0/2] balloon: add a feature bit to let Guest OS deflate virtio_balloon on OOM Denis V. Lunev
2014-11-26 10:11 ` [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom Denis V. Lunev
2014-11-26 11:16   ` Michael S. Tsirkin
2014-11-27 11:04     ` Denis V. Lunev
2014-11-27 11:08       ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.