Re: [PATCH v14 0/6] mm / virtio: Provide support for unused page reporting
From: David Hildenbrand
Date: Wed Nov 27 2019 - 05:01:27 EST
On 26.11.19 17:45, Alexander Duyck wrote:
> On Tue, 2019-11-26 at 13:20 +0100, David Hildenbrand wrote:
>> On 19.11.19 22:46, Alexander Duyck wrote:
>>> This series provides an asynchronous means of reporting unused guest
>>> pages to a hypervisor so that the memory associated with those pages can
>>> be dropped and reused by other processes and/or guests on the host. Using
>>> this it is possible to avoid unnecessary I/O to disk and greatly improve
>>> performance in the case of memory overcommit on the host.
>>>
>>> When enabled it will allocate a set of statistics to track the number of
>>> reported pages. When the nr_free for a given free area is greater than
>>> this by the high water mark we will schedule a worker to begin pulling the
>>> non-reported memory and to provide it to the reporting interface via a
>>> scatterlist.
>>>
>>> Currently this is only in use by virtio-balloon however there is the hope
>>> that at some point in the future other hypervisors might be able to make
>>> use of it. In the virtio-balloon/QEMU implementation the hypervisor is
>>> currently using MADV_DONTNEED to indicate to the host kernel that the page
>>> is currently unused. It will be zeroed and faulted back into the guest the
>>> next time the page is accessed.
>>
>> Remind me why we are using MADV_DONTNEED? Mostly for debugging purposes
>> right now, right? Did you do any measurements with MADV_FREE? I guess
>> there should be quite a performance increase in some scenarios.
>
> There are actually a few reasons for not using MADV_FREE.
>
> The first one was debugging as I could visibly see how much memory had
> been freed by just checking the memory consumption by the guest. I didn't
> have to wait for memory pressure to trigger the memory freeing. In
> addition it would force the pages out of the guest so it was much easier
> to see if I was freeing the wrong pages.
>
> The second reason is because it is much more portable. The MADV_FREE has
> only been a part of the Linux kernel since about 4.5. So if you are
> running on an older kernel the option might not be available.
I guess optionally enabling it (for !filebacked and !huge pages) in QEMU
after sensing would be possible. Fallback to ram_discard_range().
>
> The third reason is simply effort involved. If I used MADV_DONTNEED then I
> can just use ram_block_discard_range which is the same function used by
> other parts of the balloon driver.
Yes, that makes perfect sense.
>
> Finally it is my understanding is that MADV_FREE only works on anonymous
> memory (https://elixir.bootlin.com/linux/v5.4/source/mm/madvise.c#L700). I
> was concerned that using MADV_FREE wouldn't work if used on file backed
> memory such as hugetlbfs which is an option for QEMU if I am not mistaken.
Yes, MADV_FREE works just like MADV_DONTNEED only on anonymous memory.
In case of files/hugetlbfs you have to use
fallocate(rb->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, ...).
E.g., see qemu/exec.c:ram_block_discard_range. You can do something
similar to this:
static bool madv_free_sensed, madv_free_available;
int ret = -EINVAL;
/*
* MADV_FREE only works on anonymous memory, and especially not on
* hugetlbfs. Older kernels don't support it.
*/
if (rb->page_size == qemu_host_page_size && rb->fb != -1 &&
(!madv_free_sensed || madv_free_available)) {
ret = madvise(start, length, MADV_FREE);
if (ret) {
madv_free_sensed = true;
madv_free_available = false;
} else if (!madv_free_sensed) {
madv_free_sensed = true;
madv_free_available = true;
}
}
/* fallback to MADV_DONTNEED / FALLOC_FL_PUNCH_HOLE */
if (ret) {
ram_block_discard_range(rb, start, length);
}
I agree that something like should be an addon to the current patch set.
>
>>> To track if a page is reported or not the Uptodate flag was repurposed and
>>> used as a Reported flag for Buddy pages. We walk though the free list
>>> isolating pages and adding them to the scatterlist until we either
>>> encounter the end of the list or have filled the scatterlist with pages to
>>> be reported. If we fill the scatterlist before we reach the end of the
>>> list we rotate the list so that the first unreported page we encounter is
>>> moved to the head of the list as that is where we will resume after we
>>> have freed the reported pages back into the tail of the list.
>>
>> So the boundary pointer didn't actually provide that big of a benefit I
>> assume (IOW, worst thing is you have to re-scan the whole list)?
>
> I rewrote the code quite a bit to get rid of the disadvantages.
> Specifically what the boundary pointer was doing was saving our place in
> the list when we left. Even without that we still had to re-scan the
> entire list with each zone processed anyway. With these changes we end up
> potentially having to perform one additional rescan per free list.
>
> Where things differ now is that the fetching function doesn't bail out of
> the list and start over per page. Instead it fills the entire scatterlist
> before it exits, and before doing so it will advance the head to the next
> non-reported page in the list. In addition instead of walking all of the
> orders and migrate types looking for each page the code is now more
> methodical and will only work one free list at a time and do not revisit
> it until we have processed the entire zone.
Makes perfect sense to me.
>
> Even with all that we still take a pretty significant performance hit in
> the page shuffing case, however I am willing to give that up for the sake
> of being less intrusive.
Makes sense as well, especially for a first version.
>
>>> Below are the results from various benchmarks. I primarily focused on two
>>> tests. The first is the will-it-scale/page_fault2 test, and the other is
>>> a modified version of will-it-scale/page_fault1 that was enabled to use
>>> THP. I did this as it allows for better visibility into different parts
>>> of the memory subsystem. The guest is running with 32G for RAM on one
>>> node of a E5-2630 v3. The host has had some power saving features disabled
>>> by setting the /dev/cpu_dma_latency value to 10ms.
>>>
>>> Test page_fault1 (THP) page_fault2
>>> Name tasks Process Iter STDEV Process Iter STDEV
>>> Baseline 1 1203934.75 0.04% 379940.75 0.11%
>>> 16 8828217.00 0.85% 3178653.00 1.28%
>>>
>>> Patches applied 1 1207961.25 0.10% 380852.25 0.25%
>>> 16 8862373.00 0.98% 3246397.25 0.68%
>>>
>>> Patches enabled 1 1207758.75 0.17% 373079.25 0.60%
>>> MADV disabled 16 8870373.75 0.29% 3204989.75 1.08%
>>>
>>> Patches enabled 1 1261183.75 0.39% 373201.50 0.50%
>>> 16 8371359.75 0.65% 3233665.50 0.84%
>>>
>>> Patches enabled 1 1090201.50 0.25% 376967.25 0.29%
>>> page shuffle 16 8108719.75 0.58% 3218450.25 1.07%
>>>
>>> The results above are for a baseline with a linux-next-20191115 kernel,
>>> that kernel with this patch set applied but page reporting disabled in
>>> virtio-balloon, patches applied but the madvise disabled by direct
>>> assigning a device, the patches applied and page reporting fully
>>> enabled, and the patches enabled with page shuffling enabled. These
>>> results include the deviation seen between the average value reported here
>>> versus the high and/or low value. I observed that during the test memory
>>> usage for the first three tests never dropped whereas with the patches
>>> fully enabled the VM would drop to using only a few GB of the host's
>>> memory when switching from memhog to page fault tests.
>>>
>>> Most of the overhead seen with this patch set enabled seems due to page
>>> faults caused by accessing the reported pages and the host zeroing the page
>>> before giving it back to the guest. This overhead is much more visible when
>>> using THP than with standard 4K pages. In addition page shuffling seemed to
>>> increase the amount of faults generated due to an increase in memory churn.
>>
>> MADV_FREE would be interesting.
>
> I can probably code something up. However that is going to push a bunch of
> complexity into the QEMU code and doesn't really mean much to the kernel
> code. I can probably add it as another QEMU patch to the set since it is
> just a matter of having a function similar to ram_block_discard_range that
> uses MADV_FREE instead of MADV_DONTNEED.
Yes, addon patch makes perfect sense. The nice thing about MADV_FREE is
that you only take back pages from a process when really under memory
pressure (before going to SWAP). You will still get a pagefault on the
next access (to identify that the page is still in use after all), but
don't have to fault in a fresh page.
>
>>> The overall guest size is kept fairly small to only a few GB while the test
>>> is running. If the host memory were oversubscribed this patch set should
>>> result in a performance improvement as swapping memory in the host can be
>>> avoided.
>>>
>>> A brief history on the background of unused page reporting can be found at:
>>> https://lore.kernel.org/lkml/29f43d5796feed0dec8e8bb98b187d9dac03b900.camel@xxxxxxxxxxxxxxx/
>>>
>>> Changes from v12:
>>> https://lore.kernel.org/lkml/20191022221223.17338.5860.stgit@xxxxxxxxxxxxxxxxxxxxx/
>>> Rebased on linux-next 20191031
>>> Renamed page_is_reported to page_reported
>>> Renamed add_page_to_reported_list to mark_page_reported
>>> Dropped unused definition of add_page_to_reported_list for non-reporting case
>>> Split free_area_reporting out from get_unreported_tail
>>> Minor updates to cover page
>>>
>>> Changes from v13:
>>> https://lore.kernel.org/lkml/20191105215940.15144.65968.stgit@xxxxxxxxxxxxxxxxxxxxx/
>>> Rewrote core reporting functionality
>>> Merged patches 3 & 4
>>> Dropped boundary list and related code
>>> Folded get_reported_page into page_reporting_fill
>>> Folded page_reporting_fill into page_reporting_cycle
>>> Pulled reporting functionality out of free_reported_page
>>> Renamed it to __free_isolated_page
>>> Moved page reporting specific bits to page_reporting_drain
>>> Renamed phdev to prdev since we aren't "hinting" we are "reporting"
>>> Added documentation to describe the usage of unused page reporting
>>> Updated cover page and patch descriptions to avoid mention of boundary
>>>
>>>
>>> ---
>>>
>>> Alexander Duyck (6):
>>> mm: Adjust shuffle code to allow for future coalescing
>>> mm: Use zone and order instead of free area in free_list manipulators
>>> mm: Introduce Reported pages
>>> mm: Add unused page reporting documentation
>>> virtio-balloon: Pull page poisoning config out of free page hinting
>>> virtio-balloon: Add support for providing unused page reports to host
>>>
>>>
>>> Documentation/vm/unused_page_reporting.rst | 44 ++++
>>> drivers/virtio/Kconfig | 1
>>> drivers/virtio/virtio_balloon.c | 88 +++++++
>>> include/linux/mmzone.h | 56 +----
>>> include/linux/page-flags.h | 11 +
>>> include/linux/page_reporting.h | 31 +++
>>> include/uapi/linux/virtio_balloon.h | 1
>>> mm/Kconfig | 11 +
>>> mm/Makefile | 1
>>> mm/memory_hotplug.c | 2
>>> mm/page_alloc.c | 181 +++++++++++----
>>> mm/page_reporting.c | 337 ++++++++++++++++++++++++++++
>>> mm/page_reporting.h | 125 ++++++++++
>>> mm/shuffle.c | 12 -
>>> mm/shuffle.h | 6
>>> 15 files changed, 805 insertions(+), 102 deletions(-)
>>
>> So roughly 100 LOC less added, that's nice to see :)
>>
>> I'm planning to look into the details soon, just fairly busy lately. I
>> hope Mel Et al. can also comment.
>
> Agreed. I can see if I can generate something to get the MADV_FREE
> numbers. I suspect they were probably be somewhere between the MADV
> disabled and fully enabled case, since we will still be taking the page
> faults but not doing the zeroing.
Exactly.
--
Thanks,
David / dhildenb