All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Soichiro Ueda <the.latticeheart@gmail.com>,
	mst@redhat.com, jasowang@redhat.com, akpm@linux-foundation.org
Cc: virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	kalutes@google.com, mhiramat@kernel.org, cwd@google.com
Subject: Re: [PATCH] virtio_balloon: high order allocation
Date: Mon, 2 Jan 2023 12:55:13 +0100	[thread overview]
Message-ID: <874fd614-23da-c9b6-95f5-41b544b8df18@redhat.com> (raw)
In-Reply-To: <444735fd-7076-7bd3-dc5e-7135a7f3a95c@gmail.com>

On 29.12.22 06:31, Soichiro Ueda wrote:
> Hi David.
> 
>> How does this affect page migration / balloon compaction etc?
> 
> I guess this patch doesn't affect balloon compaction. When allocating
> pages using alloc_pages(), it skips compaction by masking out
> __GFP_RECLAIM if the order is larger than 0.
> 
> As for page migration, in the current implementation it migrate a
> 0-order page to another 0-order page. With this patch, it may migrate a
> high-order page to another same-order page.
> 
> But I noticed that the migrated high-order page is handled as 0-order
> page in virtballoon_migratepage().
> 
>   >     balloon_page_insert(vb_dev_info, newpage);
> 

Yes, I think suspected that it's broken.

We also might want to handle OOM accordingly by splitting the page and 
retrying migration. Almost nothing should stop a balloon page from 
getting migrated.

One thing to try is allocating a higher-order page and immediately 
splitting it into base pages, and enqueuing base pages only. Only 
inflation would be faster, because you could only deflate base pages.

> We should put the newpage into a page list of the corresponding order,
> like this.
> 
>       balloon_page_enqueue(vb_dev_info, newpage, order);
> 
> I'll fix it in the v2 patch.


Note that I have some more concerns:
* We might end up stealing all higher-order pages from the guest instead
   of eating all of the "small" leftover pieces first. This might be
   undesirable. We discussed this in the past in the context of hugepage
   ballooning [not able to locate the abandoned patches quickly].
* PG_offline won't work as expected anymore and result in kdump reading
   inflated memory, which is undesirable. One workaround would be setting
   PG_offline on all base pages, but this needs some more thought.
* How are we handling a scenario where we are requested to deflate e.g.,
   a single 4096 KiB page but we only have higher-order pages allocated?
   I suspect you would over-deflate right now, which is undesirable.

-- 
Thanks,

David / dhildenb


WARNING: multiple messages have this Message-ID (diff)
From: David Hildenbrand <david@redhat.com>
To: Soichiro Ueda <the.latticeheart@gmail.com>,
	mst@redhat.com, jasowang@redhat.com, akpm@linux-foundation.org
Cc: cwd@google.com, linux-kernel@vger.kernel.org, kalutes@google.com,
	linux-mm@kvack.org, mhiramat@kernel.org,
	virtualization@lists.linux-foundation.org
Subject: Re: [PATCH] virtio_balloon: high order allocation
Date: Mon, 2 Jan 2023 12:55:13 +0100	[thread overview]
Message-ID: <874fd614-23da-c9b6-95f5-41b544b8df18@redhat.com> (raw)
In-Reply-To: <444735fd-7076-7bd3-dc5e-7135a7f3a95c@gmail.com>

On 29.12.22 06:31, Soichiro Ueda wrote:
> Hi David.
> 
>> How does this affect page migration / balloon compaction etc?
> 
> I guess this patch doesn't affect balloon compaction. When allocating
> pages using alloc_pages(), it skips compaction by masking out
> __GFP_RECLAIM if the order is larger than 0.
> 
> As for page migration, in the current implementation it migrate a
> 0-order page to another 0-order page. With this patch, it may migrate a
> high-order page to another same-order page.
> 
> But I noticed that the migrated high-order page is handled as 0-order
> page in virtballoon_migratepage().
> 
>   >     balloon_page_insert(vb_dev_info, newpage);
> 

Yes, I think suspected that it's broken.

We also might want to handle OOM accordingly by splitting the page and 
retrying migration. Almost nothing should stop a balloon page from 
getting migrated.

One thing to try is allocating a higher-order page and immediately 
splitting it into base pages, and enqueuing base pages only. Only 
inflation would be faster, because you could only deflate base pages.

> We should put the newpage into a page list of the corresponding order,
> like this.
> 
>       balloon_page_enqueue(vb_dev_info, newpage, order);
> 
> I'll fix it in the v2 patch.


Note that I have some more concerns:
* We might end up stealing all higher-order pages from the guest instead
   of eating all of the "small" leftover pieces first. This might be
   undesirable. We discussed this in the past in the context of hugepage
   ballooning [not able to locate the abandoned patches quickly].
* PG_offline won't work as expected anymore and result in kdump reading
   inflated memory, which is undesirable. One workaround would be setting
   PG_offline on all base pages, but this needs some more thought.
* How are we handling a scenario where we are requested to deflate e.g.,
   a single 4096 KiB page but we only have higher-order pages allocated?
   I suspect you would over-deflate right now, which is undesirable.

-- 
Thanks,

David / dhildenb

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

  reply	other threads:[~2023-01-02 11:56 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-23  9:35 [PATCH] virtio_balloon: high order allocation Soichiro Ueda
2022-12-23  9:40 ` David Hildenbrand
2022-12-23  9:40   ` David Hildenbrand
2022-12-29  5:31   ` Soichiro Ueda
2023-01-02 11:55     ` David Hildenbrand [this message]
2023-01-02 11:55       ` David Hildenbrand
2022-12-24  3:26 ` Liu Shixin
2022-12-29  6:27   ` Soichiro Ueda
2022-12-23 21:04 kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874fd614-23da-c9b6-95f5-41b544b8df18@redhat.com \
    --to=david@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=cwd@google.com \
    --cc=jasowang@redhat.com \
    --cc=kalutes@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhiramat@kernel.org \
    --cc=mst@redhat.com \
    --cc=the.latticeheart@gmail.com \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.