From: teawater <teawaterz@linux.alibaba.com>
To: David Hildenbrand <david@redhat.com>
Cc: "pagupta@redhat.com" <pagupta@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
"jasowang@redhat.com" <jasowang@redhat.com>,
"mojha@codeaurora.org" <mojha@codeaurora.org>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
LKML <linux-kernel@vger.kernel.org>,
Nadav Amit <namit@vmware.com>,
Alexander Duyck <alexander.h.duyck@linux.intel.com>,
Linux Virtualization <virtualization@lists.linux-foundation.org>,
Andrew Morton <akpm@linux-foundation.org>,
Hui Zhu <teawater@gmail.com>
Subject: Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
Date: Thu, 2 Apr 2020 12:02:47 +0800 [thread overview]
Message-ID: <07AF0005-2711-4137-A96D-DA264A05C302@linux.alibaba.com> (raw)
In-Reply-To: <5e0e0659-5f70-2162-96be-5fcd0d3f46ad@redhat.com>
> 2020年4月1日 17:48,David Hildenbrand <david@redhat.com> 写道:
>
> On 31.03.20 18:37, Nadav Amit wrote:
>>> On Mar 31, 2020, at 7:09 AM, David Hildenbrand <david@redhat.com> wrote:
>>>
>>> On 31.03.20 16:07, Michael S. Tsirkin wrote:
>>>> On Tue, Mar 31, 2020 at 04:03:18PM +0200, David Hildenbrand wrote:
>>>>> On 31.03.20 15:37, Michael S. Tsirkin wrote:
>>>>>> On Tue, Mar 31, 2020 at 03:32:05PM +0200, David Hildenbrand wrote:
>>>>>>> On 31.03.20 15:24, Michael S. Tsirkin wrote:
>>>>>>>> On Tue, Mar 31, 2020 at 12:35:24PM +0200, David Hildenbrand wrote:
>>>>>>>>> On 26.03.20 10:49, Michael S. Tsirkin wrote:
>>>>>>>>>> On Thu, Mar 26, 2020 at 08:54:04AM +0100, David Hildenbrand wrote:
>>>>>>>>>>>> Am 26.03.2020 um 08:21 schrieb Michael S. Tsirkin <mst@redhat.com>:
>>>>>>>>>>>>
>>>>>>>>>>>> On Thu, Mar 12, 2020 at 09:51:25AM +0100, David Hildenbrand wrote:
>>>>>>>>>>>>>> On 12.03.20 09:47, Michael S. Tsirkin wrote:
>>>>>>>>>>>>>> On Thu, Mar 12, 2020 at 09:37:32AM +0100, David Hildenbrand wrote:
>>>>>>>>>>>>>>> 2. You are essentially stealing THPs in the guest. So the fastest
>>>>>>>>>>>>>>> mapping (THP in guest and host) is gone. The guest won't be able to make
>>>>>>>>>>>>>>> use of THP where it previously was able to. I can imagine this implies a
>>>>>>>>>>>>>>> performance degradation for some workloads. This needs a proper
>>>>>>>>>>>>>>> performance evaluation.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I think the problem is more with the alloc_pages API.
>>>>>>>>>>>>>> That gives you exactly the given order, and if there's
>>>>>>>>>>>>>> a larger chunk available, it will split it up.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> But for balloon - I suspect lots of other users,
>>>>>>>>>>>>>> we do not want to stress the system but if a large
>>>>>>>>>>>>>> chunk is available anyway, then we could handle
>>>>>>>>>>>>>> that more optimally by getting it all in one go.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> So if we want to address this, IMHO this calls for a new API.
>>>>>>>>>>>>>> Along the lines of
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> struct page *alloc_page_range(gfp_t gfp, unsigned int min_order,
>>>>>>>>>>>>>> unsigned int max_order, unsigned int *order)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> the idea would then be to return at a number of pages in the given
>>>>>>>>>>>>>> range.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> What do you think? Want to try implementing that?
>>>>>>>>>>>>>
>>>>>>>>>>>>> You can just start with the highest order and decrement the order until
>>>>>>>>>>>>> your allocation succeeds using alloc_pages(), which would be enough for
>>>>>>>>>>>>> a first version. At least I don't see the immediate need for a new
>>>>>>>>>>>>> kernel API.
>>>>>>>>>>>>
>>>>>>>>>>>> OK I remember now. The problem is with reclaim. Unless reclaim is
>>>>>>>>>>>> completely disabled, any of these calls can sleep. After it wakes up,
>>>>>>>>>>>> we would like to get the larger order that has become available
>>>>>>>>>>>> meanwhile.
>>>>>>>>>>>
>>>>>>>>>>> Yes, but that‘s a pure optimization IMHO.
>>>>>>>>>>> So I think we should do a trivial implementation first and then see what we gain from a new allocator API. Then we might also be able to justify it using real numbers.
>>>>>>>>>>
>>>>>>>>>> Well how do you propose implement the necessary semantics?
>>>>>>>>>> I think we are both agreed that alloc_page_range is more or
>>>>>>>>>> less what's necessary anyway - so how would you approximate it
>>>>>>>>>> on top of existing APIs?
>>>>>>>>> diff --git a/include/linux/balloon_compaction.h b/include/linux/balloon_compaction.h
>>>>>>
>>>>>> .....
>>>>>>
>>>>>>
>>>>>>>>> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
>>>>>>>>> index 26de020aae7b..067810b32813 100644
>>>>>>>>> --- a/mm/balloon_compaction.c
>>>>>>>>> +++ b/mm/balloon_compaction.c
>>>>>>>>> @@ -112,23 +112,35 @@ size_t balloon_page_list_dequeue(struct balloon_dev_info *b_dev_info,
>>>>>>>>> EXPORT_SYMBOL_GPL(balloon_page_list_dequeue);
>>>>>>>>>
>>>>>>>>> /*
>>>>>>>>> - * balloon_page_alloc - allocates a new page for insertion into the balloon
>>>>>>>>> - * page list.
>>>>>>>>> + * balloon_pages_alloc - allocates a new page (of at most the given order)
>>>>>>>>> + * for insertion into the balloon page list.
>>>>>>>>> *
>>>>>>>>> * Driver must call this function to properly allocate a new balloon page.
>>>>>>>>> * Driver must call balloon_page_enqueue before definitively removing the page
>>>>>>>>> * from the guest system.
>>>>>>>>> *
>>>>>>>>> + * Will fall back to smaller orders if allocation fails. The order of the
>>>>>>>>> + * allocated page is stored in page->private.
>>>>>>>>> + *
>>>>>>>>> * Return: struct page for the allocated page or NULL on allocation failure.
>>>>>>>>> */
>>>>>>>>> -struct page *balloon_page_alloc(void)
>>>>>>>>> +struct page *balloon_pages_alloc(int order)
>>>>>>>>> {
>>>>>>>>> - struct page *page = alloc_page(balloon_mapping_gfp_mask() |
>>>>>>>>> - __GFP_NOMEMALLOC | __GFP_NORETRY |
>>>>>>>>> - __GFP_NOWARN);
>>>>>>>>> - return page;
>>>>>>>>> + struct page *page;
>>>>>>>>> +
>>>>>>>>> + while (order >= 0) {
>>>>>>>>> + page = alloc_pages(balloon_mapping_gfp_mask() |
>>>>>>>>> + __GFP_NOMEMALLOC | __GFP_NORETRY |
>>>>>>>>> + __GFP_NOWARN, order);
>>>>>>>>> + if (page) {
>>>>>>>>> + set_page_private(page, order);
>>>>>>>>> + return page;
>>>>>>>>> + }
>>>>>>>>> + order--;
>>>>>>>>> + }
>>>>>>>>> + return NULL;
>>>>>>>>> }
>>>>>>>>> -EXPORT_SYMBOL_GPL(balloon_page_alloc);
>>>>>>>>> +EXPORT_SYMBOL_GPL(balloon_pages_alloc);
>>>>>>>>>
>>>>>>>>> /*
>>>>>>>>> * balloon_page_enqueue - inserts a new page into the balloon page list.
>>>>>>>>
>>>>>>>>
>>>>>>>> I think this will try to invoke direct reclaim from the first iteration
>>>>>>>> to free up the max order.
>>>>>>>
>>>>>>> %__GFP_NORETRY: The VM implementation will try only very lightweight
>>>>>>> memory direct reclaim to get some memory under memory pressure (thus it
>>>>>>> can sleep). It will avoid disruptive actions like OOM killer.
>>>>>>>
>>>>>>> Certainly good enough for a first version I would say, no?
>>>>>>
>>>>>> Frankly how well that behaves would depend a lot on the workload.
>>>>>> Can regress just as well.
>>>>>>
>>>>>> For the 1st version I'd prefer something that is the least disruptive,
>>>>>> and that IMHO means we only trigger reclaim at all in the same configuration
>>>>>> as now - when we can't satisfy the lowest order allocation.
>>>>>
>>>>> Agreed.
>>>>>
>>>>>> Anything else would be a huge amount of testing with all kind of
>>>>>> workloads.
>>>>>
>>>>> So doing a "& ~__GFP_RECLAIM" in case order > 0? (as done in
>>>>> GFP_TRANSHUGE_LIGHT)
>>>>
>>>> That will improve the situation when reclaim is not needed, but leave
>>>> the problem in place for when it's needed: if reclaim does trigger, we
>>>> can get a huge free page and immediately break it up.
>>>>
>>>> So it's ok as a first step but it will make the second step harder as
>>>> we'll need to test with reclaim :).
>>>
>>> I expect the whole "steal huge pages from your guest" to be problematic,
>>> as I already mentioned to Alex. This needs a performance evaluation.
>>>
>>> This all smells like a lot of workload dependent fine-tuning. :)
>>
>> AFAIK the hardware overheads of keeping huge-pages in the guest and backing
>> them with 4KB pages are non-negligible. Did you take those into account?
>
> Of course, the fastest mapping will be huge pages in host and guest.
> Having huge pages in your guest but not in your host cannot really be
> solved using ballooning AFAIKs. Hopefully THP in the host will be doing
> its job properly :)
>
> ... however, so far, we haven't done any performance comparisons at all.
> The only numbers from Hui Zhu that I can spot are number of THP in the
> host, which is not really expressing actual guest performance IMHO. That
> definitely has to be done to evaluate the different optimizations we
> might want to try out.
>
I did some tests with vm-scalability on Monday comparing their performance in VM:
//4 processes random r/w
usemem -R -a -Z -n 4 1g
write:
hugepage: 146367 KB/s
thp: 133550 KB/s
normal: 124248 KB/s
read:
hugepage: 103969 KB/s
thp: 100622 KB/s
normal: 88755 KB/s
Best,
Hui
> --
> Thanks,
>
> David / dhildenb
next prev parent reply other threads:[~2020-04-02 4:04 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-12 7:49 [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue Hui Zhu
2020-03-12 7:49 ` [RFC for QEMU] virtio-balloon: Add option thp-order to set VIRTIO_BALLOON_F_THP_ORDER Hui Zhu
2020-03-12 8:22 ` no-reply
2020-03-12 8:25 ` Michael S. Tsirkin
2020-03-17 10:13 ` teawater
2020-03-26 7:07 ` Michael S. Tsirkin
2020-03-12 8:18 ` [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue Michael S. Tsirkin
2020-03-12 8:37 ` David Hildenbrand
2020-03-12 8:47 ` Michael S. Tsirkin
2020-03-12 8:51 ` David Hildenbrand
2020-03-26 7:10 ` Michael S. Tsirkin
2020-03-26 7:20 ` Michael S. Tsirkin
2020-03-26 7:54 ` David Hildenbrand
2020-03-26 9:49 ` Michael S. Tsirkin
2020-03-31 10:35 ` David Hildenbrand
2020-03-31 13:24 ` Michael S. Tsirkin
2020-03-31 13:32 ` David Hildenbrand
2020-03-31 13:37 ` Michael S. Tsirkin
2020-03-31 14:03 ` David Hildenbrand
2020-03-31 14:07 ` Michael S. Tsirkin
2020-03-31 14:09 ` David Hildenbrand
2020-03-31 14:18 ` Michael S. Tsirkin
2020-03-31 14:29 ` David Hildenbrand
2020-03-31 14:34 ` David Hildenbrand
2020-03-31 15:28 ` Michael S. Tsirkin
2020-03-31 16:37 ` Nadav Amit
2020-04-01 9:48 ` David Hildenbrand
2020-04-02 4:02 ` teawater [this message]
2020-04-02 8:00 ` teawater
2020-04-02 12:37 ` Michael S. Tsirkin
2020-03-31 16:27 ` Nadav Amit
2020-04-01 11:21 ` David Hildenbrand
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=07AF0005-2711-4137-A96D-DA264A05C302@linux.alibaba.com \
--to=teawaterz@linux.alibaba.com \
--cc=akpm@linux-foundation.org \
--cc=alexander.h.duyck@linux.intel.com \
--cc=david@redhat.com \
--cc=jasowang@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mojha@codeaurora.org \
--cc=mst@redhat.com \
--cc=namit@vmware.com \
--cc=pagupta@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=teawater@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).