| Message ID | 20210621051152.305224-1-gshan@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mm/page_reporting: Make page reporting work on arm64 with 64KB page size | expand |
So the question I would have is what is the use case for this? It seems like you don't have to deal with the guest native page size issues since you are willing to break up what would otherwise be THP pages on the guest, and the fact that you are willing to go down to 2MB pages which happens to align with the host THP page size for x86 makes me wonder if that is actually the environment you are running in. Rather than having the guest control this it might make sense to look at adding an interface so that the page_reporting_register function and the page_reporting_dev_info struct could be used to report and configure the minimum page size that the host can support for the page reporting. With that the host could then guarantee that it isn't going to hurt performance by splitting pages on the host and risk hurting the virtualization performance. Also you would benefit by looking into the callers of page_reporting_register as there are more than just the virtio balloon that are consuming it. Odds are HyperV won't care about an ARM64 architecture, but your change would essentially disable it outright which is why I think this might be better to address via the consumers of page reporting rather than trying to address it in page reporting itself. Thanks, - Alex On Sun, Jun 20, 2021 at 8:11 PM Gavin Shan <gshan@redhat.com> wrote: > > The page reporting threshold is currently equal to @pageblock_order, which > is 13 and 512MB on arm64 with 64KB base page size selected. The page > reporting won't be triggered if the freeing page can't come up with a free > area like that huge. The condition is hard to be met, especially when the > system memory becomes fragmented. > > This series intends to solve the issue by having page reporting threshold > as 5 (2MB) on arm64 with 64KB base page size. The patches are organized as: > > PATCH[1/3] introduces variable (@page_reporting_order) to replace original > macro (PAGE_REPORTING_MIN_ORDER). It's also exported so that it > can be adjusted at runtime. > PATCH[2/3] renames PAGE_REPORTING_MIN_ORDER with PAGE_REPORTING_ORDER and > allows architecture to specify its own version. > PATCH[3/3] defines PAGE_REPORTING_ORDER to 5, corresponding to 2MB in size, > on arm64 when 64KB base page size is selected. It's still same > as to @pageblock_order for other architectures and cases. > > Gavin Shan (3): > mm/page_reporting: Allow to set reporting order > mm/page_reporting: Allow architecture to select reporting order > arm64: mm: Specify smaller page reporting order > > Documentation/admin-guide/kernel-parameters.txt | 6 ++++++ > arch/arm64/include/asm/page.h | 13 +++++++++++++ > mm/page_reporting.c | 8 ++++++-- > mm/page_reporting.h | 10 +++++++--- > 4 files changed, 32 insertions(+), 5 deletions(-) > > -- > 2.23.0 >
From: Alexander Duyck <alexander.duyck@gmail.com> Sent: Monday, June 21, 2021 7:02 AM > To: Gavin Shan <gshan@redhat.com> > Cc: linux-mm <linux-mm@kvack.org>; LKML <linux-kernel@vger.kernel.org>; Andrew > Morton <akpm@linux-foundation.org>; David Hildenbrand <david@redhat.com>; > Anshuman Khandual <anshuman.khandual@arm.com>; Catalin Marinas > <catalin.marinas@arm.com>; Will Deacon <will@kernel.org>; shan.gavin@gmail.com > Subject: Re: [PATCH 0/3] mm/page_reporting: Make page reporting work on arm64 with > 64KB page size > > So the question I would have is what is the use case for this? It > seems like you don't have to deal with the guest native page size > issues since you are willing to break up what would otherwise be THP > pages on the guest, and the fact that you are willing to go down to > 2MB pages which happens to align with the host THP page size for x86 > makes me wonder if that is actually the environment you are running > in. > > Rather than having the guest control this it might make sense to look > at adding an interface so that the page_reporting_register function > and the page_reporting_dev_info struct could be used to report and > configure the minimum page size that the host can support for the page > reporting. With that the host could then guarantee that it isn't going > to hurt performance by splitting pages on the host and risk hurting > the virtualization performance. > > Also you would benefit by looking into the callers of > page_reporting_register as there are more than just the virtio balloon > that are consuming it. Odds are HyperV won't care about an ARM64 > architecture, FWIW, Hyper-V *does* care about ARM64. It's already in use by the Windows Subsystem for Linux VM that's part of Windows 10 on ARM64 hardware. We're working to get the code accepted upstream. Michael > but your change would essentially disable it outright > which is why I think this might be better to address via the consumers > of page reporting rather than trying to address it in page reporting > itself. > > Thanks, > > - Alex > > On Sun, Jun 20, 2021 at 8:11 PM Gavin Shan <gshan@redhat.com> wrote: > > > > The page reporting threshold is currently equal to @pageblock_order, which > > is 13 and 512MB on arm64 with 64KB base page size selected. The page > > reporting won't be triggered if the freeing page can't come up with a free > > area like that huge. The condition is hard to be met, especially when the > > system memory becomes fragmented. > > > > This series intends to solve the issue by having page reporting threshold > > as 5 (2MB) on arm64 with 64KB base page size. The patches are organized as: > > > > PATCH[1/3] introduces variable (@page_reporting_order) to replace original > > macro (PAGE_REPORTING_MIN_ORDER). It's also exported so that it > > can be adjusted at runtime. > > PATCH[2/3] renames PAGE_REPORTING_MIN_ORDER with PAGE_REPORTING_ORDER > and > > allows architecture to specify its own version. > > PATCH[3/3] defines PAGE_REPORTING_ORDER to 5, corresponding to 2MB in size, > > on arm64 when 64KB base page size is selected. It's still same > > as to @pageblock_order for other architectures and cases. > > > > Gavin Shan (3): > > mm/page_reporting: Allow to set reporting order > > mm/page_reporting: Allow architecture to select reporting order > > arm64: mm: Specify smaller page reporting order > > > > Documentation/admin-guide/kernel-parameters.txt | 6 ++++++ > > arch/arm64/include/asm/page.h | 13 +++++++++++++ > > mm/page_reporting.c | 8 ++++++-- > > mm/page_reporting.h | 10 +++++++--- > > 4 files changed, 32 insertions(+), 5 deletions(-) > > > > -- > > 2.23.0 > >
On 6/22/21 12:02 AM, Alexander Duyck wrote: > So the question I would have is what is the use case for this? It > seems like you don't have to deal with the guest native page size > issues since you are willing to break up what would otherwise be THP > pages on the guest, and the fact that you are willing to go down to > 2MB pages which happens to align with the host THP page size for x86 > makes me wonder if that is actually the environment you are running > in. > > Rather than having the guest control this it might make sense to look > at adding an interface so that the page_reporting_register function > and the page_reporting_dev_info struct could be used to report and > configure the minimum page size that the host can support for the page > reporting. With that the host could then guarantee that it isn't going > to hurt performance by splitting pages on the host and risk hurting > the virtualization performance. > > Also you would benefit by looking into the callers of > page_reporting_register as there are more than just the virtio balloon > that are consuming it. Odds are HyperV won't care about an ARM64 > architecture, but your change would essentially disable it outright > which is why I think this might be better to address via the consumers > of page reporting rather than trying to address it in page reporting > itself. > Alex, the issue was initially found on guest with 64KB base page size when memory balloon is used. The same issue isn't found on guest with 4KB base page size. Both guests can be running on host with 4KB or 64KB base page size. Besides, the code changes are specific to ARM64, meaning they don't affect other architectures. (1) If we have the combination of guest.64KB and host.4KB, the 2MB page reporting order is still avoid splitting the THP on host. (2) If we have the combination of guest.64KB and host.64KB, the THP splitting can't be avoided. After thinking about it, I agree to reuse page_report_register() and "struct page_reporting_dev_info" to have the page reporting order. In this way, the PAGE_REPORTING_CAPACITY can be provided by consumer in future either. I will have these updates in v2 patches. Thanks, Gavin > > On Sun, Jun 20, 2021 at 8:11 PM Gavin Shan <gshan@redhat.com> wrote: >> >> The page reporting threshold is currently equal to @pageblock_order, which >> is 13 and 512MB on arm64 with 64KB base page size selected. The page >> reporting won't be triggered if the freeing page can't come up with a free >> area like that huge. The condition is hard to be met, especially when the >> system memory becomes fragmented. >> >> This series intends to solve the issue by having page reporting threshold >> as 5 (2MB) on arm64 with 64KB base page size. The patches are organized as: >> >> PATCH[1/3] introduces variable (@page_reporting_order) to replace original >> macro (PAGE_REPORTING_MIN_ORDER). It's also exported so that it >> can be adjusted at runtime. >> PATCH[2/3] renames PAGE_REPORTING_MIN_ORDER with PAGE_REPORTING_ORDER and >> allows architecture to specify its own version. >> PATCH[3/3] defines PAGE_REPORTING_ORDER to 5, corresponding to 2MB in size, >> on arm64 when 64KB base page size is selected. It's still same >> as to @pageblock_order for other architectures and cases. >> >> Gavin Shan (3): >> mm/page_reporting: Allow to set reporting order >> mm/page_reporting: Allow architecture to select reporting order >> arm64: mm: Specify smaller page reporting order >> >> Documentation/admin-guide/kernel-parameters.txt | 6 ++++++ >> arch/arm64/include/asm/page.h | 13 +++++++++++++ >> mm/page_reporting.c | 8 ++++++-- >> mm/page_reporting.h | 10 +++++++--- >> 4 files changed, 32 insertions(+), 5 deletions(-) >> >> -- >> 2.23.0 >> >
On 6/22/21 2:06 AM, Michael Kelley wrote: > From: Alexander Duyck <alexander.duyck@gmail.com> Sent: Monday, June 21, 2021 7:02 AM >> To: Gavin Shan <gshan@redhat.com> >> Cc: linux-mm <linux-mm@kvack.org>; LKML <linux-kernel@vger.kernel.org>; Andrew >> Morton <akpm@linux-foundation.org>; David Hildenbrand <david@redhat.com>; >> Anshuman Khandual <anshuman.khandual@arm.com>; Catalin Marinas >> <catalin.marinas@arm.com>; Will Deacon <will@kernel.org>; shan.gavin@gmail.com >> Subject: Re: [PATCH 0/3] mm/page_reporting: Make page reporting work on arm64 with >> 64KB page size >> >> So the question I would have is what is the use case for this? It >> seems like you don't have to deal with the guest native page size >> issues since you are willing to break up what would otherwise be THP >> pages on the guest, and the fact that you are willing to go down to >> 2MB pages which happens to align with the host THP page size for x86 >> makes me wonder if that is actually the environment you are running >> in. >> >> Rather than having the guest control this it might make sense to look >> at adding an interface so that the page_reporting_register function >> and the page_reporting_dev_info struct could be used to report and >> configure the minimum page size that the host can support for the page >> reporting. With that the host could then guarantee that it isn't going >> to hurt performance by splitting pages on the host and risk hurting >> the virtualization performance. >> >> Also you would benefit by looking into the callers of >> page_reporting_register as there are more than just the virtio balloon >> that are consuming it. Odds are HyperV won't care about an ARM64 >> architecture, > > FWIW, Hyper-V *does* care about ARM64. It's already in use by > the Windows Subsystem for Linux VM that's part of Windows 10 > on ARM64 hardware. We're working to get the code accepted > upstream. > Michael, thanks for your confirmation. As the issue found on 64KB guest when memory balloon is used, lets resolve the case first. I will look into Hyper-V case later if you agree. It won't be difficult to fix the same issue for Hyper-V after the solution is figured out for memory balloon. Thanks, Gavin > >> but your change would essentially disable it outright >> which is why I think this might be better to address via the consumers >> of page reporting rather than trying to address it in page reporting >> itself. >> >> Thanks, >> >> - Alex >> >> On Sun, Jun 20, 2021 at 8:11 PM Gavin Shan <gshan@redhat.com> wrote: >>> >>> The page reporting threshold is currently equal to @pageblock_order, which >>> is 13 and 512MB on arm64 with 64KB base page size selected. The page >>> reporting won't be triggered if the freeing page can't come up with a free >>> area like that huge. The condition is hard to be met, especially when the >>> system memory becomes fragmented. >>> >>> This series intends to solve the issue by having page reporting threshold >>> as 5 (2MB) on arm64 with 64KB base page size. The patches are organized as: >>> >>> PATCH[1/3] introduces variable (@page_reporting_order) to replace original >>> macro (PAGE_REPORTING_MIN_ORDER). It's also exported so that it >>> can be adjusted at runtime. >>> PATCH[2/3] renames PAGE_REPORTING_MIN_ORDER with PAGE_REPORTING_ORDER >> and >>> allows architecture to specify its own version. >>> PATCH[3/3] defines PAGE_REPORTING_ORDER to 5, corresponding to 2MB in size, >>> on arm64 when 64KB base page size is selected. It's still same >>> as to @pageblock_order for other architectures and cases. >>> >>> Gavin Shan (3): >>> mm/page_reporting: Allow to set reporting order >>> mm/page_reporting: Allow architecture to select reporting order >>> arm64: mm: Specify smaller page reporting order >>> >>> Documentation/admin-guide/kernel-parameters.txt | 6 ++++++ >>> arch/arm64/include/asm/page.h | 13 +++++++++++++ >>> mm/page_reporting.c | 8 ++++++-- >>> mm/page_reporting.h | 10 +++++++--- >>> 4 files changed, 32 insertions(+), 5 deletions(-) >>> >>> -- >>> 2.23.0 >>>
The page reporting threshold is currently equal to @pageblock_order, which is 13 and 512MB on arm64 with 64KB base page size selected. The page reporting won't be triggered if the freeing page can't come up with a free area like that huge. The condition is hard to be met, especially when the system memory becomes fragmented. This series intends to solve the issue by having page reporting threshold as 5 (2MB) on arm64 with 64KB base page size. The patches are organized as: PATCH[1/3] introduces variable (@page_reporting_order) to replace original macro (PAGE_REPORTING_MIN_ORDER). It's also exported so that it can be adjusted at runtime. PATCH[2/3] renames PAGE_REPORTING_MIN_ORDER with PAGE_REPORTING_ORDER and allows architecture to specify its own version. PATCH[3/3] defines PAGE_REPORTING_ORDER to 5, corresponding to 2MB in size, on arm64 when 64KB base page size is selected. It's still same as to @pageblock_order for other architectures and cases. Gavin Shan (3): mm/page_reporting: Allow to set reporting order mm/page_reporting: Allow architecture to select reporting order arm64: mm: Specify smaller page reporting order Documentation/admin-guide/kernel-parameters.txt | 6 ++++++ arch/arm64/include/asm/page.h | 13 +++++++++++++ mm/page_reporting.c | 8 ++++++-- mm/page_reporting.h | 10 +++++++--- 4 files changed, 32 insertions(+), 5 deletions(-)