Re: [PATCH v2] mm/page_reporting: Add page_reporting_delay_ms sysctl

[SJ Park <sj@kernel.org>]

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