From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 5EEA31A316E for ; Thu, 16 Jul 2026 03:30:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784172657; cv=none; b=ihltuIeOxa+2clShumkzvMh6Mr1wdAdobQWuXHZFlVZvvJWs/xZpVrvzLe2T+zJi6h6kUfJlbXwknjyjWlmcuko6BIb0LLWva4Nh5mfbKA8CGZgRMAdY9i+IG2AD+XP6kOEGZJnMpUAbQvn+nT5qGJ3KUl7ACwElt8cFPN9mKiY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784172657; c=relaxed/simple; bh=ThXP+Q7LXDEvi/EDQPIrwlZ2l1puzjqadS3wm1onWSk=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=czRvHLTonBV83BHiKV3auNbvBKnuOfy9CyaeHw2KJ+/7QxW/kx+LPe54PnLwlh1A16EgSsPfiDqAf3vutvI/B8IAr9XLxD44U9UnNtSR8FKbMRvodtFhcrwwlhKAeRfjxp1w/ILiGTQIKbNT+wTTNXuJQl+txbLzVsqLMQtA3Fg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=p3RdnhzC; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="p3RdnhzC" Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 073E11477; Wed, 15 Jul 2026 20:30:49 -0700 (PDT) Received: from [10.163.164.156] (unknown [10.163.164.156]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C5F223F905; Wed, 15 Jul 2026 20:30:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1784172653; bh=ThXP+Q7LXDEvi/EDQPIrwlZ2l1puzjqadS3wm1onWSk=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=p3RdnhzCPX5asAhYRSsFO5Oid36EY5U9G60iVOKLE2WyGHmsY1xiTx1/6UVsXIDEV F1AWLPxIyPE8Xb6RJf9enhXL8wpCkslQ5xly1oFEQ0ZOYFiKGaaq8fYFk7/HwIOJto mIvwVKa9hAbG8tLmZ0ybh4P6668wkZYr4vc2o3mE= Message-ID: <03f6d16c-0f1c-4123-abb4-81a73eecaff1@arm.com> Date: Thu, 16 Jul 2026 09:00:46 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH] mm/page_reporting: Add page_reporting_delay sysctl To: "David Hildenbrand (Arm)" , pratmal@google.com, Andrew Morton , Vlastimil Babka Cc: Greg Thelen , Suren Baghdasaryan , Michal Hocko , Brendan Jackman , Johannes Weiner , Zi Yan , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20260714171456.2350037-1-pratmal@google.com> <70392f94-137a-4220-8ff5-64dd874cf5d9@kernel.org> Content-Language: en-US From: Anshuman Khandual In-Reply-To: <70392f94-137a-4220-8ff5-64dd874cf5d9@kernel.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 15/07/26 6:29 PM, David Hildenbrand (Arm) wrote: > On 7/14/26 19:14, pratmal@google.com wrote: >> From: Pratyush Mallick >> >> 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 hypervisor 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 hypervisor 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, measured in milliseconds. The value >> defaults to 2000ms to precisely match the original (2 HZ) behavior. >> If the sysctl is modified across reporting windows, the sysctl handler >> immediately issues a mod_delayed_work() to honor the new configuration >> without waiting for the prior timeout to lapse. >> >> Signed-off-by: Pratyush Mallick >> --- >> >> Sending this as an RFC to get thoughts on exposing this delay as a sysctl. >> >> Benchmark Results: >> We kill a process allocated with 10GB memory within the VM and mesure the >> time it takes to report all the memory to host as well the delay in >> initiating the reporting. >> >> default(2s delay): https://drive.google.com/file/d/1Ouxm_raj4xPNthc_ryk0Mdbo-c_zXKiP/view?usp=sharing >> Tuned to 0s delay: https://drive.google.com/file/d/1mk58LPiYgIF4Yk6kmX5JHplhrwIw0NbC/view?usp=sharing > > [...] > > Hi! > >> >> #include "page_reporting.h" >> @@ -47,15 +48,44 @@ MODULE_PARM_DESC(page_reporting_order, "Set page reporting order"); >> */ >> EXPORT_SYMBOL_GPL(page_reporting_order); >> >> -#define PAGE_REPORTING_DELAY (2 * HZ) >> -static struct page_reporting_dev_info __rcu *pr_dev_info __read_mostly; >> - > > Why are you moving that? +1 > >> enum { >> PAGE_REPORTING_IDLE = 0, >> PAGE_REPORTING_REQUESTED, >> PAGE_REPORTING_ACTIVE >> }; >> >> +static unsigned int page_reporting_delay = 2000; > > Maybe "2 * MSEC_PER_SEC;" > > Would we want to call that page_reporting_delay_ms to make it clearer what we > are dealing with? +1 > >> +static struct page_reporting_dev_info __rcu *pr_dev_info __read_mostly; >> + >> +static int page_reporting_delay_sysctl(const struct ctl_table *table, int write, >> + void *buffer, size_t *lenp, loff_t *ppos) > > We prefer two tabs here in MM land. > >> +{ >> + int ret; >> + struct page_reporting_dev_info *prdev; >> + >> + ret = proc_dointvec(table, write, buffer, lenp, ppos); >> + if (ret < 0 || !write) >> + return ret; > > Would we want to cap it at reasonable values? Possibly with a macro PAGE_REPORTING_DELAY_MS_MAX or similar. > >> + >> + rcu_read_lock(); >> + prdev = rcu_dereference(pr_dev_info); >> + if (prdev && atomic_read(&prdev->state) == PAGE_REPORTING_REQUESTED) >> + mod_delayed_work(system_wq, &prdev->work, msecs_to_jiffies(page_reporting_delay)); >> + rcu_read_unlock(); > > Is that really required? Seems unnecessary given that we expect something in the > range of a couple of seconds max. Agreed.