Re: [PATCH v1 3/3] virtio-balloon: Switch back to OOM handler for VIRTIO_BALLOON_F_DEFLATE_ON_OOM

Testing this patch is on my short-term TODO list, but I wasn't able to get
to it this week. It is prioritized.

In the meantime, I can anecdotally vouch that kernels before 4.19, the ones
using the OOM notifier callback, have roughly 10x faster balloon inflation
when pressuring the cache. So I anticipate this patch will return to that
state and help my use case.

I will try to post official measurements of this patch next week.

On Sun, Feb 16, 2020 at 1:47 AM Michael S. Tsirkin [off-list ref] wrote:

On Fri, Feb 14, 2020 at 10:51:43AM +0100, David Hildenbrand wrote:

quoted

On 05.02.20 17:34, David Hildenbrand wrote:

quoted

Commit 71994620bb25 ("virtio_balloon: replace oom notifier with

shrinker")

quoted

quoted

changed the behavior when deflation happens automatically. Instead of
deflating when called by the OOM handler, the shrinker is used.

However, the balloon is not simply some slab cache that should be
shrunk when under memory pressure. The shrinker does not have a

concept of

quoted

quoted

priorities, so this behavior cannot be configured.

There was a report that this results in undesired side effects when
inflating the balloon to shrink the page cache. [1]
    "When inflating the balloon against page cache (i.e. no free memory
     remains) vmscan.c will both shrink page cache, but also invoke the
     shrinkers -- including the balloon's shrinker. So the balloon
     driver allocates memory which requires reclaim, vmscan gets this
     memory by shrinking the balloon, and then the driver adds the
     memory back to the balloon. Basically a busy no-op."

The name "deflate on OOM" makes it pretty clear when deflation should
happen - after other approaches to reclaim memory failed, not while
reclaiming. This allows to minimize the footprint of a guest - memory
will only be taken out of the balloon when really needed.

Especially, a drop_slab() will result in the whole balloon getting
deflated - undesired. While handling it via the OOM handler might not

be

quoted

quoted

perfect, it keeps existing behavior. If we want a different behavior,

then

quoted

quoted

we need a new feature bit and document it properly (although, there

should

quoted

quoted

be a clear use case and the intended effects should be well described).

Keep using the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT, because
this has no such side effects. Always register the shrinker with
VIRTIO_BALLOON_F_FREE_PAGE_HINT now. We are always allowed to reuse

free

quoted

quoted

pages that are still to be processed by the guest. The hypervisor takes
care of identifying and resolving possible races between processing a
hinting request and the guest reusing a page.

In contrast to pre commit 71994620bb25 ("virtio_balloon: replace oom
notifier with shrinker"), don't add a moodule parameter to configure

the

quoted

quoted

number of pages to deflate on OOM. Can be re-added if really needed.
Also, pay attention that leak_balloon() returns the number of 4k pages

-

quoted

quoted

convert it properly in virtio_balloon_oom_notify().

Note1: using the OOM handler is frowned upon, but it really is what we
       need for this feature.

Note2: without VIRTIO_BALLOON_F_MUST_TELL_HOST (iow, always with QEMU)

we

quoted

quoted

       could actually skip sending deflation requests to our

hypervisor,

quoted

quoted

       making the OOM path *very* simple. Besically freeing pages and
       updating the balloon. If the communication with the host ever
       becomes a problem on this call path.

@Michael, how to proceed with this?

I'd like to see some reports that this helps people.
e.g. a tested-by tag.

quoted

--
Thanks,

David / dhildenb