Re: [PATCH v10 23/37] mm: page_alloc: skip kernel_init_pages for FPI_ZEROED when safe

[Lorenzo Stoakes <ljs@kernel.org>]

On Mon, Jun 08, 2026 at 04:38:46AM -0400, Michael S. Tsirkin wrote:
> In __free_pages_prepare(), when FPI_ZEROED is set the page is already
> known to be zero. We can skip kernel_init_pages() if page poisoning is
> not enabled (because poison would overwrite the zeroes).
>
> This avoids redundant zeroing work when freeing pages that are already
> known to contain all zeros.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---
>  mm/page_alloc.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 008f1a311c40..e3a7c40c769c 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1443,7 +1443,14 @@ __always_inline bool __free_pages_prepare(struct page *page,
>  		if (kasan_has_integrated_init())
>  			init = false;
>  	}
> -	if (init)
> +	/*
> +	 * Skip redundant zeroing when the page is already known-zero
> +	 * (FPI_ZEROED) and page poisoning did not overwrite it.
> +	 * When page_poisoning is enabled, kernel_poison_pages above
> +	 * wrote PAGE_POISON (0xAA), so we must re-zero.
> +	 */

Again, please stop specifying arbitrary hex values in comments, this seems
mostly 'describing what we do here'.

Maybe drop to just e.g.:

/* if poisoned or not zeroed by a virtualised host, zero now. */

or suchlike?

> +	if (init && !((fpi_flags & FPI_ZEROED) &&
> +		      !page_poisoning_enabled_static()))

This condition is absolutely horrible, !(X && !Y), you're making life difficult
for the readers.

'if not both zeroed and not poisoned' is how that reads logically. Which is hard
to understand.

De Morgan's law gives us -> !zeroed || posioned

How about:

	if (init && (!(fpi_flags & FPI_ZEROED) || page_poisoning_enabled_static())

Or preferably something like:

	const bool poisoned = page_poisoning_enabled_static();
	const bool vm_host_zeroed = fpi_flags & FPI_ZEROED;

	...

	if (init && (poisoned || !vm_host_zeroed))
		...

?


>  		kernel_init_pages(page, 1 << order);
>
>  	/*
> --
> MST
>

Thanks, Lorenzo