OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]

Subject: Re: [PATCH] virtio_balloon: use non-blocking allocation

On 01/02/2018 10:50 PM, Tetsuo Handa wrote:
Commit c7cdff0e864713a0 ("virtio_balloon: fix deadlock on OOM") tried to
avoid OOM lockup by moving memory allocations to outside of balloon_lock.

Now, Wei is trying to allocate far more pages outside of balloon_lock and
some more memory inside of balloon_lock in order to perform efficient
communication between host and guest using scatter-gather API.

Since pages allocated outside of balloon_lock are not visible to the OOM
notifier path until fill_balloon() holds balloon_lock (and enqueues the
pending pages), allocating more pages than now may lead to unacceptably
premature OOM killer invocation.

It would be possible to make the pending pages visible to the OOM notifier
path. But there is no need to try to allocate memory so hard from the
beginning. As of commit 18468d93e53b037e ("mm: introduce a common
interface for balloon pages mobility"), it made sense to try allocation
as hard as possible. But after commit 5a10b7dbf904bfe0 ("virtio_balloon:
free some memory from balloon on OOM"), it no longer makes sense to try
allocation as hard as possible, for fill_balloon() will after all have to
release just allocated memory if some allocation request hits the OOM
notifier path. Therefore, this patch disables __GFP_DIRECT_RECLAIM when
allocating memory for inflating the balloon. Then, memory for inflating
the balloon can be allocated inside balloon_lock, and we can release just
allocated memory as needed.

Also, this patch adds __GFP_NOWARN, for possibility of hitting memory
allocation failure is increased by removing __GFP_DIRECT_RECLAIM, which
might spam the kernel log buffer. At the same time, this patch moves
"puff" messages to outside of balloon_lock, for it is not a good thing to
block the OOM notifier path for 1/5 of a second. (Moreover, it is better
to release the workqueue and allow processing other pending items. But
that change is out of this patch's scope.)

__GFP_NOMEMALLOC is currently not required because workqueue context
which calls balloon_page_alloc() won't cause __gfp_pfmemalloc_flags()
to return ALLOC_OOM. But since some process context might start calling
balloon_page_alloc() in future, this patch does not remove

(Only compile tested. Please do runtime tests before committing.)

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Wei Wang <wei.w.wang@intel.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
  drivers/virtio/virtio_balloon.c | 23 +++++++++++++----------
  mm/balloon_compaction.c         |  5 +++--
  2 files changed, 16 insertions(+), 12 deletions(-)

I think it is better to simply make the temporal "LIST_HEAD(pages)" to be visible to oom notify, e.g. make it "struct list_head vb->inflating_pages"

Then we can change virtioballoon_oom_notify():

static int oom_notify()
    if (*freed != oom_pages && !list_empty(&vb->inflating_pages))
               return NOTIFY_BAD;

    return NOTIFY_OK;

virtioballoon_oom_notify() {
    int ret;

    do {
        ret = oom_notify()
    } while (ret == NOTIFY_BAD);

    return ret;

I view the above as something "nice to have" (users also have an option to disable F_DEFLATE_ON_OOM, in which case inflated pages are also not released by oom). I can help with this after the "virtio_balloon enhancement" series is done.


[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]