Re: [PATCH v3] mm/page_reporting: Add page_reporting_delay_ms module parameter

[David Hildenbrand (Arm)]

On 7/28/26 13:13, Lorenzo Stoakes (ARM) wrote:
> On Mon, Jul 27, 2026 at 11:05:45PM +0000, pratmal@xxxxxxxxxx wrote:
>> From: Pratyush Mallick <pratmal@xxxxxxxxxx>
>>
>> Currently, the free page reporting 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.
> 
> Since you're talking about increasing it, maybe set the floor at the current
> value of 2s?
> 
>>
>> This patch exposes the delay as a module parameter:
>> /sys/module/page_reporting/parameters/page_reporting_delay_ms, measured
>> in milliseconds and defaults to 2000ms.
> 
> I'm not sure this is great as it means we now have a parameter we have to
> support forever and autotuning becomes harder to implement, also if later the
> implementation is changed, this might prevent a reimplementation.

While I'd prefer to keep the implementation as simple as possible unless really
required, I guess auto-tune could be enabled by setting the parameter to e.g.,
-1 in the future.

> 
> Have you considered actually adding logic to detect the problem you're having
> and delay in that case?
> 
> It seems like this is the "easy" solution but it has a price too.
> 
>>
>> Signed-off-by: Pratyush Mallick <pratmal@xxxxxxxxxx>
>> ---
>> v3:
>>  - Converted page_reporting_delay_ms from a sysctl to a module parameter.
>>  - Dropped the max value cap (PAGE_REPORTING_DELAY_MS_MAX).
>>  - Documented page_reporting_delay_ms in kernel-parameters.txt.
>>  - Updated code comments in mm/page_reporting.c.
>>  - v2: https://lore.kernel.org/linux-mm/3da27fde-25dc-4cb8-8e05-74cd26fc2f7c@xxxxxxxxxx/T/#t
>>
>> 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
>>  .../admin-guide/kernel-parameters.txt         |  6 ++++++
>>  mm/page_reporting.c                           | 21 ++++++++++++-------
>>  2 files changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index b5493a7f8f22..364c2dce8e70 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -4810,6 +4810,12 @@ Kernel parameters
>>  			Adjust the minimal page reporting order. The page
>>  			reporting is disabled when it exceeds MAX_PAGE_ORDER.
>>
>> +	page_reporting.page_reporting_delay_ms=
>> +			[KNL] Free page reporting delay in milliseconds
>> +			Format: <unsigned integer>
>> +			Adjust the delay in milliseconds between free page
>> +			reporting intervals. Default is 2000 (2 seconds).
>> +
>>  	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 942e84b6908a..a67311468204 100644
>> --- a/mm/page_reporting.c
>> +++ b/mm/page_reporting.c
>> @@ -47,7 +47,11 @@ MODULE_PARM_DESC(page_reporting_order, "Set page reporting order");
>>   */
>>  EXPORT_SYMBOL_GPL(page_reporting_order);
>>
>> -#define PAGE_REPORTING_DELAY	(2 * HZ)
>> +static unsigned int page_reporting_delay_ms = 2 * MSEC_PER_SEC;
>> +module_param(page_reporting_delay_ms, uint, 0644);
>> +MODULE_PARM_DESC(page_reporting_delay_ms,
>> +		 "Set page reporting delay in milliseconds");
>> +
>>  static struct page_reporting_dev_info __rcu *pr_dev_info __read_mostly;
>>
>>  enum {
>> @@ -76,11 +80,11 @@ __page_reporting_request(struct page_reporting_dev_info *prdev)
>>  		return;
>>
>>  	/*
>> -	 * Delay the start of work to allow a sizable queue to build. For
>> -	 * now we are limiting this to running no more than once every
>> -	 * couple of seconds.
>> +	 * Delay the start of work to allow a sizable queue to build.
>> +	 * We limit this based on page_reporting_delay_ms.
>>  	 */
>> -	schedule_delayed_work(&prdev->work, PAGE_REPORTING_DELAY);
>> +	schedule_delayed_work(&prdev->work,
>> +			      msecs_to_jiffies(page_reporting_delay_ms));
> 
> Err, do we not want to limit this to something sensible? What if the user
> specifies 0 does it just hammer the system at that stage?

IIUC, 0 just means "as soon there is a suitable free page block to report, start
reporting immediately".

With 2s, we wait 2s before we start reporting immediately by kicking the
workqueue immediately.

So "0" does not mean "report all the time", rather "start reporting immediately
as we are notified about a pageblock to report".

-- 
Cheers,

David