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

[Pratyush Mallick]

> Why don't you make the lengths of the line and the section title same?
Will fix the RST underline length.

> So it is milliseconds. Why don't you add the unit to the name, like
> page_reporting_delay_ms?
Agreed, I'll rename the sysctl to page_reporting_delay_ms.
Also I think it would better to make this a module param to be consistent
with page_reporting_order.

> 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?
I don't have a strong justification for where the maximum limit should be set.
The value is unsigned int, so it's bounded at UINT_MAX ms (~49.7 days)
So at worst the worker just schedules that far out and reporting
is effectively paused for that window.
I'll drop this in V3.

> Too trivial nit, but, why don't you put the sysctl.h at the end of the list?
Apologies. Will fix this.

> A trivial comment again. Why don't you wrap the above line for 80 columns
> limit? [...] seems most lines of this file is still keeping the 80 columns limit?
Apologies. I will fix this as well.

Thanks for the feedback.

Thanks,
Pratyush

On Wed, Jul 22, 2026 at 5:06 PM SJ Park <sj@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx>
> - Lorenzo Stoakes <ljs@xxxxxxxxxx>
> - "Liam R. Howlett" <liam@xxxxxxxxxxxxx>
> - Mike Rapoport <rppt@xxxxxxxxxx>
> - Jonathan Corbet <corbet@xxxxxxx>
> - Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>
> - linux-doc@xxxxxxxxxxxxxxx
>
> On Wed, 22 Jul 2026 21:15:17 +0000 pratmal@xxxxxxxxxx wrote:
>
> > From: Pratyush Mallick <pratmal@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
> > v2:
> >  - Documented page_reporting_delay_ms in Documentation/admin-guide/sysctl/vm.rst.
> >  - v1: https://lore.kernel.org/linux-mm/20260722192935.1646848-1-pratmal@xxxxxxxxxx/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@xxxxxxxxxx/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