From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51546) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xtwsu-0003nL-KN for qemu-devel@nongnu.org; Thu, 27 Nov 2014 06:05:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xtwsp-00083E-OM for qemu-devel@nongnu.org; Thu, 27 Nov 2014 06:05:16 -0500 Received: from relay.parallels.com ([195.214.232.42]:51048) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xtwsp-0007zc-D8 for qemu-devel@nongnu.org; Thu, 27 Nov 2014 06:05:11 -0500 Message-ID: <54770557.6010303@openvz.org> Date: Thu, 27 Nov 2014 14:04:55 +0300 From: "Denis V. Lunev" MIME-Version: 1.0 References: <1416996685-15115-1-git-send-email-den@openvz.org> <1416996685-15115-3-git-send-email-den@openvz.org> <20141126111603.GB4222@redhat.com> In-Reply-To: <20141126111603.GB4222@redhat.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] balloon: add a feature bit to let Guest OS deflate balloon on oom List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, 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 >> >> 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 >> 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 >> Signed-off-by: Denis V. Lunev >> CC: Anthony Liguori >> CC: Michael S. Tsirkin >> --- >> 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