From: "Michael S. Tsirkin" Subject: Re: [PATCH v1 3/3] virtio-balloon: Switch back to OOM handler for VIRTIO_BALLOON_F_DEFLATE_ON_OOM Date: Sun, 16 Feb 2020 04:47:12 -0500 Message-ID: <20200216044641-mutt-send-email-mst@kernel.org> References: <20200205163402.42627-1-david@redhat.com> <20200205163402.42627-4-david@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: virtualization-bounces@lists.linux-foundation.org Sender: "Virtualization" To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, Tyler Sanderson , linux-mm@kvack.org, Nadav Amit , David Rientjes , Alexander Duyck , Michal Hocko List-Id: virtualization@lists.linuxfoundation.org On Fri, Feb 14, 2020 at 10:51:43AM +0100, David Hildenbrand wrote: > On 05.02.20 17:34, David Hildenbrand wrote: > > Commit 71994620bb25 ("virtio_balloon: replace oom notifier with shrinker") > > 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 > > 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 > > perfect, it keeps existing behavior. If we want a different behavior, then > > we need a new feature bit and document it properly (although, there should > > 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 > > 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 > > 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 - > > 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 > > could actually skip sending deflation requests to our hypervisor, > > 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. > -- > Thanks, > > David / dhildenb