* Re: [PATCH v2] mm/page_reporting: Add page_reporting_delay_ms sysctl
2026-07-22 21:15 [PATCH v2] mm/page_reporting: Add page_reporting_delay_ms sysctl pratmal
@ 2026-07-22 22:54 ` Andrew Morton
2026-07-23 0:06 ` SJ Park
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Morton @ 2026-07-22 22:54 UTC (permalink / raw)
To: pratmal
Cc: Anshuman Khandual, David Hildenbrand, Vlastimil Babka,
Greg Thelen, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, linux-mm, linux-kernel
On Wed, 22 Jul 2026 21:15:17 +0000 pratmal@google.com wrote:
> From: Pratyush Mallick <pratmal@google.com>
>
> Currently, the free page reporting daemon uses a hardcoded delay of
> (2 HZ) between reporting intervals. While this is a reasonable
> default, it lacks the flexibility to adapt to varying guest workloads.
>
> A low delay allows aggressive memory reclamation, returning unused
> pages to the host as quickly as possible. However, during spiky
> allocation/free churn, this immediate reporting can lead to a severe
> performance penalty (nested page faults) as the guest re-allocates memory
> that the host has just unmapped. In these scenarios, there is benefit
> from increasing the delay to batch free pages over a longer window,
> absorbing the churn without hypercall and re-fault overhead.
>
> This patch refactors the delay into a dynamically tunable sysctl,
> /proc/sys/vm/page_reporting_delay_ms, measured in milliseconds. The value
> defaults to 2000ms to precisely match the original (2 HZ) behavior.
>
Seems reasonable.
> +The default value is 2000 (2 seconds). The minimum allowed value is
> +0 (immediate reporting) and the maximum allowed value is 10000 (10 seconds).
Why implement a max? If setting it to something enormous causes bad
behavior then Don't Do That?
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] mm/page_reporting: Add page_reporting_delay_ms sysctl
2026-07-22 21:15 [PATCH v2] mm/page_reporting: Add page_reporting_delay_ms sysctl pratmal
2026-07-22 22:54 ` Andrew Morton
@ 2026-07-23 0:06 ` SJ Park
1 sibling, 0 replies; 3+ messages in thread
From: SJ Park @ 2026-07-23 0:06 UTC (permalink / raw)
To: pratmal
Cc: SJ Park, Anshuman Khandual, David Hildenbrand, Andrew Morton,
Vlastimil Babka, Greg Thelen, Suren Baghdasaryan, Michal Hocko,
Brendan Jackman, Johannes Weiner, Zi Yan, linux-mm, linux-kernel,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Mike Rapoport, Jonathan Corbet, Shuah Khan, linux-doc
get_maintainer.pl suggests adding below to the recipients list of this patch.
Let me add them. I show you already Cc-ed David, but with his old email
address.
- David Hildenbrand <david@kernel.org>
- Lorenzo Stoakes <ljs@kernel.org>
- "Liam R. Howlett" <liam@infradead.org>
- Mike Rapoport <rppt@kernel.org>
- Jonathan Corbet <corbet@lwn.net>
- Shuah Khan <skhan@linuxfoundation.org>
- linux-doc@vger.kernel.org
On Wed, 22 Jul 2026 21:15:17 +0000 pratmal@google.com wrote:
> From: Pratyush Mallick <pratmal@google.com>
>
> Currently, the free page reporting daemon uses a hardcoded delay of
> (2 HZ) between reporting intervals. While this is a reasonable
> default, it lacks the flexibility to adapt to varying guest workloads.
>
> A low delay allows aggressive memory reclamation, returning unused
> pages to the host as quickly as possible. However, during spiky
> allocation/free churn, this immediate reporting can lead to a severe
> performance penalty (nested page faults) as the guest re-allocates memory
> that the host has just unmapped. In these scenarios, there is benefit
> from increasing the delay to batch free pages over a longer window,
> absorbing the churn without hypercall and re-fault overhead.
>
> This patch refactors the delay into a dynamically tunable sysctl,
> /proc/sys/vm/page_reporting_delay_ms, measured in milliseconds. The value
> defaults to 2000ms to precisely match the original (2 HZ) behavior.
>
> Signed-off-by: Pratyush Mallick <pratmal@google.com>
> ---
> v2:
> - Documented page_reporting_delay_ms in Documentation/admin-guide/sysctl/vm.rst.
> - v1: https://lore.kernel.org/linux-mm/20260722192935.1646848-1-pratmal@google.com/T/#u
> v1: Fixed feedback from RFC.
> - Added lower and upper cap to sysctl value.
> - Reverted the reordering on page_reporting_delay_ms.
> - Dropped the mod_delayed_work() change.
> - RFC: https://lore.kernel.org/linux-mm/20260714171456.2350037-1-pratmal@google.com/T/#u
> Documentation/admin-guide/sysctl/vm.rst | 13 +++++++++++
> mm/page_reporting.c | 30 ++++++++++++++++++++++---
> 2 files changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/admin-guide/sysctl/vm.rst b/Documentation/admin-guide/sysctl/vm.rst
> index b9b0c218bfb4..6efa460b4547 100644
> --- a/Documentation/admin-guide/sysctl/vm.rst
> +++ b/Documentation/admin-guide/sysctl/vm.rst
> @@ -66,6 +66,7 @@ Currently, these files are in /proc/sys/vm:
> - overcommit_ratio
> - page-cluster
> - page_lock_unfairness
> +- page_reporting_delay
> - panic_on_oom
> - percpu_pagelist_high_fraction
> - stat_interval
> @@ -896,6 +897,18 @@ stolen from under a waiter. After the lock is stolen the number of times
> specified in this file (default is 5), the "fair lock handoff" semantics
> will apply, and the waiter will only be awakened if the lock can be taken.
>
> +page_reporting_delay
> +=======================
Why don't you make the lengths of the line and the section title same?
> +
> +This value determines the delay in milliseconds between free page
> +reporting intervals. A lower delay allows aggressive memory
> +reclamation by returning unused pages to the host quickly, while a
> +higher delay helps to batch free pages over a longer window, absorbing
> +allocation/free churn without hypercall and re-fault overhead.
> +
> +The default value is 2000 (2 seconds). The minimum allowed value is
> +0 (immediate reporting) and the maximum allowed value is 10000 (10 seconds).
So it is milliseconds. Why don't you add the unit to the name, like
page_reporting_delay_ms?
Also, coule you elaborate why there is 10 seconds maximum limit? What's the
problem of having no limit, and why 10 seconds is the reasonable one?
> +
> panic_on_oom
> ============
>
> diff --git a/mm/page_reporting.c b/mm/page_reporting.c
> index 942e84b6908a..805da4bc1101 100644
> --- a/mm/page_reporting.c
> +++ b/mm/page_reporting.c
> @@ -6,6 +6,7 @@
> #include <linux/export.h>
> #include <linux/module.h>
> #include <linux/delay.h>
> +#include <linux/sysctl.h>
> #include <linux/scatterlist.h>
Too trivial nit, but, why don't you put the sysctl.h at the end of the list?
>
> #include "page_reporting.h"
> @@ -47,7 +48,10 @@ MODULE_PARM_DESC(page_reporting_order, "Set page reporting order");
> */
> EXPORT_SYMBOL_GPL(page_reporting_order);
>
> -#define PAGE_REPORTING_DELAY (2 * HZ)
> +#define PAGE_REPORTING_DELAY_MS_MAX (10 * MSEC_PER_SEC)
> +
> +static unsigned int page_reporting_delay_ms = 2 * MSEC_PER_SEC;
> +static unsigned int page_reporting_delay_ms_max = PAGE_REPORTING_DELAY_MS_MAX;
> static struct page_reporting_dev_info __rcu *pr_dev_info __read_mostly;
>
> enum {
> @@ -56,6 +60,19 @@ enum {
> PAGE_REPORTING_ACTIVE
> };
>
> +
> +static struct ctl_table page_reporting_sysctls[] = {
> + {
> + .procname = "page_reporting_delay",
> + .data = &page_reporting_delay_ms,
> + .maxlen = sizeof(unsigned int),
> + .mode = 0644,
> + .proc_handler = proc_douintvec_minmax,
> + .extra1 = SYSCTL_ZERO,
> + .extra2 = &page_reporting_delay_ms_max,
> + },
> +};
> +
> /* request page reporting */
> static void
> __page_reporting_request(struct page_reporting_dev_info *prdev)
> @@ -80,7 +97,7 @@ __page_reporting_request(struct page_reporting_dev_info *prdev)
> * now we are limiting this to running no more than once every
> * couple of seconds.
> */
> - schedule_delayed_work(&prdev->work, PAGE_REPORTING_DELAY);
> + schedule_delayed_work(&prdev->work, msecs_to_jiffies(page_reporting_delay_ms));
> }
A trivial comment again. Why don't you wrap the above line for 80 columns
limit? I know that's not a hard limit anymore and I show a few lines of this
file already exceeds 80 columns. But seems most lines of this file is still
keeping the 80 columns limit?
>
> /* notify prdev of free page reporting request */
> @@ -340,7 +357,7 @@ static void page_reporting_process(struct work_struct *work)
> */
> state = atomic_cmpxchg(&prdev->state, state, PAGE_REPORTING_IDLE);
> if (state == PAGE_REPORTING_REQUESTED)
> - schedule_delayed_work(&prdev->work, PAGE_REPORTING_DELAY);
> + schedule_delayed_work(&prdev->work, msecs_to_jiffies(page_reporting_delay_ms));
Ditto.
> }
>
> static DEFINE_MUTEX(page_reporting_mutex);
> @@ -416,3 +433,10 @@ void page_reporting_unregister(struct page_reporting_dev_info *prdev)
> mutex_unlock(&page_reporting_mutex);
> }
> EXPORT_SYMBOL_GPL(page_reporting_unregister);
> +
> +static int __init page_reporting_sysctl_init(void)
> +{
> + register_sysctl_init("vm", page_reporting_sysctls);
> + return 0;
> +}
> +late_initcall(page_reporting_sysctl_init);
> --
> 2.55.0.229.g6434b31f56-goog
Thanks,
SJ
^ permalink raw reply [flat|nested] 3+ messages in thread