[PATCH v5 0/4] mm/page_reporting: Make page reporting work on arm64 with 64KB page size

[PATCH v5 0/4] mm/page_reporting: Make page reporting work on arm64 with 64KB page size

[Gavin Shan]

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/4] Fix some coding style in __page_reporting_request().
   PATCH[2/4] Represents page reporting order with variable so that it can
              be exported as module parameter.
   PATCH[3/4] Allows the device driver (e.g. virtio_balloon) to specify
              the page reporting order when the device info is registered.
   PATCH[4/4] Specifies the page reporting order to 5, corresponding to
              2MB in size on ARM64 when 64KB base page size is used.

Changelog
=========
v5:
   * Restore @page_reporting_order to @pageblock_order when
     device is registered in PATCH[2/4] to keep "git bisect"
     friendly at least.                                           (Alex)
v4:
   * Set @page_reporting_order to MAX_ORDER. Its value is
     specified by the driver or falls back to @pageblock_order
     when page reporting device is registered.                    (Alex)
   * Include "module.h" in page_reporting.c                       (Andrew)
v3:
   * Avoid overhead introduced by function all                    (Alex)
   * Export page reporting order as module parameter              (Gavin)
v2:
   * Rewrite the patches as Alex suggested                        (Alex)

Gavin Shan (4):
  mm/page_reporting: Fix code style in __page_reporting_request()
  mm/page_reporting: Export reporting order as module parameter
  mm/page_reporting: Allow driver to specify reporting
  virtio_balloon: Specify page reporting order if needed

 .../admin-guide/kernel-parameters.txt         |  6 +++++
 drivers/virtio/virtio_balloon.c               | 17 ++++++++++++++
 include/linux/page_reporting.h                |  3 +++
 mm/page_reporting.c                           | 22 +++++++++++++++----
 mm/page_reporting.h                           |  5 ++---
 5 files changed, 46 insertions(+), 7 deletions(-)

-- 
2.23.0

[PATCH v5 1/4] mm/page_reporting: Fix code style in __page_reporting_request()

[Gavin Shan]

The lines of comments would be starting with one, instead two space.
This corrects the style.

Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
---
 mm/page_reporting.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/page_reporting.c b/mm/page_reporting.c
index c50d93ffa252..df9c5054e1b4 100644
--- a/mm/page_reporting.c
+++ b/mm/page_reporting.c
@@ -31,8 +31,8 @@ __page_reporting_request(struct page_reporting_dev_info *prdev)
 		return;
 
 	/*
-	 *  If reporting is already active there is nothing we need to do.
-	 *  Test against 0 as that represents PAGE_REPORTING_IDLE.
+	 * If reporting is already active there is nothing we need to do.
+	 * Test against 0 as that represents PAGE_REPORTING_IDLE.
 	 */
 	state = atomic_xchg(&prdev->state, PAGE_REPORTING_REQUESTED);
 	if (state != PAGE_REPORTING_IDLE)
-- 
2.23.0

[PATCH v5 2/4] mm/page_reporting: Export reporting order as module parameter

[Gavin Shan]

The macro PAGE_REPORTING_MIN_ORDER is defined as the page reporting
threshold. It can't be adjusted at runtime.

This introduces a variable (@page_reporting_order) to replace the
marcro (PAGE_REPORTING_MIN_ORDER). MAX_ORDER is assigned to it at
the beginning, meaning the page reporting is disabled. It will be
restored to @pageblock_order when page reporting device is registered.
It's also exported so that the page reporting order can be adjusted
at runtime.

Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
---
v5: Restore @page_reporting_order to @pageblock_order when device
    is registered
---
 Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
 mm/page_reporting.c                             | 13 +++++++++++--
 mm/page_reporting.h                             |  5 ++---
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index cb89dbdedc46..566c4b9af3cd 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3566,6 +3566,12 @@
 			off: turn off poisoning (default)
 			on: turn on poisoning
 
+	page_reporting.page_reporting_order=
+			[KNL] Minimal page reporting order
+			Format: <integer>
+			Adjust the minimal page reporting order. The page
+			reporting is disabled when it exceeds (MAX_ORDER-1).
+
 	panic=		[KNL] Kernel behaviour on panic: delay <timeout>
 			timeout > 0: seconds before rebooting
 			timeout = 0: wait forever
diff --git a/mm/page_reporting.c b/mm/page_reporting.c
index df9c5054e1b4..133222234f25 100644
--- a/mm/page_reporting.c
+++ b/mm/page_reporting.c
@@ -4,12 +4,17 @@
 #include <linux/page_reporting.h>
 #include <linux/gfp.h>
 #include <linux/export.h>
+#include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/scatterlist.h>
 
 #include "page_reporting.h"
 #include "internal.h"
 
+unsigned int page_reporting_order = MAX_ORDER;
+module_param(page_reporting_order, uint, 0644);
+MODULE_PARM_DESC(page_reporting_order, "Set page reporting order");
+
 #define PAGE_REPORTING_DELAY	(2 * HZ)
 static struct page_reporting_dev_info __rcu *pr_dev_info __read_mostly;
 
@@ -229,7 +234,7 @@ page_reporting_process_zone(struct page_reporting_dev_info *prdev,
 
 	/* Generate minimum watermark to be able to guarantee progress */
 	watermark = low_wmark_pages(zone) +
-		    (PAGE_REPORTING_CAPACITY << PAGE_REPORTING_MIN_ORDER);
+		    (PAGE_REPORTING_CAPACITY << page_reporting_order);
 
 	/*
 	 * Cancel request if insufficient free memory or if we failed
@@ -239,7 +244,7 @@ page_reporting_process_zone(struct page_reporting_dev_info *prdev,
 		return err;
 
 	/* Process each free list starting from lowest order/mt */
-	for (order = PAGE_REPORTING_MIN_ORDER; order < MAX_ORDER; order++) {
+	for (order = page_reporting_order; order < MAX_ORDER; order++) {
 		for (mt = 0; mt < MIGRATE_TYPES; mt++) {
 			/* We do not pull pages from the isolate free list */
 			if (is_migrate_isolate(mt))
@@ -324,6 +329,10 @@ int page_reporting_register(struct page_reporting_dev_info *prdev)
 		goto err_out;
 	}
 
+	/* Update the page reporting order with @pageblock_order */
+	if (page_reporting_order > pageblock_order)
+		page_reporting_order = pageblock_order;
+
 	/* initialize state and work structures */
 	atomic_set(&prdev->state, PAGE_REPORTING_IDLE);
 	INIT_DELAYED_WORK(&prdev->work, &page_reporting_process);
diff --git a/mm/page_reporting.h b/mm/page_reporting.h
index 2c385dd4ddbd..c51dbc228b94 100644
--- a/mm/page_reporting.h
+++ b/mm/page_reporting.h
@@ -10,10 +10,9 @@
 #include <linux/pgtable.h>
 #include <linux/scatterlist.h>
 
-#define PAGE_REPORTING_MIN_ORDER	pageblock_order
-
 #ifdef CONFIG_PAGE_REPORTING
 DECLARE_STATIC_KEY_FALSE(page_reporting_enabled);
+extern unsigned int page_reporting_order;
 void __page_reporting_notify(void);
 
 static inline bool page_reported(struct page *page)
@@ -38,7 +37,7 @@ static inline void page_reporting_notify_free(unsigned int order)
 		return;
 
 	/* Determine if we have crossed reporting threshold */
-	if (order < PAGE_REPORTING_MIN_ORDER)
+	if (order < page_reporting_order)
 		return;
 
 	/* This will add a few cycles, but should be called infrequently */
-- 
2.23.0

[PATCH v5 3/4] mm/page_reporting: Allow driver to specify reporting

[Gavin Shan]

The page reporting order (threshold) is sticky to @pageblock_order
by default. The page reporting can never be triggered because the
freeing page can't come up with a free area like that huge. The
situation becomes worse when the system memory becomes heavily
fragmented.

For example, the following configurations are used on ARM64 when 64KB
base page size is enabled. In this specific case, the page reporting
won't be triggered until the freeing page comes up with a 512MB free
area. That's hard to be met, especially when the system memory becomes
heavily fragmented.

   PAGE_SIZE:          64KB
   HPAGE_SIZE:         512MB
   pageblock_order:    13       (512MB)
   MAX_ORDER:          14

This allows the drivers to specify the page reporting order when the
page reporting device is registered. It falls back to @pageblock_order
if it's not specified by the driver. The existing users (hv_balloon
and virtio_balloon) don't specify it and @pageblock_order is still
taken as their page reporting order. So this shouldn't introduce any
functional changes.

Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
---
v5: Restore @page_reporting_order to @pageblock_order when device
    is registered and minimal page reporting order isn't provided
---
 include/linux/page_reporting.h |  3 +++
 mm/page_reporting.c            | 11 ++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/page_reporting.h b/include/linux/page_reporting.h
index 3b99e0ec24f2..fe648dfa3a7c 100644
--- a/include/linux/page_reporting.h
+++ b/include/linux/page_reporting.h
@@ -18,6 +18,9 @@ struct page_reporting_dev_info {
 
 	/* Current state of page reporting */
 	atomic_t state;
+
+	/* Minimal order of page reporting */
+	unsigned int order;
 };
 
 /* Tear-down and bring-up for page reporting devices */
diff --git a/mm/page_reporting.c b/mm/page_reporting.c
index 133222234f25..077925cfcc06 100644
--- a/mm/page_reporting.c
+++ b/mm/page_reporting.c
@@ -319,6 +319,7 @@ DEFINE_STATIC_KEY_FALSE(page_reporting_enabled);
 
 int page_reporting_register(struct page_reporting_dev_info *prdev)
 {
+	unsigned int order;
 	int err = 0;
 
 	mutex_lock(&page_reporting_mutex);
@@ -329,9 +330,13 @@ int page_reporting_register(struct page_reporting_dev_info *prdev)
 		goto err_out;
 	}
 
-	/* Update the page reporting order with @pageblock_order */
-	if (page_reporting_order > pageblock_order)
-		page_reporting_order = pageblock_order;
+	/*
+	 * Update the page reporting order if it's specified by driver.
+	 * Otherwise, it falls back to @pageblock_order.
+	 */
+	order = prdev->order ? : pageblock_order;
+	if (page_reporting_order > order)
+		page_reporting_order = order;
 
 	/* initialize state and work structures */
 	atomic_set(&prdev->state, PAGE_REPORTING_IDLE);
-- 
2.23.0

[PATCH v5 4/4] virtio_balloon: Specify page reporting order if needed

[Gavin Shan]

The page reporting won't be triggered if the freeing page can't come
up with a free area, whose size is equal or bigger than the threshold
(page reporting order). The default page reporting order, equal to
@pageblock_order, is too huge on some architectures to trigger page
reporting. One example is ARM64 when 64KB base page size is used.

      PAGE_SIZE:          64KB
      pageblock_order:    13       (512MB)
      MAX_ORDER:          14

This specifies the page reporting order to 5 (2MB) for this specific
case so that page reporting can be triggered.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
---
 drivers/virtio/virtio_balloon.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 510e9318854d..47dce91f788c 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -993,6 +993,23 @@ static int virtballoon_probe(struct virtio_device *vdev)
 			goto out_unregister_oom;
 		}
 
+		/*
+		 * The default page reporting order is @pageblock_order, which
+		 * corresponds to 512MB in size on ARM64 when 64KB base page
+		 * size is used. The page reporting won't be triggered if the
+		 * freeing page can't come up with a free area like that huge.
+		 * So we specify the page reporting order to 5, corresponding
+		 * to 2MB. It helps to avoid THP splitting if 4KB base page
+		 * size is used by host.
+		 *
+		 * Ideally, the page reporting order is selected based on the
+		 * host's base page size. However, it needs more work to report
+		 * that value. The hard-coded order would be fine currently.
+		 */
+#if defined(CONFIG_ARM64) && defined(CONFIG_ARM64_64K_PAGES)
+		vb->pr_dev_info.order = 5;
+#endif
+
 		err = page_reporting_register(&vb->pr_dev_info);
 		if (err)
 			goto out_unregister_oom;
-- 
2.23.0

Re: [PATCH v5 0/4] mm/page_reporting: Make page reporting work on arm64 with 64KB page size

[Michael S. Tsirkin]

On Fri, Jun 25, 2021 at 12:21:46PM +0800, Gavin Shan 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/4] Fix some coding style in __page_reporting_request().
>    PATCH[2/4] Represents page reporting order with variable so that it can
>               be exported as module parameter.
>    PATCH[3/4] Allows the device driver (e.g. virtio_balloon) to specify
>               the page reporting order when the device info is registered.
>    PATCH[4/4] Specifies the page reporting order to 5, corresponding to
>               2MB in size on ARM64 when 64KB base page size is used.

I sent comments on v4. They still apply I think. Want me to repeat them
here?

> Changelog
> =========
> v5:
>    * Restore @page_reporting_order to @pageblock_order when
>      device is registered in PATCH[2/4] to keep "git bisect"
>      friendly at least.                                           (Alex)
> v4:
>    * Set @page_reporting_order to MAX_ORDER. Its value is
>      specified by the driver or falls back to @pageblock_order
>      when page reporting device is registered.                    (Alex)
>    * Include "module.h" in page_reporting.c                       (Andrew)
> v3:
>    * Avoid overhead introduced by function all                    (Alex)
>    * Export page reporting order as module parameter              (Gavin)
> v2:
>    * Rewrite the patches as Alex suggested                        (Alex)
> 
> Gavin Shan (4):
>   mm/page_reporting: Fix code style in __page_reporting_request()
>   mm/page_reporting: Export reporting order as module parameter
>   mm/page_reporting: Allow driver to specify reporting
>   virtio_balloon: Specify page reporting order if needed
> 
>  .../admin-guide/kernel-parameters.txt         |  6 +++++
>  drivers/virtio/virtio_balloon.c               | 17 ++++++++++++++
>  include/linux/page_reporting.h                |  3 +++
>  mm/page_reporting.c                           | 22 +++++++++++++++----
>  mm/page_reporting.h                           |  5 ++---
>  5 files changed, 46 insertions(+), 7 deletions(-)
> 
> -- 
> 2.23.0

Re: [PATCH v5 0/4] mm/page_reporting: Make page reporting work on arm64 with 64KB page size

[Gavin Shan]

On 6/25/21 3:58 PM, Michael S. Tsirkin wrote:
> On Fri, Jun 25, 2021 at 12:21:46PM +0800, Gavin Shan 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/4] Fix some coding style in __page_reporting_request().
>>     PATCH[2/4] Represents page reporting order with variable so that it can
>>                be exported as module parameter.
>>     PATCH[3/4] Allows the device driver (e.g. virtio_balloon) to specify
>>                the page reporting order when the device info is registered.
>>     PATCH[4/4] Specifies the page reporting order to 5, corresponding to
>>                2MB in size on ARM64 when 64KB base page size is used.
> 
> I sent comments on v4. They still apply I think. Want me to repeat them
> here?
> 

Thanks for your comments, Michael. I've replied to your comments
through v4. There are some future work as Alex and David pointed
out before: In order to remove the hack in virtio-memballoon, the
VMM needs to report the page reporting order. I will keep working
on this when I have spare time.

Thanks,
Gavin

>> Changelog
>> =========
>> v5:
>>     * Restore @page_reporting_order to @pageblock_order when
>>       device is registered in PATCH[2/4] to keep "git bisect"
>>       friendly at least.                                           (Alex)
>> v4:
>>     * Set @page_reporting_order to MAX_ORDER. Its value is
>>       specified by the driver or falls back to @pageblock_order
>>       when page reporting device is registered.                    (Alex)
>>     * Include "module.h" in page_reporting.c                       (Andrew)
>> v3:
>>     * Avoid overhead introduced by function all                    (Alex)
>>     * Export page reporting order as module parameter              (Gavin)
>> v2:
>>     * Rewrite the patches as Alex suggested                        (Alex)
>>
>> Gavin Shan (4):
>>    mm/page_reporting: Fix code style in __page_reporting_request()
>>    mm/page_reporting: Export reporting order as module parameter
>>    mm/page_reporting: Allow driver to specify reporting
>>    virtio_balloon: Specify page reporting order if needed
>>
>>   .../admin-guide/kernel-parameters.txt         |  6 +++++
>>   drivers/virtio/virtio_balloon.c               | 17 ++++++++++++++
>>   include/linux/page_reporting.h                |  3 +++
>>   mm/page_reporting.c                           | 22 +++++++++++++++----
>>   mm/page_reporting.h                           |  5 ++---
>>   5 files changed, 46 insertions(+), 7 deletions(-)
>>
>> -- 
>> 2.23.0
> 

Re: [PATCH v5 0/4] mm/page_reporting: Make page reporting work on arm64 with 64KB page size

[Alexander Duyck]

On Thu, Jun 24, 2021 at 7:20 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/4] Fix some coding style in __page_reporting_request().
>    PATCH[2/4] Represents page reporting order with variable so that it can
>               be exported as module parameter.
>    PATCH[3/4] Allows the device driver (e.g. virtio_balloon) to specify
>               the page reporting order when the device info is registered.
>    PATCH[4/4] Specifies the page reporting order to 5, corresponding to
>               2MB in size on ARM64 when 64KB base page size is used.
>
> Changelog
> =========
> v5:
>    * Restore @page_reporting_order to @pageblock_order when
>      device is registered in PATCH[2/4] to keep "git bisect"
>      friendly at least.                                           (Alex)

These latest changes address the concerns I had.

Thanks.

- Alex

Re: [PATCH v5 0/4] mm/page_reporting: Make page reporting work on arm64 with 64KB page size

[Gavin Shan]

On 6/26/21 12:18 AM, Alexander Duyck wrote:
> On Thu, Jun 24, 2021 at 7:20 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/4] Fix some coding style in __page_reporting_request().
>>     PATCH[2/4] Represents page reporting order with variable so that it can
>>                be exported as module parameter.
>>     PATCH[3/4] Allows the device driver (e.g. virtio_balloon) to specify
>>                the page reporting order when the device info is registered.
>>     PATCH[4/4] Specifies the page reporting order to 5, corresponding to
>>                2MB in size on ARM64 when 64KB base page size is used.
>>
>> Changelog
>> =========
>> v5:
>>     * Restore @page_reporting_order to @pageblock_order when
>>       device is registered in PATCH[2/4] to keep "git bisect"
>>       friendly at least.                                           (Alex)
> 
> These latest changes address the concerns I had.
> 

Thanks again for your review, Alex. However, v4 was merged and it's fine
since v5 only resolves 'git-bisect' friendly issue on PATCH[v4 2/4].

Thanks,
Gavin