Linux virtualization list
 help / color / mirror / Atom feed
* Re: [PATCH v10 18/37] mm: page_alloc: use aliasing checks instead of user_alloc_needs_zeroing
From: Lorenzo Stoakes @ 2026-06-08 11:39 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <d9b5fc983725665749024312f9ff5ede4f05e405.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:38:00AM -0400, Michael S. Tsirkin wrote:
> Replace user_alloc_needs_zeroing() with the direct aliasing checks
> (cpu_dcache_is_aliasing() || cpu_icache_is_aliasing()) in the
> post_alloc_hook aliasing guard.
>
> user_alloc_needs_zeroing() includes a !init_on_alloc term that
> means "allocator didn't zero this page."  But in this guard's
> context (!zeroed && !init && __GFP_ZERO), we already know the page
> is zero; init incorporates init_on_alloc via want_init_on_alloc().
> The only question left is whether the cache architecture needs
> the data re-zeroed through a congruent mapping, which is purely
> cpu_dcache_is_aliasing() || cpu_icache_is_aliasing().
>
> On non-aliasing architectures with init_on_free=true and
> init_on_alloc=false, this avoids a redundant re-zero of an
> already-zero page.
>
> Note on PowerPC: PowerPC overrides clear_user_page to call
> flush_dcache_page after clear_page, but on freshly allocated
> pages PG_dcache_clean is already clear (cleared by
> __free_pages_prepare), so flush_dcache_page is a no-op.
> Skipping this here thus has no effect.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6

This seems like an odd ordering of patches, can we group like changes
together?

> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 45e824b1ec75..edfc83571985 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1880,7 +1880,7 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
>  	 */
>  	if (!zeroed && !init && (gfp_flags & __GFP_ZERO) &&
>  	    user_addr != USER_ADDR_NONE &&
> -	    user_alloc_needs_zeroing())
> +	    (cpu_dcache_is_aliasing() || cpu_icache_is_aliasing()))

Let's try and simplify things rather than adding endlessly huge if conditionals?

It's now incredibly hard to track exactly what's going on here, and that is
bug-bait.

>  		init = true;
>  	/*
>  	 * If memory is still not initialized, initialize it now.
> --
> MST
>

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 16/37] mm: alloc_swap_folio: pass raw fault address to vma_alloc_folio
From: Lorenzo Stoakes @ 2026-06-08 11:37 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <d2be78654e9dcc7cf5b8a542c2c8f331187c80c9.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:37:41AM -0400, Michael S. Tsirkin wrote:
> Same change as the previous patch but for alloc_swap_folio:

Please don't say 'same change as the previous patch' :) explain what you're
doing here. It's a pain to have to go check otherwise.

> pass vmf->address directly instead of ALIGN_DOWN(vmf->address, ...).
>
> Note: NUMA interleave is not affected by the raw address;
> the ilx calculation shifts addr >> PAGE_SHIFT >> order,
> dropping sub-page bits regardless of alignment.

You're expressing the same thing as the last patch differently, but then
eliding other explanations from that?

All the same questions as I asked for the last apply to this also.

And also - if you've now made this a _requirement_ that is broken
otherwise, then aren't these bisection hazards and should be squashed into
the change?...

No patch should break anything at any point.

>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh
> ---
>  mm/memory.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 21f640674c4f..6c14b90f558e 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4750,8 +4750,7 @@ static struct folio *alloc_swap_folio(struct vm_fault *vmf)
>  	/* Try allocating the highest of the remaining orders. */
>  	gfp = vma_thp_gfp_mask(vma);
>  	while (orders) {
> -		addr = ALIGN_DOWN(vmf->address, PAGE_SIZE << order);
> -		folio = vma_alloc_folio(gfp, order, vma, addr);
> +		folio = vma_alloc_folio(gfp, order, vma, vmf->address);
>  		if (folio) {
>  			if (!mem_cgroup_swapin_charge_folio(folio, vma->vm_mm,
>  							    gfp, entry))
> --
> MST
>

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 15/37] mm: alloc_anon_folio: pass raw fault address to vma_alloc_folio
From: Lorenzo Stoakes @ 2026-06-08 11:35 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <2e931dc3daa76b57eaedd5b2d3d49f1075797252.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:37:25AM -0400, Michael S. Tsirkin wrote:
> Pass vmf->address directly instead of ALIGN_DOWN(vmf->address, ...).
> NUMA interleave is not affected: the ilx calculation in
> get_vma_policy() shifts addr >> PAGE_SHIFT >> order, which
> drops sub-page bits regardless of alignment. post_alloc_hook
> will use the raw address for cache-friendly zeroing via
> folio_zero_user().

I'm confused as to the justification for this? You're saying 'make change X,
it's safe because Y'. So the justification is now this post_alloc_hook thing.

But are you now creating a new requirement of vma_alloc_folio() that you must
specify the actual address we are faulting on, not an address within the folio
or the folio's base address?

(If that's a requirement, why is it?)

If so you should update the vma_alloc_folio() description 'virtual address of
the allocation' is not at all clear.

And if that _is_ a requirement, then are you sure all allocation paths are
correct?

I already see addr & HPAGE_PMD_MASK in vma_alloc_anon_folio_pmd() for
instance?

If it's not a requirement, why are we doing this? it's surely useless in
that case?

>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh
> ---
>  mm/memory.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 86a973119bd4..21f640674c4f 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -5268,8 +5268,7 @@ static struct folio *alloc_anon_folio(struct vm_fault *vmf)
>  	/* Try allocating the highest of the remaining orders. */
>  	gfp = vma_thp_gfp_mask(vma);
>  	while (orders) {
> -		addr = ALIGN_DOWN(vmf->address, PAGE_SIZE << order);

If you're removing this usage, could you also remove the silly thing of us
declaring this at function scope then using it in branches when we should always
have declared these separately?

> -		folio = vma_alloc_folio(gfp, order, vma, addr);
> +		folio = vma_alloc_folio(gfp, order, vma, vmf->address);
>  		if (folio) {
>  			if (mem_cgroup_charge(folio, vma->vm_mm, gfp)) {
>  				count_mthp_stat(order, MTHP_STAT_ANON_FAULT_FALLBACK_CHARGE);
> --
> MST
>

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 14/37] mm: remove arch vma_alloc_zeroed_movable_folio overrides
From: Lorenzo Stoakes @ 2026-06-08 11:29 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli, Magnus Lindholm,
	Greg Ungerer, Geert Uytterhoeven
In-Reply-To: <c374d7a35a35eb4bd5ed85f87ff6885fae605fa9.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:37:08AM -0400, Michael S. Tsirkin wrote:
> Now that the generic vma_alloc_zeroed_movable_folio() uses
> __GFP_ZERO, the arch-specific macros on alpha, m68k, s390, and
> x86 that did the same thing are redundant.  Remove them.
>
> arm64 is not affected: it has a real function override that
> handles MTE tag zeroing, not just __GFP_ZERO.
>
> Suggested-by: David Hildenbrand <david@kernel.org>
> Acked-by: Magnus Lindholm <linmag7@gmail.com>
> Acked-by: Greg Ungerer <gerg@linux-m68k.org>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> # m68k
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> Reviewed-by: Gregory Price <gourry@gourry.net>
> ---
>  arch/alpha/include/asm/page.h   | 3 ---
>  arch/m68k/include/asm/page_no.h | 3 ---
>  arch/s390/include/asm/page.h    | 3 ---
>  arch/x86/include/asm/page.h     | 3 ---
>  include/linux/highmem.h         | 8 +++++---
>  5 files changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/arch/alpha/include/asm/page.h b/arch/alpha/include/asm/page.h
> index 59d01f9b77f6..4327029cd660 100644
> --- a/arch/alpha/include/asm/page.h
> +++ b/arch/alpha/include/asm/page.h
> @@ -12,9 +12,6 @@
>
>  extern void clear_page(void *page);
>
> -#define vma_alloc_zeroed_movable_folio(vma, vaddr) \
> -	vma_alloc_folio(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, 0, vma, vaddr)
> -
>  extern void copy_page(void * _to, void * _from);
>  #define copy_user_page(to, from, vaddr, pg)	copy_page(to, from)
>
> diff --git a/arch/m68k/include/asm/page_no.h b/arch/m68k/include/asm/page_no.h
> index d2532bc407ef..f511b763a235 100644
> --- a/arch/m68k/include/asm/page_no.h
> +++ b/arch/m68k/include/asm/page_no.h
> @@ -12,9 +12,6 @@ extern unsigned long memory_end;
>
>  #define copy_user_page(to, from, vaddr, pg)	copy_page(to, from)
>
> -#define vma_alloc_zeroed_movable_folio(vma, vaddr) \
> -	vma_alloc_folio(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, 0, vma, vaddr)
> -
>  #define __pa(vaddr)		((unsigned long)(vaddr))
>  #define __va(paddr)		((void *)((unsigned long)(paddr)))
>
> diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h
> index 56da819a79e6..e995d2a413f9 100644
> --- a/arch/s390/include/asm/page.h
> +++ b/arch/s390/include/asm/page.h
> @@ -67,9 +67,6 @@ static inline void copy_page(void *to, void *from)
>
>  #define copy_user_page(to, from, vaddr, pg)	copy_page(to, from)
>
> -#define vma_alloc_zeroed_movable_folio(vma, vaddr) \
> -	vma_alloc_folio(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, 0, vma, vaddr)
> -
>  #ifdef CONFIG_STRICT_MM_TYPECHECKS
>  #define STRICT_MM_TYPECHECKS
>  #endif
> diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
> index 416dc88e35c1..92fa975b46f3 100644
> --- a/arch/x86/include/asm/page.h
> +++ b/arch/x86/include/asm/page.h
> @@ -28,9 +28,6 @@ static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
>  	copy_page(to, from);
>  }
>
> -#define vma_alloc_zeroed_movable_folio(vma, vaddr) \
> -	vma_alloc_folio(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, 0, vma, vaddr)
> -
>  #ifndef __pa
>  #define __pa(x)		__phys_addr((unsigned long)(x))
>  #endif
> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index 8b0afaabbc6e..642718a50c27 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -303,7 +303,6 @@ static inline void clear_user_highpages(struct page *page, unsigned long vaddr,
>  #endif
>  }
>
> -#ifndef vma_alloc_zeroed_movable_folio

We're specifying this function unconditionally even though arm64 overrides?

>  /**
>   * vma_alloc_zeroed_movable_folio - Allocate a zeroed page for a VMA.
>   * @vma: The VMA the page is to be allocated for.
> @@ -317,12 +316,15 @@ static inline void clear_user_highpages(struct page *page, unsigned long vaddr,
>   * we are out of memory.
>   */
>  static inline
> -struct folio *vma_alloc_zeroed_movable_folio(struct vm_area_struct *vma,
> +struct folio *vma_alloc_zeroed_movable_folio_noprof(struct vm_area_struct *vma,
>  				   unsigned long vaddr)
>  {
> -	return vma_alloc_folio(GFP_HIGHUSER_MOVABLE | __GFP_ZERO,
> +	return vma_alloc_folio_noprof(GFP_HIGHUSER_MOVABLE | __GFP_ZERO,
>  			      0, vma, vaddr);

This whole change seems unnecessary?

>  }
> +#ifndef vma_alloc_zeroed_movable_folio
> +#define vma_alloc_zeroed_movable_folio(...) \
> +	alloc_hooks(vma_alloc_zeroed_movable_folio_noprof(__VA_ARGS__))
>  #endif

I don't know why we need to add more of this alloc_hooks() dance when we could
just do:

#define vma_alloc_zeroed_movable_folio(vma, vaddr) \
	vma_alloc_folio(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, 0, vma, vaddr)

Like the existing arch stuff?

>
>  static inline void clear_highpage(struct page *page)
> --
> MST
>

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 12/37] mm: use folio_zero_user for user pages in post_alloc_hook
From: Lorenzo Stoakes @ 2026-06-08 11:23 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <f92f6f06e5804b4ea7f68b8664b7e69953b50f4e.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:36:38AM -0400, Michael S. Tsirkin wrote:
> When post_alloc_hook() needs to zero a page for an explicit
> __GFP_ZERO allocation for a user page (user_addr is set), use folio_zero_user()
> instead of kernel_init_pages().  This zeros near the faulting
> address last, keeping those cachelines hot for the impending
> user access.
>
> folio_zero_user() is only used for explicit __GFP_ZERO, not for
> init_on_alloc.  On architectures with virtually-indexed caches
> (e.g., ARM), clear_user_highpage() performs per-line cache
> operations; using it for init_on_alloc would add overhead that
> kernel_init_pages() avoids (the page fault path flushes the
> cache at PTE installation time regardless).
>
> No functional change yet: current callers do not pass __GFP_ZERO
> for user pages (they zero at the callsite instead).  Subsequent
> patches will convert them.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---
>  mm/page_alloc.c | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 4676fd49819e..d4fbf1861a8a 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1861,9 +1861,38 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
>  		for (i = 0; i != 1 << order; ++i)
>  			page_kasan_tag_reset(page + i);
>  	}
> -	/* If memory is still not initialized, initialize it now. */
> -	if (init)
> -		kernel_init_pages(page, 1 << order);
> +	/*
> +	 * On architectures with cache aliasing, pages zeroed via the
> +	 * kernel direct map (e.g. init_on_free) must be re-zeroed
> +	 * through a user-congruent mapping.  Host-zeroed pages
> +	 * (zeroed flag) don't need this: physical RAM is clean.
> +	 */
> +	if (!init && (gfp_flags & __GFP_ZERO) &&
> +	    user_addr != USER_ADDR_NONE &&
> +	    user_alloc_needs_zeroing())

We check this (gfp_flags & __GFP_ZERO) && user_addr != USER_ADDR_NONE thing
twice, can we just put in a 'init_should_folio_zero' const bool or something?

> +		init = true;

As Vlasta says not sure if we want to add complexity just for these arches.

> +	/*
> +	 * If memory is still not initialized, initialize it now.

I kinda hate that 'init' is unclear as to 'do init' or 'was init somewhere
else'... Anwyay.

> +	 * When __GFP_ZERO was explicitly requested and user_addr is set,
> +	 * use folio_zero_user() which zeros near the faulting address
> +	 * last, keeping those cachelines hot.  For init_on_alloc, use
> +	 * kernel_init_pages() to avoid unnecessary cache flush overhead
> +	 * on architectures with virtually-indexed caches.

This whole paragraph seems pretty useless and just describing the code?

> +	 */
> +	if (init) {
> +		if ((gfp_flags & __GFP_ZERO) && user_addr != USER_ADDR_NONE) {
> +			/*
> +			 * folio_zero_user relies on folio_nr_pages which
> +			 * requires __GFP_COMP for order > 0.  All user folio
> +			 * allocations set __GFP_COMP via __folio_alloc.

This whole paragraph is useless and very like the kind of stuff AI generates for
comments, i.e. overly long + entirely unnecessary stuff.

> +			 * user_addr != USER_ADDR_NONE implies sleepable
> +			 * context (user page fault).

Can you safely assume that? Also inferring which context we are in from this
parameter seems risky.

It seems to me that you're now making it such that kernel developers:

- Have to know when and when not to specify a user address, and under what
  circumstances we might consider that to be mapped.

- Need to know to do this correctly for aliasing architectures or have silent
  correctness issues.

- Need to take context into account when specifying this.

We definitely need to find a simpler way to do this!

> +			 */
> +			VM_WARN_ON_ONCE(order && !(gfp_flags & __GFP_COMP));

Surely by now we can assume this?

> +			folio_zero_user(page_folio(page), user_addr);
> +		} else
> +			kernel_init_pages(page, 1 << order);

I hate this hanging else branch... definitely prefer {} on both branches.

But in any case it seems like we could avoid some indentation with something
like:

	if (init && init_should_folio_zero) {
		...
	} else if (init) {
		...
	}

Or even a:

	if (!init)
		goto out;

And stick an out label below?

> +	}

>
>  	set_page_owner(page, order, gfp_flags);
>  	page_table_check_alloc(page, order);
> --
> MST
>

Oh and in general it seems that this conflicts with [0] which removes
kernel_init_pages().

[0];https://lore.kernel.org/all/20260422102729.166599-1-hsalunke@amd.com/

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 00/37] mm/virtio: skip redundant zeroing of host-zeroed pages
From: Vlastimil Babka (SUSE) @ 2026-06-08 11:13 UTC (permalink / raw)
  To: Michael S. Tsirkin, linux-kernel
  Cc: David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <8d5d2fa3-a60c-4d04-b3bf-7a8fe89cb1a0@kernel.org>

On 6/8/26 13:02, Vlastimil Babka (SUSE) wrote:
> On 6/8/26 10:33, Michael S. Tsirkin wrote:
>> Further, on architectures with aliasing caches, upstream with init_on_alloc
> 
> It seems those are niche architectures so we can ignore that part for perf
> purposes; the other reason why user_alloc_needs_zeroing() would be true is
> booting with init_on_alloc.

OK I misread how user_alloc_needs_zeroing() works wrt init_on_alloc, as it's
negated. But you're changing that anyway to skip that user zeroing, right?

"
This series eliminates that double-zeroing by moving the zeroing
into the post_alloc_hook + propagating the "host
already zeroed this page" information through the buddy allocator.
"

So relying on "everything in buddy is zeroed" would still work I'd think.

>> double-zeros user pages: once via kernel_init_pages() in
>> post_alloc_hook, and again via clear_user_highpage() at the
>> callsite (because user_alloc_needs_zeroing() returns true).
>> This series eliminates that double-zeroing by moving the zeroing
>> into the post_alloc_hook + propagating the "host
>> already zeroed this page" information through the buddy allocator.
> 
> I wonder if this whole thing would be much easier if those that would want
> performance would enable only init_on_free and not init_on_alloc at the same
> time. If you enable both you're paranoid and just eat the cost I guess. Then
> you're maybe also paranoid so much that you wouldn't want to trust the host
> zeroing anyway?
> 
> post_alloc_hook() already has logic that if init_on_free is enabled, there's
> no init during alloc. So then I think you would only need to communicate
> that host zeroed some pages when the guest adds them to the buddy allocator,
> and that's it?
> 
> I.e. if we can just assume that everything in the buddy is zeroed, we don't
> need all the PG_zero flag and whatnot complexity.


^ permalink raw reply

* Re: [PATCH v10 36/37] mm: balloon: use put_page_zeroed for zeroed balloon pages
From: David Hildenbrand (Arm) @ 2026-06-08 11:10 UTC (permalink / raw)
  To: Michael S. Tsirkin, linux-kernel
  Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Muchun Song,
	Oscar Salvador, Andrew Morton, Lorenzo Stoakes, Liam R. Howlett,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache,
	Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Hugh Dickins,
	Matthew Brost, Joshua Hahn, Rakie Kim, Byungchul Park,
	Gregory Price, Ying Huang, Alistair Popple, Christoph Lameter,
	David Rientjes, Roman Gushchin, Harry Yoo, Axel Rasmussen,
	Yuanchu Xie, Wei Xu, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
	Baoquan He, virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <619342ca2c01c60862fe49cd4a790d171cd65ef3.1780906288.git.mst@redhat.com>

On 6/8/26 10:40, Michael S. Tsirkin wrote:
> When a balloon page marked PageZeroed is freed during migration,
> use put_page_zeroed() to propagate the zeroed hint to the buddy
> allocator. Previously the hint was silently lost via plain put_page().
> 
> No page has PageZeroed set yet; the next patch
> (VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE) will set it on
> pages the host has zeroed during inflate.
> Note: during balloon migration, the migration core holds an
> extra reference, so put_page_zeroed() will not be the final
> put. The zeroed hint is lost in that case, which is
> acceptable: it is a best-effort optimization.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---
>  mm/balloon.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/balloon.c b/mm/balloon.c
> index 96a8f1e20bc6..6c9dd8ab0c5d 100644
> --- a/mm/balloon.c
> +++ b/mm/balloon.c
> @@ -324,7 +324,15 @@ static int balloon_page_migrate(struct page *newpage, struct page *page,
>  	balloon_page_finalize(page);
>  	spin_unlock_irqrestore(&balloon_pages_lock, flags);
>  
> -	put_page(page);
> +	if (PageZeroed(page)) {
> +		/* Atomic to serialize with memory_failure's
> +		 * TestSetPageHWPoison; not under zone->lock here.
> +		 */
> +		ClearPageZeroed(page);
> +		put_page_zeroed(page);
> +	} else {
> +		put_page(page);
> +	}
>  
>  	return 0;
>  }

I think I raised previously that this is best done later.

This patch set is currently trying to do too many things, and is on the larger
side. I think we should try to reduce it to the bare minimum and get some
agreement between maintainers on the core design.

-- 
Cheers,

David

^ permalink raw reply

* Re: [PATCH v10 07/37] mm: thread user_addr through page allocator for cache-friendly zeroing
From: David Hildenbrand (Arm) @ 2026-06-08 11:08 UTC (permalink / raw)
  To: Lorenzo Stoakes, Michael S. Tsirkin
  Cc: linux-kernel, Jason Wang, Xuan Zhuo, Eugenio Pérez,
	Muchun Song, Oscar Salvador, Andrew Morton, Liam R. Howlett,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache,
	Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Hugh Dickins,
	Matthew Brost, Joshua Hahn, Rakie Kim, Byungchul Park,
	Gregory Price, Ying Huang, Alistair Popple, Christoph Lameter,
	David Rientjes, Roman Gushchin, Harry Yoo, Axel Rasmussen,
	Yuanchu Xie, Wei Xu, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
	Baoquan He, virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <aiaUDTeHDA45ZXFS@lucifer>

On 6/8/26 12:23, Lorenzo Stoakes wrote:
> I noticed this patch, again, sneaks in some additional code changes that
> are not mentioned in the commit message and seem irrelevant to the patch.
> 
> Not sure if the AI is doing this, but please don't do that.
> 
> On Mon, Jun 08, 2026 at 04:35:17AM -0400, Michael S. Tsirkin wrote:
>> Thread a user virtual address from vma_alloc_folio() down through
>> the page allocator to post_alloc_hook(). This is plumbing
>> preparation for a subsequent patch that will use user_addr to
>> call folio_zero_user() for cache-friendly zeroing of user pages.
> 
> This feels like very weak justification for code that massively changes mm
> code and makes it all much worse.
> 
>>
>> The user_addr is stored in struct alloc_context and flows through:
>>   vma_alloc_folio -> folio_alloc_mpol -> __alloc_pages_mpol ->
>>   __alloc_frozen_pages -> get_page_from_freelist -> prep_new_page ->
>>   post_alloc_hook
> 
> Is this the only relevant code path? You're changing a TON of code here
> that will have multiple different code paths?
> 
>>
>> USER_ADDR_NONE ((unsigned long)-1) is used for non-user
>> allocations, since address 0 is a valid userspace mapping.
> 
> Ugh god, so now we're passing a user address through allocation paths that
> might not even be aware of this or be allocating memory at a point when the
> mapping is known?

The original ideas was to do this only with internal interfaces. I think I
raised before to leave hugetlb alone for now.

Fundamentally, user_alloc_needs_zeroing() is something we should get rid of, to
just be able use __GFP_ZERO and do zeroing at exactly one place.


The question is how to pass that information (user_addr) through internal APIs.

I previously said, that ideally we'd end up with a folio allocation interface in
mm/page_alloc.c, from where we could get this done more cleanly internally.

But I don't want what the previous proposals did: use GFP flags+leak state or
return magic "zeroed" flags.

-- 
Cheers,

David

^ permalink raw reply

* Re: [PATCH v10 07/37] mm: thread user_addr through page allocator for cache-friendly zeroing
From: Lorenzo Stoakes @ 2026-06-08 11:06 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <aiaUDTeHDA45ZXFS@lucifer>

In terms of an alternative, I feel that we should:

1. Continue to not change external APIs (correct to not do so)

2. Avoid adding this to any code paths that don't strictly need it

3. Refactor the existing code to make it harder to get wrong. Don't have an
   easily confused sentinel, rather have some kind of context state that is
   passed through.

I mean we already have alloc_context and you're already updating it :)

But instead of overloading user_addr to indicate all kinds of things, instead
make life easier by actually breaking things out.

Like:

enum alloc_context_type {
	KERNEL_ALLOCATION,
	USER_MAPPED_ALLOCATION,
	USER_UNMAPPED_ALLOCATION, // Maybe? Do we ever?
	/* Perhaps some other states we want to encode? */
};

struct alloc_context {
	...

	enum alloc_context_type type;
	unsigned long user_addr; // Only set if type == USER_ALLOCATION

	// Maybe something suggesting context or whether we init before in some
	// cases?
};

And thread that through?

That way we can also add further context later if required rather than this
awkward easily misunderstood parameter.

It might means some further prepatory patches but it avoids the confusion of not
knowing what to set this to, and could perhaps be set further up the stack?

Then look to completely eliminate cases where we zero other than with
__GFP_ZERO.

Perhaps others have ideas too, but this kind of approach seems more appropriate?

It also seems right to send this work as a entirely separate series. 'Always
zero user memory using __GFP_ZERO' seems like a totally valid independent
series.

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 00/37] mm/virtio: skip redundant zeroing of host-zeroed pages
From: Vlastimil Babka (SUSE) @ 2026-06-08 11:02 UTC (permalink / raw)
  To: Michael S. Tsirkin, linux-kernel
  Cc: David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <cover.1780906288.git.mst@redhat.com>

On 6/8/26 10:33, Michael S. Tsirkin wrote:
> Further, on architectures with aliasing caches, upstream with init_on_alloc

It seems those are niche architectures so we can ignore that part for perf
purposes; the other reason why user_alloc_needs_zeroing() would be true is
booting with init_on_alloc.

> double-zeros user pages: once via kernel_init_pages() in
> post_alloc_hook, and again via clear_user_highpage() at the
> callsite (because user_alloc_needs_zeroing() returns true).
> This series eliminates that double-zeroing by moving the zeroing
> into the post_alloc_hook + propagating the "host
> already zeroed this page" information through the buddy allocator.

I wonder if this whole thing would be much easier if those that would want
performance would enable only init_on_free and not init_on_alloc at the same
time. If you enable both you're paranoid and just eat the cost I guess. Then
you're maybe also paranoid so much that you wouldn't want to trust the host
zeroing anyway?

post_alloc_hook() already has logic that if init_on_free is enabled, there's
no init during alloc. So then I think you would only need to communicate
that host zeroed some pages when the guest adds them to the buddy allocator,
and that's it?

I.e. if we can just assume that everything in the buddy is zeroed, we don't
need all the PG_zero flag and whatnot complexity.

^ permalink raw reply

* Re: [PATCH v10 13/37] mm: use __GFP_ZERO in vma_alloc_zeroed_movable_folio
From: Lorenzo Stoakes @ 2026-06-08 10:55 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <aiaau7RIhkJMS_oe@lucifer>

On Mon, Jun 08, 2026 at 11:40:02AM +0100, Lorenzo Stoakes wrote:
> On Mon, Jun 08, 2026 at 04:36:51AM -0400, Michael S. Tsirkin wrote:
> > Now that post_alloc_hook() handles cache-friendly user page
> > zeroing via folio_zero_user(), convert vma_alloc_zeroed_movable_folio()
> > to pass __GFP_ZERO instead of zeroing at the callsite.
> >
> > Note: before this series, replacing clear_user_highpage() with
> > __GFP_ZERO was unsafe on cache-aliasing architectures because
> > __GFP_ZERO uses clear_page() without a dcache flush. With this
> > series, it is safe if the caller passes a valid user address
> > (not USER_ADDR_NONE) to vma_alloc_folio() etc., which delivers
>
> Wait, so now you're making actual correctness predicated on correctly
> passing the right user address??
>
> > it to post_alloc_hook() for the dcache flush via
> > folio_zero_user(). It is only unsafe if USER_ADDR_NONE is passed.
>
> Yeah, ok I'm beating a dead horse a bit here, but no to this approach.
>
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Assisted-by: Claude:claude-opus-4-6
> > ---
> >  include/linux/highmem.h | 9 ++-------
> >  1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> > index d7aac9de1c8a..8b0afaabbc6e 100644
> > --- a/include/linux/highmem.h
> > +++ b/include/linux/highmem.h
> > @@ -320,13 +320,8 @@ static inline
> >  struct folio *vma_alloc_zeroed_movable_folio(struct vm_area_struct *vma,
> >  				   unsigned long vaddr)
> >  {
> > -	struct folio *folio;
> > -
> > -	folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, vma, vaddr);
> > -	if (folio && user_alloc_needs_zeroing())
>
> So now we are unconditionally zeroing the pages even if
> !user_alloc_needs_zeroing()? You don't mention this in the commit message
> and it seems like it'll regress performance?

OK sorry I see in 12/37 you do this there instead.

>
> > -		clear_user_highpage(&folio->page, vaddr);
> > -
> > -	return folio;
> > +	return vma_alloc_folio(GFP_HIGHUSER_MOVABLE | __GFP_ZERO,
> > +			      0, vma, vaddr);
> >  }
> >  #endif
> >
> > --
> > MST
> >
>
> Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 13/37] mm: use __GFP_ZERO in vma_alloc_zeroed_movable_folio
From: Lorenzo Stoakes @ 2026-06-08 10:39 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <862a59e98104db76ee3998460bfc55e937c218c4.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:36:51AM -0400, Michael S. Tsirkin wrote:
> Now that post_alloc_hook() handles cache-friendly user page
> zeroing via folio_zero_user(), convert vma_alloc_zeroed_movable_folio()
> to pass __GFP_ZERO instead of zeroing at the callsite.
>
> Note: before this series, replacing clear_user_highpage() with
> __GFP_ZERO was unsafe on cache-aliasing architectures because
> __GFP_ZERO uses clear_page() without a dcache flush. With this
> series, it is safe if the caller passes a valid user address
> (not USER_ADDR_NONE) to vma_alloc_folio() etc., which delivers

Wait, so now you're making actual correctness predicated on correctly
passing the right user address??

> it to post_alloc_hook() for the dcache flush via
> folio_zero_user(). It is only unsafe if USER_ADDR_NONE is passed.

Yeah, ok I'm beating a dead horse a bit here, but no to this approach.

>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---
>  include/linux/highmem.h | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index d7aac9de1c8a..8b0afaabbc6e 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -320,13 +320,8 @@ static inline
>  struct folio *vma_alloc_zeroed_movable_folio(struct vm_area_struct *vma,
>  				   unsigned long vaddr)
>  {
> -	struct folio *folio;
> -
> -	folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, vma, vaddr);
> -	if (folio && user_alloc_needs_zeroing())

So now we are unconditionally zeroing the pages even if
!user_alloc_needs_zeroing()? You don't mention this in the commit message
and it seems like it'll regress performance?

> -		clear_user_highpage(&folio->page, vaddr);
> -
> -	return folio;
> +	return vma_alloc_folio(GFP_HIGHUSER_MOVABLE | __GFP_ZERO,
> +			      0, vma, vaddr);
>  }
>  #endif
>
> --
> MST
>

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 11/37] mm: page_alloc: move prep_compound_page before post_alloc_hook
From: Lorenzo Stoakes @ 2026-06-08 10:33 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <de0433334a7763b81ce8ca8f8a332c800cfe64ed.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:36:20AM -0400, Michael S. Tsirkin wrote:
> Move prep_compound_page() before post_alloc_hook() in prep_new_page().
>
> The next patch adds a folio_zero_user() call to post_alloc_hook(),
> which uses folio_nr_pages() to determine how many pages to zero.
> Without compound metadata set up first, folio_nr_pages() returns 1
> for higher-order allocations, so only the first page would be zeroed.
>
> All other operations in post_alloc_hook() (arch_alloc_page, KASAN,
> debug, page owner, etc.) use raw page pointers with explicit order
> counts and are unaffected by this reordering.

I'd put this justification for why this is safe above the 'next patch' stuff.

>
> Also reorder compaction_alloc_noprof() for consistency. Compaction
> currently passes USER_ADDR_NONE so folio_zero_user() is not called

Yeah again, this 'just so' stuff of when or when not an address is passed is
concerning.

> there, but keeping the same ordering avoids a future tripping hazard.

No functional changes or functional changes? If none then say so.

>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh
> ---
>  mm/compaction.c | 4 ++--
>  mm/page_alloc.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 72684fe81e83..4336e433c99b 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1849,10 +1849,10 @@ static struct folio *compaction_alloc_noprof(struct folio *src, unsigned long da
>  		set_page_private(&freepage[size], start_order);
>  	}
>  	dst = (struct folio *)freepage;
> -	post_alloc_hook(&dst->page, order, __GFP_MOVABLE, USER_ADDR_NONE);
> -	set_page_refcounted(&dst->page);
>  	if (order)
>  		prep_compound_page(&dst->page, order);
> +	post_alloc_hook(&dst->page, order, __GFP_MOVABLE, USER_ADDR_NONE);
> +	set_page_refcounted(&dst->page);
>  	cc->nr_freepages -= 1 << order;
>  	cc->nr_migratepages -= 1 << order;
>  	return page_rmappable_folio(&dst->page);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0943ab724032..4676fd49819e 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1874,11 +1874,11 @@ static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags
>  							unsigned int alloc_flags,
>  							unsigned long user_addr)
>  {
> -	post_alloc_hook(page, order, gfp_flags, user_addr);
> -
>  	if (order && (gfp_flags & __GFP_COMP))
>  		prep_compound_page(page, order);
>
> +	post_alloc_hook(page, order, gfp_flags, user_addr);
> +
>  	/*
>  	 * page is set pfmemalloc when ALLOC_NO_WATERMARKS was necessary to
>  	 * allocate the page. The expectation is that the caller is taking
> --
> MST
>

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 08/37] mm: add alloc_contig_frozen_pages_user for cache-friendly zeroing
From: Lorenzo Stoakes @ 2026-06-08 10:29 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <f2036f596f17f66fb55b74a5fd85e8b7ab51876d.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:35:29AM -0400, Michael S. Tsirkin wrote:
> Add a _user variant of alloc_contig_frozen_pages that accepts a user_addr
> parameter for cache-friendly zeroing of contiguous allocations.
>
> No functional change; all existing callers continue to pass
> USER_ADDR_NONE.

Well, except that you're adding a new function...

>
> Note for reviewers: non-compound contiguous allocations are

Please don't put notes for reviewers in commit messages.

> zeroed via kernel_init_pages, same as before this patch.
> There is no fault address because these allocations are not
> from the page fault path. For compound allocations, user_addr
> reaches post_alloc_hook() which calls folio_zero_user() with
> the dcache flush on cache-aliasing architectures.

Yeah it's exactly this kind of 'just have to know' stuff that makes this
user_addr approach unacceptable.

We mustn't add more cognitive overhead for already confusing code. Now
everybody using these has to figure out what 'user_addr' means and will
inevitably get it wrong.

This whole approach needs to be rethought.

>
> Note about Sashiko (sashiko.dev) false positives: sashiko
> flags two issues here: (1) user_addr silently ignored for
> non-compound allocations, and (2) post_alloc_hook ignores
> user_addr. Both are false positives: (1) non-compound
> contiguous allocations have no fault address to pass, and
> (2) post_alloc_hook does use user_addr when it is not
> USER_ADDR_NONE.

Please don't put AI hallucinations in commit messages.

If you want something to not appear in a commit message, but want reviewers
to see it, put it below the '---' in the patch.


>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

This patch is unacceptable for the same reasons given for 7/37.

> Assisted-by: Claude:claude-opus-4-6
> ---
>  include/linux/gfp.h |  6 ++++++
>  mm/page_alloc.c     | 42 ++++++++++++++++++++++++++++++++----------
>  2 files changed, 38 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index ee35c5367abc..73109d4e31a4 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -453,6 +453,12 @@ struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
>  #define alloc_contig_frozen_pages(...) \
>  	alloc_hooks(alloc_contig_frozen_pages_noprof(__VA_ARGS__))
>
> +struct page *alloc_contig_frozen_pages_user_noprof(unsigned long nr_pages,
> +		gfp_t gfp_mask, int nid, nodemask_t *nodemask,
> +		unsigned long user_addr);
> +#define alloc_contig_frozen_pages_user(...) \
> +	alloc_hooks(alloc_contig_frozen_pages_user_noprof(__VA_ARGS__))
> +
>  struct page *alloc_contig_pages_noprof(unsigned long nr_pages, gfp_t gfp_mask,
>  		int nid, nodemask_t *nodemask);
>  #define alloc_contig_pages(...)	\
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 21b52c879751..6d3f284c607d 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6975,13 +6975,15 @@ static void __free_contig_frozen_range(unsigned long pfn, unsigned long nr_pages
>  }
>
>  /**
> - * alloc_contig_frozen_range() -- tries to allocate given range of frozen pages
> + * __alloc_contig_frozen_range() -- tries to allocate given range of frozen pages
>   * @start:	start PFN to allocate
>   * @end:	one-past-the-last PFN to allocate
>   * @alloc_flags:	allocation information
>   * @gfp_mask:	GFP mask. Node/zone/placement hints are ignored; only some
>   *		action and reclaim modifiers are supported. Reclaim modifiers
>   *		control allocation behavior during compaction/migration/reclaim.
> + * @user_addr:	user virtual address for cache-friendly zeroing, or
> + *		USER_ADDR_NONE for kernel allocations.

Yeah, I really do not want us passing USER_ADDR_NONE for kernel allocations
thanks. This is hugely confusing already pretty confusing logic.

>   *
>   * The PFN range does not have to be pageblock aligned. The PFN range must
>   * belong to a single zone.
> @@ -6997,8 +6999,9 @@ static void __free_contig_frozen_range(unsigned long pfn, unsigned long nr_pages
>   *
>   * Return: zero on success or negative error code.
>   */
> -int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
> -		acr_flags_t alloc_flags, gfp_t gfp_mask)
> +static int __alloc_contig_frozen_range(unsigned long start, unsigned long end,
> +		acr_flags_t alloc_flags, gfp_t gfp_mask,
> +		unsigned long user_addr)
>  {
>  	const unsigned int order = ilog2(end - start);
>  	unsigned long outer_start, outer_end;
> @@ -7125,7 +7128,7 @@ int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
>  		struct page *head = pfn_to_page(start);
>
>  		check_new_pages(head, order);
> -		prep_new_page(head, order, gfp_mask, 0, USER_ADDR_NONE);
> +		prep_new_page(head, order, gfp_mask, 0, user_addr);
>  	} else {
>  		ret = -EINVAL;
>  		WARN(true, "PFN range: requested [%lu, %lu), allocated [%lu, %lu)\n",
> @@ -7135,6 +7138,13 @@ int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
>  	undo_isolate_page_range(start, end);
>  	return ret;
>  }
> +
> +int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
> +		acr_flags_t alloc_flags, gfp_t gfp_mask)
> +{
> +	return __alloc_contig_frozen_range(start, end, alloc_flags, gfp_mask,
> +					   USER_ADDR_NONE);
> +}
>  EXPORT_SYMBOL(alloc_contig_frozen_range_noprof);
>
>  /**
> @@ -7227,14 +7237,16 @@ static bool zone_spans_last_pfn(const struct zone *zone,
>  	return zone_spans_pfn(zone, last_pfn);
>  }
>
> -/**
> - * alloc_contig_frozen_pages() -- tries to find and allocate contiguous range of frozen pages
> +/*
> + * alloc_contig_frozen_pages_user_noprof() -- allocate contiguous frozen pages with user address
>   * @nr_pages:	Number of contiguous pages to allocate
>   * @gfp_mask:	GFP mask. Node/zone/placement hints limit the search; only some
>   *		action and reclaim modifiers are supported. Reclaim modifiers
>   *		control allocation behavior during compaction/migration/reclaim.
>   * @nid:	Target node
>   * @nodemask:	Mask for other possible nodes
> + * @user_addr:	user virtual address for cache-friendly zeroing, or
> + *		USER_ADDR_NONE for kernel allocations.
>   *
>   * This routine is a wrapper around alloc_contig_frozen_range(). It scans over
>   * zones on an applicable zonelist to find a contiguous pfn range which can then
> @@ -7253,8 +7265,9 @@ static bool zone_spans_last_pfn(const struct zone *zone,
>   *
>   * Return: pointer to contiguous frozen pages on success, or NULL if not successful.
>   */
> -struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
> -		gfp_t gfp_mask, int nid, nodemask_t *nodemask)
> +struct page *alloc_contig_frozen_pages_user_noprof(unsigned long nr_pages,
> +		gfp_t gfp_mask, int nid, nodemask_t *nodemask,
> +		unsigned long user_addr)
>  {
>  	unsigned long ret, pfn, flags;
>  	struct zonelist *zonelist;
> @@ -7282,10 +7295,11 @@ struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
>  				 * win the race and cause allocation to fail.
>  				 */
>  				spin_unlock_irqrestore(&zone->lock, flags);
> -				ret = alloc_contig_frozen_range_noprof(pfn,
> +				ret = __alloc_contig_frozen_range(pfn,
>  							pfn + nr_pages,
>  							ACR_FLAGS_NONE,
> -							gfp_mask);
> +							gfp_mask,
> +							user_addr);
>  				if (!ret)
>  					return pfn_to_page(pfn);
>  				spin_lock_irqsave(&zone->lock, flags);
> @@ -7307,6 +7321,14 @@ struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
>  	}
>  	return NULL;
>  }
> +EXPORT_SYMBOL(alloc_contig_frozen_pages_user_noprof);

Generally we don't add EXPORT_SYMBOL() for new stuff unless
unavoidable. EXPORT_SYMBOL_GPL() is preferred.

> +
> +struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
> +		gfp_t gfp_mask, int nid, nodemask_t *nodemask)
> +{
> +	return alloc_contig_frozen_pages_user_noprof(nr_pages, gfp_mask, nid,
> +						     nodemask, USER_ADDR_NONE);
> +}
>  EXPORT_SYMBOL(alloc_contig_frozen_pages_noprof);
>
>  /**
> --
> MST
>

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 07/37] mm: thread user_addr through page allocator for cache-friendly zeroing
From: Lorenzo Stoakes @ 2026-06-08 10:23 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <50d410b47fe3f45327783e05bd306d5eaab75e65.1780906288.git.mst@redhat.com>

I noticed this patch, again, sneaks in some additional code changes that
are not mentioned in the commit message and seem irrelevant to the patch.

Not sure if the AI is doing this, but please don't do that.

On Mon, Jun 08, 2026 at 04:35:17AM -0400, Michael S. Tsirkin wrote:
> Thread a user virtual address from vma_alloc_folio() down through
> the page allocator to post_alloc_hook(). This is plumbing
> preparation for a subsequent patch that will use user_addr to
> call folio_zero_user() for cache-friendly zeroing of user pages.

This feels like very weak justification for code that massively changes mm
code and makes it all much worse.

>
> The user_addr is stored in struct alloc_context and flows through:
>   vma_alloc_folio -> folio_alloc_mpol -> __alloc_pages_mpol ->
>   __alloc_frozen_pages -> get_page_from_freelist -> prep_new_page ->
>   post_alloc_hook

Is this the only relevant code path? You're changing a TON of code here
that will have multiple different code paths?

>
> USER_ADDR_NONE ((unsigned long)-1) is used for non-user
> allocations, since address 0 is a valid userspace mapping.

Ugh god, so now we're passing a user address through allocation paths that
might not even be aware of this or be allocating memory at a point when the
mapping is known?

>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Honestly I absolutely hate this patch.

You're passing a user address through allocation paths that might not even
have one associated with them, you're vandalising a bunch of core mm stuff
here for virtio, and then also adding a sentinel to indicate 'not user
address' just to confuse matters further.

We separate the physical allocation of memory from mapping it, and you're
now coupling something that is explicitly decoupled to suit a specific
usecase.

It feels like a hack and I don't think we can accept this upstream, as-is.

> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh

This really feels like the kind of brute force thing AI does when coming up
with code for this kind of thing.

I think it needs some architectural thought rather than that here. I'd
suggest thinking about how we might achieve the same objective without
doing this.

> ---
>  include/linux/gfp.h |  2 +-
>  mm/compaction.c     |  5 ++---
>  mm/hugetlb.c        | 36 ++++++++++++++++++++----------------
>  mm/internal.h       | 21 ++++++++++++++++++---
>  mm/mempolicy.c      | 44 ++++++++++++++++++++++++++++++++------------
>  mm/mmap.c           |  6 ++++++
>  mm/page_alloc.c     | 44 +++++++++++++++++++++++++++++---------------
>  mm/slub.c           |  4 ++--
>  8 files changed, 110 insertions(+), 52 deletions(-)
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 7ccbda35b9ad..ee35c5367abc 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -337,7 +337,7 @@ static inline struct folio *folio_alloc_noprof(gfp_t gfp, unsigned int order)
>  static inline struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
>  		struct mempolicy *mpol, pgoff_t ilx, int nid)
>  {
> -	return folio_alloc_noprof(gfp, order);
> +	return __folio_alloc_noprof(gfp, order, numa_node_id(), NULL);
>  }
>  #endif
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 3648ce22c807..72684fe81e83 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -82,7 +82,7 @@ static inline bool is_via_compact_memory(int order) { return false; }
>
>  static struct page *mark_allocated_noprof(struct page *page, unsigned int order, gfp_t gfp_flags)
>  {
> -	post_alloc_hook(page, order, __GFP_MOVABLE);
> +	post_alloc_hook(page, order, __GFP_MOVABLE, USER_ADDR_NONE);

This is really confusing. Essentially 'we don't know what the user address
is' here. And now we're making anybody who calls this have to figure out
what the user address field is actually for and under what circumstances we supply it.

>  	set_page_refcounted(page);
>  	return page;
>  }
> @@ -1849,8 +1849,7 @@ static struct folio *compaction_alloc_noprof(struct folio *src, unsigned long da
>  		set_page_private(&freepage[size], start_order);
>  	}
>  	dst = (struct folio *)freepage;
> -
> -	post_alloc_hook(&dst->page, order, __GFP_MOVABLE);
> +	post_alloc_hook(&dst->page, order, __GFP_MOVABLE, USER_ADDR_NONE);
>  	set_page_refcounted(&dst->page);
>  	if (order)
>  		prep_compound_page(&dst->page, order);
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 4b80b167cc9c..f3bc15a7889a 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1786,7 +1786,8 @@ struct address_space *hugetlb_folio_mapping_lock_write(struct folio *folio)
>  }
>
>  static struct folio *alloc_buddy_frozen_folio(int order, gfp_t gfp_mask,
> -		int nid, nodemask_t *nmask, nodemask_t *node_alloc_noretry)
> +		int nid, nodemask_t *nmask, nodemask_t *node_alloc_noretry,
> +		unsigned long addr)

addr? Can we use user_addr please? This is confusing.

>  {
>  	struct folio *folio;
>  	bool alloc_try_hard = true;
> @@ -1803,7 +1804,7 @@ static struct folio *alloc_buddy_frozen_folio(int order, gfp_t gfp_mask,
>  	if (alloc_try_hard)
>  		gfp_mask |= __GFP_RETRY_MAYFAIL;
>
> -	folio = (struct folio *)__alloc_frozen_pages(gfp_mask, order, nid, nmask);
> +	folio = (struct folio *)__alloc_frozen_pages(gfp_mask, order, nid, nmask, addr);
>
>  	/*
>  	 * If we did not specify __GFP_RETRY_MAYFAIL, but still got a
> @@ -1832,7 +1833,7 @@ static struct folio *alloc_buddy_frozen_folio(int order, gfp_t gfp_mask,
>
>  static struct folio *only_alloc_fresh_hugetlb_folio(struct hstate *h,
>  		gfp_t gfp_mask, int nid, nodemask_t *nmask,
> -		nodemask_t *node_alloc_noretry)
> +		nodemask_t *node_alloc_noretry, unsigned long addr)
>  {
>  	struct folio *folio;
>  	int order = huge_page_order(h);
> @@ -1844,7 +1845,7 @@ static struct folio *only_alloc_fresh_hugetlb_folio(struct hstate *h,
>  		folio = alloc_gigantic_frozen_folio(order, gfp_mask, nid, nmask);
>  	else
>  		folio = alloc_buddy_frozen_folio(order, gfp_mask, nid, nmask,
> -						 node_alloc_noretry);
> +						 node_alloc_noretry, addr);
>  	if (folio)
>  		init_new_hugetlb_folio(folio);
>  	return folio;
> @@ -1858,11 +1859,12 @@ static struct folio *only_alloc_fresh_hugetlb_folio(struct hstate *h,
>   * pages is zero, and the accounting must be done in the caller.
>   */
>  static struct folio *alloc_fresh_hugetlb_folio(struct hstate *h,
> -		gfp_t gfp_mask, int nid, nodemask_t *nmask)
> +		gfp_t gfp_mask, int nid, nodemask_t *nmask,
> +		unsigned long addr)
>  {
>  	struct folio *folio;
>
> -	folio = only_alloc_fresh_hugetlb_folio(h, gfp_mask, nid, nmask, NULL);
> +	folio = only_alloc_fresh_hugetlb_folio(h, gfp_mask, nid, nmask, NULL, addr);
>  	if (folio)
>  		hugetlb_vmemmap_optimize_folio(h, folio);
>  	return folio;
> @@ -1902,7 +1904,7 @@ static struct folio *alloc_pool_huge_folio(struct hstate *h,
>  		struct folio *folio;
>
>  		folio = only_alloc_fresh_hugetlb_folio(h, gfp_mask, node,
> -					nodes_allowed, node_alloc_noretry);
> +					nodes_allowed, node_alloc_noretry, USER_ADDR_NONE);
>  		if (folio)
>  			return folio;
>  	}
> @@ -2071,7 +2073,8 @@ int dissolve_free_hugetlb_folios(unsigned long start_pfn, unsigned long end_pfn)
>   * Allocates a fresh surplus page from the page allocator.
>   */
>  static struct folio *alloc_surplus_hugetlb_folio(struct hstate *h,
> -				gfp_t gfp_mask,	int nid, nodemask_t *nmask)
> +				gfp_t gfp_mask,	int nid, nodemask_t *nmask,
> +				unsigned long addr)

Sometimes user_addr, sometimes addr. no thanks.

>  {
>  	struct folio *folio = NULL;
>
> @@ -2083,7 +2086,7 @@ static struct folio *alloc_surplus_hugetlb_folio(struct hstate *h,
>  		goto out_unlock;
>  	spin_unlock_irq(&hugetlb_lock);
>
> -	folio = alloc_fresh_hugetlb_folio(h, gfp_mask, nid, nmask);
> +	folio = alloc_fresh_hugetlb_folio(h, gfp_mask, nid, nmask, addr);
>  	if (!folio)
>  		return NULL;
>
> @@ -2126,7 +2129,7 @@ static struct folio *alloc_migrate_hugetlb_folio(struct hstate *h, gfp_t gfp_mas
>  	if (hstate_is_gigantic(h))
>  		return NULL;
>
> -	folio = alloc_fresh_hugetlb_folio(h, gfp_mask, nid, nmask);
> +	folio = alloc_fresh_hugetlb_folio(h, gfp_mask, nid, nmask, USER_ADDR_NONE);
>  	if (!folio)
>  		return NULL;
>
> @@ -2162,14 +2165,14 @@ struct folio *alloc_buddy_hugetlb_folio_with_mpol(struct hstate *h,
>  	if (mpol_is_preferred_many(mpol)) {
>  		gfp_t gfp = gfp_mask & ~(__GFP_DIRECT_RECLAIM | __GFP_NOFAIL);
>
> -		folio = alloc_surplus_hugetlb_folio(h, gfp, nid, nodemask);
> +		folio = alloc_surplus_hugetlb_folio(h, gfp, nid, nodemask, addr);
>
>  		/* Fallback to all nodes if page==NULL */
>  		nodemask = NULL;
>  	}
>
>  	if (!folio)
> -		folio = alloc_surplus_hugetlb_folio(h, gfp_mask, nid, nodemask);
> +		folio = alloc_surplus_hugetlb_folio(h, gfp_mask, nid, nodemask, addr);
>  	mpol_cond_put(mpol);
>  	return folio;
>  }
> @@ -2276,7 +2279,8 @@ static int gather_surplus_pages(struct hstate *h, long delta)
>  		 * down the road to pick the current node if that is the case.
>  		 */
>  		folio = alloc_surplus_hugetlb_folio(h, htlb_alloc_mask(h),
> -						    NUMA_NO_NODE, &alloc_nodemask);
> +						    NUMA_NO_NODE, &alloc_nodemask,
> +						    USER_ADDR_NONE);
>  		if (!folio) {
>  			alloc_ok = false;
>  			break;
> @@ -2682,7 +2686,7 @@ static int alloc_and_dissolve_hugetlb_folio(struct folio *old_folio,
>  			spin_unlock_irq(&hugetlb_lock);
>  			gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE;
>  			new_folio = alloc_fresh_hugetlb_folio(h, gfp_mask,
> -							      nid, NULL);
> +							      nid, NULL, USER_ADDR_NONE);
>  			if (!new_folio)
>  				return -ENOMEM;
>  			goto retry;
> @@ -3380,13 +3384,13 @@ static void __init hugetlb_hstate_alloc_pages_onenode(struct hstate *h, int nid)
>  			gfp_t gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE;
>
>  			folio = only_alloc_fresh_hugetlb_folio(h, gfp_mask, nid,
> -					&node_states[N_MEMORY], NULL);
> +					&node_states[N_MEMORY], NULL, USER_ADDR_NONE);
>  			if (!folio && !list_empty(&folio_list) &&
>  			    hugetlb_vmemmap_optimizable_size(h)) {
>  				prep_and_add_allocated_folios(h, &folio_list);
>  				INIT_LIST_HEAD(&folio_list);
>  				folio = only_alloc_fresh_hugetlb_folio(h, gfp_mask, nid,
> -						&node_states[N_MEMORY], NULL);
> +						&node_states[N_MEMORY], NULL, USER_ADDR_NONE);
>  			}
>  			if (!folio)
>  				break;
> diff --git a/mm/internal.h b/mm/internal.h
> index 5a2ddcf68e0b..9d2198114510 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -662,6 +662,16 @@ void calculate_min_free_kbytes(void);
>  int __meminit init_per_zone_wmark_min(void);
>  void page_alloc_sysctl_init(void);
>
> +/*
> + * Sentinel for user_addr: indicates a non-user allocation.
> + * Cannot use 0 because address 0 is a valid userspace mapping.
> + * (unsigned long)-1 is safe because:
> + * 1. vm_end = addr + len <= TASK_SIZE, and vm_end is exclusive,
> + *    so -1 is never inside any VMA.
> + * 2. It will only be compared to page-aligned addresses.
> + */
> +#define USER_ADDR_NONE	((unsigned long)-1)
> +
>  /*
>   * Structure for holding the mostly immutable allocation parameters passed
>   * between functions involved in allocations, including the alloc_pages*
> @@ -693,6 +703,7 @@ struct alloc_context {
>  	 */
>  	enum zone_type highest_zoneidx;
>  	bool spread_dirty_pages;
> +	unsigned long user_addr;
>  };
>
>  /*
> @@ -916,13 +927,14 @@ static inline void init_compound_tail(struct page *tail,
>  	prep_compound_tail(tail, head, order);
>  }
>
> -void post_alloc_hook(struct page *page, unsigned int order, gfp_t gfp_flags);
> +void post_alloc_hook(struct page *page, unsigned int order, gfp_t gfp_flags,
> +		     unsigned long user_addr);
>  extern bool free_pages_prepare(struct page *page, unsigned int order);
>
>  extern int user_min_free_kbytes;
>
>  struct page *__alloc_frozen_pages_noprof(gfp_t, unsigned int order, int nid,
> -		nodemask_t *);
> +		nodemask_t *, unsigned long user_addr);
>  #define __alloc_frozen_pages(...) \
>  	alloc_hooks(__alloc_frozen_pages_noprof(__VA_ARGS__))
>  void free_frozen_pages(struct page *page, unsigned int order);
> @@ -930,10 +942,13 @@ void free_unref_folios(struct folio_batch *fbatch);
>
>  #ifdef CONFIG_NUMA
>  struct page *alloc_frozen_pages_noprof(gfp_t, unsigned int order);
> +struct folio *folio_alloc_mpol_user_noprof(gfp_t gfp, unsigned int order,
> +		struct mempolicy *pol, pgoff_t ilx, int nid,
> +		unsigned long user_addr);
>  #else
>  static inline struct page *alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order)
>  {
> -	return __alloc_frozen_pages_noprof(gfp, order, numa_node_id(), NULL);
> +	return __alloc_frozen_pages_noprof(gfp, order, numa_node_id(), NULL, USER_ADDR_NONE);
>  }
>  #endif
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index a1707ad498a8..f573ff32e94d 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2413,7 +2413,8 @@ bool mempolicy_in_oom_domain(struct task_struct *tsk,
>  }
>
>  static struct page *alloc_pages_preferred_many(gfp_t gfp, unsigned int order,
> -						int nid, nodemask_t *nodemask)
> +						int nid, nodemask_t *nodemask,
> +						unsigned long user_addr)
>  {
>  	struct page *page;
>  	gfp_t preferred_gfp;
> @@ -2426,25 +2427,29 @@ static struct page *alloc_pages_preferred_many(gfp_t gfp, unsigned int order,
>  	 */
>  	preferred_gfp = gfp | __GFP_NOWARN;
>  	preferred_gfp &= ~(__GFP_DIRECT_RECLAIM | __GFP_NOFAIL);
> -	page = __alloc_frozen_pages_noprof(preferred_gfp, order, nid, nodemask);
> +	page = __alloc_frozen_pages_noprof(preferred_gfp, order, nid,
> +					   nodemask, user_addr);
>  	if (!page)
> -		page = __alloc_frozen_pages_noprof(gfp, order, nid, NULL);
> +		page = __alloc_frozen_pages_noprof(gfp, order, nid, NULL,
> +						   user_addr);
>
>  	return page;
>  }
>
>  /**
> - * alloc_pages_mpol - Allocate pages according to NUMA mempolicy.
> + * __alloc_pages_mpol - Allocate pages according to NUMA mempolicy.
>   * @gfp: GFP flags.
>   * @order: Order of the page allocation.
>   * @pol: Pointer to the NUMA mempolicy.
>   * @ilx: Index for interleave mempolicy (also distinguishes alloc_pages()).
>   * @nid: Preferred node (usually numa_node_id() but @mpol may override it).
> + * @user_addr: User fault address for cache-friendly zeroing, or USER_ADDR_NONE.

OK this isn't great - 'for cache-friendly zeroing' is vague, confusing, and
an break in the abstraction (don't tell me one specific detail of what
you're doing down the callstack) .

'User fault address' is nebulous and confusing.

>   *
>   * Return: The page on success or NULL if allocation fails.
>   */
> -static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
> -		struct mempolicy *pol, pgoff_t ilx, int nid)
> +static struct page *__alloc_pages_mpol(gfp_t gfp, unsigned int order,
> +		struct mempolicy *pol, pgoff_t ilx, int nid,
> +		unsigned long user_addr)
>  {
>  	nodemask_t *nodemask;
>  	struct page *page;
> @@ -2452,7 +2457,8 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>  	nodemask = policy_nodemask(gfp, pol, ilx, &nid);
>
>  	if (pol->mode == MPOL_PREFERRED_MANY)
> -		return alloc_pages_preferred_many(gfp, order, nid, nodemask);
> +		return alloc_pages_preferred_many(gfp, order, nid, nodemask,
> +						 user_addr);
>
>  	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
>  	    /* filter "hugepage" allocation, unless from alloc_pages() */
> @@ -2476,7 +2482,7 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>  			 */
>  			page = __alloc_frozen_pages_noprof(
>  				gfp | __GFP_THISNODE | __GFP_NORETRY, order,
> -				nid, NULL);
> +				nid, NULL, user_addr);
>  			if (page || !(gfp & __GFP_DIRECT_RECLAIM))
>  				return page;
>  			/*
> @@ -2488,7 +2494,7 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>  		}
>  	}
>
> -	page = __alloc_frozen_pages_noprof(gfp, order, nid, nodemask);
> +	page = __alloc_frozen_pages_noprof(gfp, order, nid, nodemask, user_addr);
>
>  	if (unlikely(pol->mode == MPOL_INTERLEAVE ||
>  		     pol->mode == MPOL_WEIGHTED_INTERLEAVE) && page) {
> @@ -2504,11 +2510,18 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>  	return page;
>  }
>
> -struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
> +static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>  		struct mempolicy *pol, pgoff_t ilx, int nid)
>  {
> -	struct page *page = alloc_pages_mpol(gfp | __GFP_COMP, order, pol,
> -			ilx, nid);
> +	return __alloc_pages_mpol(gfp, order, pol, ilx, nid, USER_ADDR_NONE);
> +}
> +
> +struct folio *folio_alloc_mpol_user_noprof(gfp_t gfp, unsigned int order,
> +		struct mempolicy *pol, pgoff_t ilx, int nid,
> +		unsigned long user_addr)
> +{
> +	struct page *page = __alloc_pages_mpol(gfp | __GFP_COMP, order, pol,
> +			ilx, nid, user_addr);
>  	if (!page)
>  		return NULL;
>
> @@ -2516,6 +2529,13 @@ struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
>  	return page_rmappable_folio(page);
>  }
>
> +struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
> +		struct mempolicy *pol, pgoff_t ilx, int nid)
> +{
> +	return folio_alloc_mpol_user_noprof(gfp, order, pol, ilx, nid,
> +					    USER_ADDR_NONE);
> +}
> +
>  struct page *alloc_frozen_pages_noprof(gfp_t gfp, unsigned order)
>  {
>  	struct mempolicy *pol = &default_policy;
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 5754d1c36462..73413cebc418 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -855,6 +855,12 @@ __get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
>  	if (IS_ERR_VALUE(addr))
>  		return addr;
>
> +	/*
> +	 * The check below ensures vm_end = addr + len <= TASK_SIZE.
> +	 * Since (unsigned long)-1 (USER_ADDR_NONE) >= TASK_SIZE and
> +	 * vm_end is exclusive, USER_ADDR_NONE is thus never a valid
> +	 * userspace address.
> +	 */

You're adding what seems to be an AI-generated comment randomly here for some
reason? Let's not thanks?

>  	if (addr > TASK_SIZE - len)
>  		return -ENOMEM;
>  	if (offset_in_page(addr))
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 6a605d05e8cd..21b52c879751 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1815,7 +1815,7 @@ static inline bool should_skip_init(gfp_t flags)
>  }
>
>  inline void post_alloc_hook(struct page *page, unsigned int order,
> -				gfp_t gfp_flags)
> +				gfp_t gfp_flags, unsigned long user_addr)
>  {
>  	const bool zero_tags = gfp_flags & __GFP_ZEROTAGS;
>  	bool init = !want_init_on_free() && want_init_on_alloc(gfp_flags) &&
> @@ -1870,9 +1870,10 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
>  }
>
>  static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
> -							unsigned int alloc_flags)
> +							unsigned int alloc_flags,
> +							unsigned long user_addr)
>  {
> -	post_alloc_hook(page, order, gfp_flags);
> +	post_alloc_hook(page, order, gfp_flags, user_addr);
>
>  	if (order && (gfp_flags & __GFP_COMP))
>  		prep_compound_page(page, order);
> @@ -3956,7 +3957,8 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
>  		page = rmqueue(zonelist_zone(ac->preferred_zoneref), zone, order,
>  				gfp_mask, alloc_flags, ac->migratetype);
>  		if (page) {
> -			prep_new_page(page, order, gfp_mask, alloc_flags);
> +			prep_new_page(page, order, gfp_mask, alloc_flags,
> +				      ac->user_addr);
>
>  			return page;
>  		} else {
> @@ -4184,7 +4186,8 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
>
>  	/* Prep a captured page if available */
>  	if (page)
> -		prep_new_page(page, order, gfp_mask, alloc_flags);
> +		prep_new_page(page, order, gfp_mask, alloc_flags,
> +			      ac->user_addr);
>
>  	/* Try get a page from the freelist if available */
>  	if (!page)
> @@ -5061,7 +5064,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
>  	struct zoneref *z;
>  	struct per_cpu_pages *pcp;
>  	struct list_head *pcp_list;
> -	struct alloc_context ac;
> +	struct alloc_context ac = { .user_addr = USER_ADDR_NONE };
>  	gfp_t alloc_gfp;
>  	unsigned int alloc_flags = ALLOC_WMARK_LOW;
>  	int nr_populated = 0, nr_account = 0;
> @@ -5176,7 +5179,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
>  		}
>  		nr_account++;
>
> -		prep_new_page(page, 0, gfp, 0);
> +		prep_new_page(page, 0, gfp, 0, USER_ADDR_NONE);
>  		set_page_refcounted(page);
>  		page_array[nr_populated++] = page;
>  	}
> @@ -5201,12 +5204,13 @@ EXPORT_SYMBOL_GPL(alloc_pages_bulk_noprof);
>   * This is the 'heart' of the zoned buddy allocator.
>   */
>  struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
> -		int preferred_nid, nodemask_t *nodemask)
> +		int preferred_nid, nodemask_t *nodemask,
> +		unsigned long user_addr)
>  {
>  	struct page *page;
>  	unsigned int alloc_flags = ALLOC_WMARK_LOW;
>  	gfp_t alloc_gfp; /* The gfp_t that was actually used for allocation */
> -	struct alloc_context ac = { };
> +	struct alloc_context ac = { .user_addr = user_addr };
>
>  	/*
>  	 * There are several places where we assume that the order value is sane
> @@ -5267,10 +5271,12 @@ EXPORT_SYMBOL(__alloc_frozen_pages_noprof);
>
>  struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
>  		int preferred_nid, nodemask_t *nodemask)
> +
>  {
>  	struct page *page;
>
> -	page = __alloc_frozen_pages_noprof(gfp, order, preferred_nid, nodemask);
> +	page = __alloc_frozen_pages_noprof(gfp, order, preferred_nid,
> +					   nodemask, USER_ADDR_NONE);
>  	if (page)
>  		set_page_refcounted(page);
>  	return page;
> @@ -5313,7 +5319,8 @@ struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
>  		gfp |= __GFP_NOWARN;
>
>  	pol = get_vma_policy(vma, addr, order, &ilx);
> -	folio = folio_alloc_mpol_noprof(gfp, order, pol, ilx, numa_node_id());
> +	folio = folio_alloc_mpol_user_noprof(gfp, order, pol, ilx,
> +					     numa_node_id(), addr);
>  	mpol_cond_put(pol);
>  	return folio;
>  }
> @@ -5321,10 +5328,17 @@ struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
>  struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
>  		struct vm_area_struct *vma, unsigned long addr)
>  {
> +	struct page *page;
> +
>  	if (vma->vm_flags & VM_DROPPABLE)
>  		gfp |= __GFP_NOWARN;
>
> -	return folio_alloc_noprof(gfp, order);
> +	page = __alloc_frozen_pages_noprof(gfp | __GFP_COMP, order,
> +					   numa_node_id(), NULL, addr);
> +	if (!page)
> +		return NULL;
> +	set_page_refcounted(page);
> +	return page_rmappable_folio(page);

Err, what?

You're adding arbitrary new code here? What's the justification? Is it an
open-coded version of what was there before just to propagate the addr?

In any case this is totally unacceptable, don't open code like this, don't
make changes like this when the commit message doesn't mention them.

>  }
>  #endif
>  EXPORT_SYMBOL(vma_alloc_folio_noprof);
> @@ -6905,7 +6919,7 @@ static void split_free_frozen_pages(struct list_head *list, gfp_t gfp_mask)
>  		list_for_each_entry_safe(page, next, &list[order], lru) {
>  			int i;
>
> -			post_alloc_hook(page, order, gfp_mask);
> +			post_alloc_hook(page, order, gfp_mask, USER_ADDR_NONE);
>  			if (!order)
>  				continue;
>
> @@ -7111,7 +7125,7 @@ int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
>  		struct page *head = pfn_to_page(start);
>
>  		check_new_pages(head, order);
> -		prep_new_page(head, order, gfp_mask, 0);
> +		prep_new_page(head, order, gfp_mask, 0, USER_ADDR_NONE);
>  	} else {
>  		ret = -EINVAL;
>  		WARN(true, "PFN range: requested [%lu, %lu), allocated [%lu, %lu)\n",
> @@ -7776,7 +7790,7 @@ struct page *alloc_frozen_pages_nolock_noprof(gfp_t gfp_flags, int nid, unsigned
>  	gfp_t alloc_gfp = __GFP_NOWARN | __GFP_ZERO | __GFP_NOMEMALLOC | __GFP_COMP
>  			| gfp_flags;
>  	unsigned int alloc_flags = ALLOC_TRYLOCK;
> -	struct alloc_context ac = { };
> +	struct alloc_context ac = { .user_addr = USER_ADDR_NONE };
>  	struct page *page;
>
>  	VM_WARN_ON_ONCE(gfp_flags & ~__GFP_ACCOUNT);
> diff --git a/mm/slub.c b/mm/slub.c
> index a2bf3756ca7d..f397fa2f3f80 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3275,7 +3275,7 @@ static inline struct slab *alloc_slab_page(gfp_t flags, int node,
>  	else if (node == NUMA_NO_NODE)
>  		page = alloc_frozen_pages(flags, order);
>  	else
> -		page = __alloc_frozen_pages(flags, order, node, NULL);
> +		page = __alloc_frozen_pages(flags, order, node, NULL, USER_ADDR_NONE);
>
>  	if (!page)
>  		return NULL;
> @@ -5236,7 +5236,7 @@ static void *___kmalloc_large_node(size_t size, gfp_t flags, int node)
>  	if (node == NUMA_NO_NODE)
>  		page = alloc_frozen_pages_noprof(flags, order);
>  	else
> -		page = __alloc_frozen_pages_noprof(flags, order, node, NULL);
> +		page = __alloc_frozen_pages_noprof(flags, order, node, NULL, USER_ADDR_NONE);
>
>  	if (page) {
>  		ptr = page_address(page);
> --
> MST
>

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 06/37] mm: move vma_alloc_folio_noprof to page_alloc.c
From: Lorenzo Stoakes @ 2026-06-08 10:05 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <5a3034dc7b3d5ea1edcc8b9baff63c980b7ae434.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:35:05AM -0400, Michael S. Tsirkin wrote:
> Move vma_alloc_folio_noprof() from an inline in gfp.h (for !NUMA)
> and mempolicy.c (for NUMA) to page_alloc.c.

This is an incorrectly described patch. You're not moving the function,
you're also altering its behaviour.

Do the two changes separately, and justify why you are changing this
behaviour.

>
> This prepares for a subsequent patch that will thread user_addr
> through the allocator: having vma_alloc_folio_noprof in page_alloc.c
> means user_addr can be passed to the internal allocation path
> without changing public API signatures or duplicating plumbing
> in both gfp.h and mempolicy.c.

We seem to have a fun mess with some NUMA stuff living in mempolicy.c and
some living in page_alloc.c with #ifdef CONFIG_NUMA's around it.

But I guess that was a pre-existing problem...

>
> The !NUMA path gains the VM_DROPPABLE -> __GFP_NOWARN check
> that the NUMA path already had.

What is your justification for doing this? Commit messages that put the
code changes in English are not helpful, tell me _why_ you are doing this.

It seems that you're suggesting not doing this was a pre-existing bug?
Therefore it would need a fixes/cc: stable no? Unless you feel this isn't
significant enough to require that?

If not, then why are you changing this behaviour?

>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh
> ---
>  include/linux/gfp.h |  9 ++-------
>  mm/mempolicy.c      | 32 --------------------------------
>  mm/page_alloc.c     | 43 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 45 insertions(+), 39 deletions(-)
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 51ef13ed756e..7ccbda35b9ad 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -318,13 +318,13 @@ static inline struct page *alloc_pages_node_noprof(int nid, gfp_t gfp_mask,
>
>  #define  alloc_pages_node(...)			alloc_hooks(alloc_pages_node_noprof(__VA_ARGS__))
>
> +struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
> +		struct vm_area_struct *vma, unsigned long addr);
>  #ifdef CONFIG_NUMA
>  struct page *alloc_pages_noprof(gfp_t gfp, unsigned int order);
>  struct folio *folio_alloc_noprof(gfp_t gfp, unsigned int order);
>  struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
>  		struct mempolicy *mpol, pgoff_t ilx, int nid);
> -struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order, struct vm_area_struct *vma,
> -		unsigned long addr);
>  #else
>  static inline struct page *alloc_pages_noprof(gfp_t gfp_mask, unsigned int order)
>  {
> @@ -339,11 +339,6 @@ static inline struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int orde
>  {
>  	return folio_alloc_noprof(gfp, order);
>  }
> -static inline struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
> -		struct vm_area_struct *vma, unsigned long addr)
> -{
> -	return folio_alloc_noprof(gfp, order);
> -}
>  #endif
>
>  #define alloc_pages(...)			alloc_hooks(alloc_pages_noprof(__VA_ARGS__))
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index d139b074a599..a1707ad498a8 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2516,38 +2516,6 @@ struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
>  	return page_rmappable_folio(page);
>  }
>
> -/**
> - * vma_alloc_folio - Allocate a folio for a VMA.
> - * @gfp: GFP flags.
> - * @order: Order of the folio.
> - * @vma: Pointer to VMA.
> - * @addr: Virtual address of the allocation.  Must be inside @vma.
> - *
> - * Allocate a folio for a specific address in @vma, using the appropriate
> - * NUMA policy.  The caller must hold the mmap_lock of the mm_struct of the
> - * VMA to prevent it from going away.  Should be used for all allocations
> - * for folios that will be mapped into user space, excepting hugetlbfs, and
> - * excepting where direct use of folio_alloc_mpol() is more appropriate.
> - *
> - * Return: The folio on success or NULL if allocation fails.
> - */
> -struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order, struct vm_area_struct *vma,
> -		unsigned long addr)
> -{
> -	struct mempolicy *pol;
> -	pgoff_t ilx;
> -	struct folio *folio;
> -
> -	if (vma->vm_flags & VM_DROPPABLE)
> -		gfp |= __GFP_NOWARN;
> -
> -	pol = get_vma_policy(vma, addr, order, &ilx);
> -	folio = folio_alloc_mpol_noprof(gfp, order, pol, ilx, numa_node_id());
> -	mpol_cond_put(pol);
> -	return folio;
> -}
> -EXPORT_SYMBOL(vma_alloc_folio_noprof);
> -
>  struct page *alloc_frozen_pages_noprof(gfp_t gfp, unsigned order)
>  {
>  	struct mempolicy *pol = &default_policy;
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 8dae5b3f5876..6a605d05e8cd 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5286,6 +5286,49 @@ struct folio *__folio_alloc_noprof(gfp_t gfp, unsigned int order, int preferred_
>  }
>  EXPORT_SYMBOL(__folio_alloc_noprof);
>
> +#ifdef CONFIG_NUMA
> +/**
> + * vma_alloc_folio - Allocate a folio for a VMA.
> + * @gfp: GFP flags.
> + * @order: Order of the folio.
> + * @vma: Pointer to VMA.
> + * @addr: Virtual address of the allocation.  Must be inside @vma.
> + *
> + * Allocate a folio for a specific address in @vma, using the appropriate
> + * NUMA policy.  The caller must hold the mmap_lock of the mm_struct of the
> + * VMA to prevent it from going away.  Should be used for all allocations
> + * for folios that will be mapped into user space, excepting hugetlbfs, and
> + * excepting where direct use of folio_alloc_mpol() is more appropriate.
> + *
> + * Return: The folio on success or NULL if allocation fails.
> + */
> +struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
> +		struct vm_area_struct *vma, unsigned long addr)
> +{
> +	struct mempolicy *pol;
> +	pgoff_t ilx;
> +	struct folio *folio;
> +
> +	if (vma->vm_flags & VM_DROPPABLE)
> +		gfp |= __GFP_NOWARN;
> +
> +	pol = get_vma_policy(vma, addr, order, &ilx);
> +	folio = folio_alloc_mpol_noprof(gfp, order, pol, ilx, numa_node_id());
> +	mpol_cond_put(pol);
> +	return folio;
> +}
> +#else
> +struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
> +		struct vm_area_struct *vma, unsigned long addr)
> +{
> +	if (vma->vm_flags & VM_DROPPABLE)
> +		gfp |= __GFP_NOWARN;
> +
> +	return folio_alloc_noprof(gfp, order);
> +}
> +#endif
> +EXPORT_SYMBOL(vma_alloc_folio_noprof);
> +
>  /*
>   * Common helper functions. Never use with __GFP_HIGHMEM because the returned
>   * address cannot represent highmem pages. Use alloc_pages and then kmap if
> --
> MST
>

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 05/37] mm: hugetlb: remove dead alloc_hugetlb_folio stub
From: Lorenzo Stoakes @ 2026-06-08  9:56 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <a9b5764c69269e5c0518e496b057a75354373b43.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:34:57AM -0400, Michael S. Tsirkin wrote:
> Remove the !CONFIG_HUGETLB_PAGE stub for alloc_hugetlb_folio().
>
> The stub is dead code: all callers are in mm/hugetlb.c
> (CONFIG_HUGETLB_PAGE) or fs/hugetlbfs/inode.c (CONFIG_HUGETLBFS),

obj-$(CONFIG_HUGETLBFS)	+= hugetlb.o hugetlb_sysfs.o hugetlb_sysctl.o

mm/hugetlb.c seems dependent on CONFIG_HUGETLBFS not CONFIG_HUGETLB_PAGE?

> and CONFIG_HUGETLB_PAGE is def_bool HUGETLBFS with nothing
> selecting it independently.
>
> The stub is also broken: it returns NULL, but all callers check
> IS_ERR(folio), so a NULL return would not be caught and would
> crash on the subsequent folio dereference.
>
> Remove it now since follow-up patches change the signature of
> alloc_hugetlb_folio and would otherwise need to update the
> broken stub too.
>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Assisted-by: Claude:claude-opus-4-6
> Reviewed-by: Dev Jain <dev.jain@arm.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

The logic seems good but you should fix up the commit message. With that
fixed:

Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>

Thanks, Lorenzo

> ---
>  include/linux/hugetlb.h | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 5957bc25efa8..1f7ae6609e51 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -1123,13 +1123,6 @@ static inline void wait_for_freed_hugetlb_folios(void)
>  {
>  }
>
> -static inline struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
> -					   unsigned long addr,
> -					   bool cow_from_owner)
> -{
> -	return NULL;
> -}
> -
>  static inline struct folio *
>  alloc_hugetlb_folio_reserve(struct hstate *h, int preferred_nid,
>  			    nodemask_t *nmask, gfp_t gfp_mask)
> --
> MST
>

^ permalink raw reply

* Re: [PATCH v10 03/37] mm: page_alloc: propagate PageReported flag across buddy splits
From: Lorenzo Stoakes @ 2026-06-08  9:52 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <1a407d39a9ebd95e9de67e44f9ad2db37c9f31e9.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:34:39AM -0400, Michael S. Tsirkin wrote:
> When a reported free page is split via expand() to satisfy a
> smaller allocation, the sub-pages placed back on the free lists
> lose the PageReported flag.  This means they will be unnecessarily
> re-reported to the hypervisor in the next reporting cycle, wasting
> work.
>
> While I was unable to quantify the performance difference, it is
> an obvious waste, even if small.

Hmm, that makes me wonder if this is really worth it?

You're making everything more complicated propagating yet another parameter
around (which could actually in itself cause perf/bloat issues) for
something you say yourself isn't really doing much?

>
> Propagate the PageReported flag to sub-pages during expand(),
> both in page_del_and_expand() and try_to_claim_block(), so
>
> split_large_buddy() also propagates PageReported via a bool
> parameter: the caller saves PageReported before
> del_page_from_free_list() clears it, then passes the saved
> value. The flag is set after __free_one_page() with a
> PageBuddy check, matching the page_reporting_drain() pattern.
> Free-path callers pass false (freshly freed pages are never
> reported).
> that they are recognized as already-reported.

The sentence is completely garbled?

>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Again this really doesn't feel like it belongs to this series.

If it's really a bug, it should have fixes/Cc: stable, but honestly my
assessment here is this doesn't seem worthwhile at all.

You're not making a case for it and you're adding complexity. We don't just
take AI-reported bugs at face value (if this was indeed one).

(If sashiko-reported some people do Reported-by: sashiko-bot@kernel.org
now)

> Assisted-by: Claude:claude-opus-4-6
> ---
>  mm/page_alloc.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index d49c254174da..8dae5b3f5876 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1502,7 +1502,8 @@ static void free_pcppages_bulk(struct zone *zone, int count,
>
>  /* Split a multi-block free page into its individual pageblocks. */
>  static void split_large_buddy(struct zone *zone, struct page *page,
> -			      unsigned long pfn, int order, fpi_t fpi)
> +			      unsigned long pfn, int order, fpi_t fpi,
> +			      bool reported)
>  {
>  	unsigned long end = pfn + (1 << order);
>
> @@ -1517,6 +1518,8 @@ static void split_large_buddy(struct zone *zone, struct page *page,
>  		int mt = get_pfnblock_migratetype(page, pfn);
>
>  		__free_one_page(page, pfn, zone, order, mt, fpi);
> +		if (reported && PageBuddy(page) && buddy_order(page) == order)

Isn't this racey? We just freed this page to the buddy allocator, couldn't
it be allocated before we set this flag?

> +			__SetPageReported(page);
>  		pfn += 1 << order;
>  		if (pfn == end)
>  			break;
> @@ -1559,11 +1562,12 @@ static void free_one_page(struct zone *zone, struct page *page,
>  		llist_for_each_entry_safe(p, tmp, llnode, pcp_llist) {
>  			unsigned int p_order = p->private;
>
> -			split_large_buddy(zone, p, page_to_pfn(p), p_order, fpi_flags);
> +			split_large_buddy(zone, p, page_to_pfn(p), p_order,
> +					  fpi_flags, false);

I don't love adding a mystery meat boolean parameter like this.

>  			__count_vm_events(PGFREE, 1 << p_order);
>  		}
>  	}
> -	split_large_buddy(zone, page, pfn, order, fpi_flags);
> +	split_large_buddy(zone, page, pfn, order, fpi_flags, false);
>  	spin_unlock_irqrestore(&zone->lock, flags);
>
>  	__count_vm_events(PGFREE, 1 << order);
> @@ -1694,7 +1698,7 @@ struct page *__pageblock_pfn_to_page(unsigned long start_pfn,
>   * -- nyc
>   */
>  static inline unsigned int expand(struct zone *zone, struct page *page, int low,
> -				  int high, int migratetype)
> +				  int high, int migratetype, bool reported)
>  {
>  	unsigned int size = 1 << high;
>  	unsigned int nr_added = 0;
> @@ -1716,6 +1720,15 @@ static inline unsigned int expand(struct zone *zone, struct page *page, int low,
>  		__add_to_free_list(&page[size], zone, high, migratetype, false);
>  		set_buddy_order(&page[size], high);
>  		nr_added += size;
> +
> +		/*
> +		 * The parent page has been reported to the host.  The
> +		 * sub-pages are part of the same reported block, so mark
> +		 * them reported too.  This avoids re-reporting pages that
> +		 * the host already knows about.
> +		 */

This comment seems entirely redundant (and very much like something claude would
add, it's too verbose like this).

> +		if (reported)
> +			__SetPageReported(&page[size]);
>  	}
>
>  	return nr_added;
> @@ -1726,9 +1739,10 @@ static __always_inline void page_del_and_expand(struct zone *zone,
>  						int high, int migratetype)
>  {
>  	int nr_pages = 1 << high;
> +	bool was_reported = page_reported(page);
>
>  	__del_page_from_free_list(page, zone, high, migratetype);
> -	nr_pages -= expand(zone, page, low, high, migratetype);
> +	nr_pages -= expand(zone, page, low, high, migratetype, was_reported);

I don't love propagating yet another parameter like this.

>  	account_freepages(zone, -nr_pages, migratetype);
>  }
>
> @@ -2116,11 +2130,13 @@ static bool __move_freepages_block_isolate(struct zone *zone,
>  	/* We're a part of a larger buddy */
>  	if (PageBuddy(buddy) && buddy_order(buddy) > pageblock_order) {
>  		int order = buddy_order(buddy);
> +		bool reported = PageReported(buddy);
>
>  		del_page_from_free_list(buddy, zone, order,
>  					get_pfnblock_migratetype(buddy, buddy_pfn));
>  		toggle_pageblock_isolate(page, isolate);
> -		split_large_buddy(zone, buddy, buddy_pfn, order, FPI_NONE);
> +		split_large_buddy(zone, buddy, buddy_pfn, order, FPI_NONE,
> +				  reported);
>  		return true;
>  	}
>
> @@ -2283,10 +2299,12 @@ try_to_claim_block(struct zone *zone, struct page *page,
>  	/* Take ownership for orders >= pageblock_order */
>  	if (current_order >= pageblock_order) {
>  		unsigned int nr_added;
> +		bool was_reported = page_reported(page);
>
>  		del_page_from_free_list(page, zone, current_order, block_type);
>  		change_pageblock_range(page, current_order, start_type);
> -		nr_added = expand(zone, page, order, current_order, start_type);
> +		nr_added = expand(zone, page, order, current_order, start_type,
> +				  was_reported);
>  		account_freepages(zone, nr_added, start_type);
>  		return page;
>  	}
> --
> MST
>

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 01/37] mm: mempolicy: fix interleave index calculation
From: Lorenzo Stoakes @ 2026-06-08  9:43 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <f5b1dcc97baae7183e96743873b943315d93cdb6.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:34:06AM -0400, Michael S. Tsirkin wrote:
> The NUMA interleave index was computed as two separate terms:
>
>     *ilx += vma->vm_pgoff >> order;
>     *ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
>
> This has two problems:
>
> 1. When vm_start is not aligned to the folio size, the
>    subtraction before the shift lets low bits affect the
>    result via borrows.

This feels really vague. Do you have examples of where the calculation has
been impacted like this?

How is this a problem that affects things in practice?

>
> 2. For file-backed VMAs, shifting vm_pgoff and the VMA
>    offset independently loses carries between them, giving
>    wrong chunk indices when vm_pgoff is not aligned to order.

Similar comments to the above. An example would be helpful here.

>
> Combine into a single expression that adds vm_pgoff and

Combining this kind of thing into a single expression is the complete
opposite of what we want to do when refactoring code. No thanks.

> the page-granularity VMA offset first, then shifts once:
>
>     *ilx += (vma->vm_pgoff +
>             (addr >> PAGE_SHIFT) -
>             (vma->vm_start >> PAGE_SHIFT)) >> order;
>
> For anonymous VMAs, vm_pgoff equals vm_start >> PAGE_SHIFT,

This is completely incorrect.

For anonymous VMAs:

vm_pgoff = vm_start_at_first_fault >> PAGE_SHIFT.

So if you remap a _faulted_ VMA, the vm_start changes, vm_pgoff does
not. The two terms can be COMPLETELY independent.

> so the vm_pgoff and vm_start terms cancel and the result

No, they do not.

> reduces to addr >> (PAGE_SHIFT + order), same as before.

No, it doesn't.

>
> For file-backed VMAs, the sum vm_pgoff + (addr >> PAGE_SHIFT)
> - (vm_start >> PAGE_SHIFT) gives the file page offset of addr.

That's the page offset in a VMA for both anon and file-backed?

(addr - vm_start) >> PAGE_SHIFT is the page offset into a VMA (canonically
determined by linear_page_index())

> Shifting by order gives the correct file chunk index.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

You're claiming we have an incorrect calculation here, but are not
providing a Fixes patch or Cc: stable or sending this separately as a fix?

> Assisted-by: Claude:claude-opus-4-6
> Reviewed-by: Gregory Price <gourry@gourry.net>
> ---
>  mm/mempolicy.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 4e4421b22b59..d139b074a599 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2048,8 +2048,9 @@ struct mempolicy *get_vma_policy(struct vm_area_struct *vma,
>  		pol = get_task_policy(current);
>  	if (pol->mode == MPOL_INTERLEAVE ||
>  	    pol->mode == MPOL_WEIGHTED_INTERLEAVE) {
> -		*ilx += vma->vm_pgoff >> order;
> -		*ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
> +		*ilx += (vma->vm_pgoff +
> +			(addr >> PAGE_SHIFT) -
> +			(vma->vm_start >> PAGE_SHIFT)) >> order;

This is horrible code. Not only are you doing everything in a single
expression for some reason, you're also making the parens confusing and not
explaining what you're doing here at all.

The code before was at least tractable, this is objectively making it
worse.

And anyway, the canonical way to find the page offset into a VMA is
linear_page_index():

static inline pgoff_t linear_page_index(const struct vm_area_struct *vma,
					const unsigned long address)
{
	pgoff_t pgoff;
	pgoff = (address - vma->vm_start) >> PAGE_SHIFT;
	pgoff += vma->vm_pgoff;
	return pgoff;
}

So isn't a far better solution therefore:

	const pgoff_t index = linear_page_index(vma, addr);

	*ilx += index >> order;

Which has the benefit of being readable, uses the canonical method for
determining page offset in the VMA + eliminates the open-coded stuff?

>  	}
>  	return pol;
>  }
> --
> MST
>

Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH v10 02/37] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Lorenzo Stoakes @ 2026-06-08  9:43 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli, Miaohe Lin
In-Reply-To: <df06b66fe4ff8e925ee0714955abc2183a727b90.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:34:23AM -0400, Michael S. Tsirkin wrote:
> TestSetPageHWPoison() is called without zone->lock, so its atomic
> update to page->flags can race with non-atomic flag operations
> that run under zone->lock in the buddy allocator.
>
> In particular, __free_pages_prepare() does:
>
>     page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>
> This non-atomic read-modify-write, while correctly excluding
> __PG_HWPOISON from the mask, can still lose a concurrent
> TestSetPageHWPoison if the read happens before the poison bit
> is set and the write happens after.  Follow-up patches in this
> series add similar non-atomic flag operations as well.
>
> Fix by acquiring zone->lock around TestSetPageHWPoison and
> around ClearPageHWPoison in the retry path.  This
> serializes with all buddy flag manipulation.  The cost is
> negligible: one lock/unlock in an extremely rare path
> (hardware memory errors).
>
> Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
> in this file operate on pages already removed from the buddy
> allocator or on non-buddy pages (DAX, hugetlb), so they do not
> need zone->lock protection.
>
> Acked-by: Miaohe Lin <linmiaohe@huawei.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Can we have Fixes: and Cc: stable and also send this separately please?

These patches seem like unrelated fixups that you've discovered along the way,
and don't belong as part of the already rather large series, unless I'm missing
something here.

Thanks, Lorenzo

> Assisted-by: Claude:claude-opus-4-6
> ---
>  mm/memory-failure.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index ee42d4361309..3880486028a1 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2348,6 +2348,8 @@ int memory_failure(unsigned long pfn, int flags)
>  	unsigned long page_flags;
>  	bool retry = true;
>  	int hugetlb = 0;
> +	struct zone *zone;
> +	unsigned long mf_flags;
>
>  	if (!sysctl_memory_failure_recovery)
>  		panic("Memory failure on page %lx", pfn);
> @@ -2390,7 +2392,11 @@ int memory_failure(unsigned long pfn, int flags)
>  	if (hugetlb)
>  		goto unlock_mutex;
>
> +	/* Serialize with non-atomic buddy flag operations */
> +	zone = page_zone(p);
> +	spin_lock_irqsave(&zone->lock, mf_flags);
>  	if (TestSetPageHWPoison(p)) {
> +		spin_unlock_irqrestore(&zone->lock, mf_flags);
>  		res = -EHWPOISON;
>  		if (flags & MF_ACTION_REQUIRED)
>  			res = kill_accessing_process(current, pfn, flags);
> @@ -2399,6 +2405,7 @@ int memory_failure(unsigned long pfn, int flags)
>  		action_result(pfn, MF_MSG_ALREADY_POISONED, MF_FAILED);
>  		goto unlock_mutex;
>  	}
> +	spin_unlock_irqrestore(&zone->lock, mf_flags);
>
>  	/*
>  	 * We need/can do nothing about count=0 pages.
> @@ -2420,7 +2427,10 @@ int memory_failure(unsigned long pfn, int flags)
>  			} else {
>  				/* We lost the race, try again */
>  				if (retry) {
> +					/* Serialize with non-atomic buddy flag operations */
> +					spin_lock_irqsave(&zone->lock, mf_flags);
>  					ClearPageHWPoison(p);
> +					spin_unlock_irqrestore(&zone->lock, mf_flags);
>  					retry = false;
>  					goto try_again;
>  				}
> --
> MST
>

^ permalink raw reply

* Re: [PATCH v1] vsock/virtio: rework MSG_ZEROCOPY flag handling
From: David Laight @ 2026-06-08  9:37 UTC (permalink / raw)
  To: Arseniy Krasnov
  Cc: Stefan Hajnoczi, Stefano Garzarella, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin,
	Jason Wang, Bobby Eshleman, Xuan Zhuo, Eugenio Pérez,
	Simon Horman, kvm, virtualization, netdev, linux-kernel, oxffffaa,
	rulkc
In-Reply-To: <e5200406-d336-4ddf-9abd-78fccda24546@rulkc.org>

On Mon, 8 Jun 2026 11:10:24 +0300
Arseniy Krasnov <avkrasnov@rulkc.org> wrote:

> On 05/06/2026 18:08, David Laight wrote:
> > On Fri,  5 Jun 2026 14:53:14 +0300
> > Arseniy Krasnov <avkrasnov@rulkc.org> wrote:
> >  
> >> Logically it was based on TCP implementation, so to make further
> >> support easier, rewrite it in the TCP way.
> >>
> >> Signed-off-by: Arseniy Krasnov <avkrasnov@rulkc.org>
> >> ---
> >>  net/vmw_vsock/virtio_transport_common.c | 64 ++++++++++++-------------
> >>  1 file changed, 32 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> >> index 2fd9eaaf5ca6..00caeeaa5590 100644
> >> --- a/net/vmw_vsock/virtio_transport_common.c
> >> +++ b/net/vmw_vsock/virtio_transport_common.c
> >> @@ -73,10 +73,13 @@ static bool virtio_transport_can_zcopy(const struct virtio_transport *t_ops,
> >>  static int virtio_transport_fill_skb(struct sk_buff *skb,
> >>  				     struct virtio_vsock_pkt_info *info,
> >>  				     size_t len,
> >> -				     bool zcopy)
> >> +				     bool zcopy, struct ubuf_info *uarg)
> >>  {
> >>  	struct msghdr *msg = info->msg;
> >>  
> >> +	/* We have completion - attach it to 'skb'. */
> >> +	skb_zcopy_set(skb, uarg, NULL);
> >> +
> >>  	if (zcopy)
> >>  		return __zerocopy_sg_from_iter(msg, NULL, skb,
> >>  					       &msg->msg_iter, len, NULL);
> >> @@ -208,7 +211,8 @@ static struct sk_buff *virtio_transport_alloc_skb(struct virtio_vsock_pkt_info *
> >>  						  u32 src_cid,
> >>  						  u32 src_port,
> >>  						  u32 dst_cid,
> >> -						  u32 dst_port)
> >> +						  u32 dst_port,
> >> +						  struct ubuf_info *uarg)
> >>  {
> >>  	struct vsock_sock *vsk;
> >>  	struct sk_buff *skb;
> >> @@ -245,7 +249,7 @@ static struct sk_buff *virtio_transport_alloc_skb(struct virtio_vsock_pkt_info *
> >>  	if (info->msg && payload_len > 0) {
> >>  		int err;
> >>  
> >> -		err = virtio_transport_fill_skb(skb, info, payload_len, zcopy);
> >> +		err = virtio_transport_fill_skb(skb, info, payload_len, zcopy, uarg);
> >>  		if (err)
> >>  			goto out;
> >>  
> >> @@ -321,38 +325,36 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> >>  	if (pkt_len == 0 && info->op == VIRTIO_VSOCK_OP_RW)
> >>  		return pkt_len;
> >>  
> >> -	if (info->msg) {
> >> -		/* If zerocopy is not enabled by 'setsockopt()', we behave as
> >> -		 * there is no MSG_ZEROCOPY flag set.
> >> +	if (info->msg && (info->msg->msg_flags & MSG_ZEROCOPY)) {
> >> +		/* If 'info->msg' is not NULL, this is only VIRTIO_VSOCK_OP_RW.
> >> +		 * 'MSG_ZEROCOPY' flag handling here is based on the same flag
> >> +		 * handling from 'tcp_sendmsg_locked()'.
> >>  		 */
> >> -		if (!sock_flag(sk_vsock(vsk), SOCK_ZEROCOPY))
> >> -			info->msg->msg_flags &= ~MSG_ZEROCOPY;
> >> +		if (info->msg->msg_ubuf) {
> >> +			uarg = info->msg->msg_ubuf;
> >> +			can_zcopy = virtio_transport_can_zcopy(t_ops, info, pkt_len);
> >> +		} else if (sock_flag(sk_vsock(vsk), SOCK_ZEROCOPY)) {
> >> +			uarg = msg_zerocopy_realloc(sk_vsock(vsk), pkt_len,
> >> +						    NULL, false);
> >> +			if (!uarg) {
> >> +				virtio_transport_put_credit(vvs, pkt_len);
> >> +				return -ENOMEM;
> >> +			}
> >>  
> >> -		if (info->msg->msg_flags & MSG_ZEROCOPY)
> >>  			can_zcopy = virtio_transport_can_zcopy(t_ops, info, pkt_len);
> >>  
> >> +			if (!can_zcopy)
> >> +				uarg_to_msgzc(uarg)->zerocopy = 0;
> >> +
> >> +			have_uref = true;
> >> +		}
> >> +
> >> +		/* 'can_zcopy' means that this transmission will be
> >> +		 * in zerocopy way (e.g. using 'frags' array).
> >> +		 */  
> > I've not looked at the tcp code, but the above doesn't look right.
> > I don't see why msg->msg_ubuf might be non-NULL without SOCK_ZEROCOPY set.
> > That would give the outer code a callback when the last skb is freed but
> > still copy the data.  
> 
> Hi, 
> 
> I guess case when 'msg->msg_ubuf' is non-NULL is special case today for io_uring MSG_ZEROCOPY implementation.
> It was added here https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=eb315a7d1396b1139fc7daea55f2d3191e8e7092
> As I see implementation of its tests in tools/testing/selftests/net/io_uring_zerocopy_tx.c , it doesn't require setting SOCK_ZEROCOPY option for
> socket, so for virtio vsock case I just copied same logic to maintain compatibility, because there is MSG_ZEROCOPY io_uring test for virtio/vsock.

That code path probably sets info->msg->msg_flags & MSG_ZEROCOPY so wants 'zerocopy'.
But wants it's own callback function called rather than one that msg_zerocopy_realloc()
adds.

But there is no reason why a caller might not just want a notification that
all the skb associated with the sendmsg have been freed without requesting zerocopy.
Maybe no one does it today, but it is trivial to support.

> 
> 
> >
> > I also don't see the point of calling msg_zerocopy_realloc() to get a
> > callback when the last skb is freed and then setting
> > 	uarg_to_msgzc(uarg)->zerocopy = 0;
> > so that the callback doesn't actually do anything.
> > It isn't as though you 'find out' later on that you can't actually do
> > zerocopy.  
> 
> 
> Sorry, what do You mean "last skb" ? In this code we first allocate uarg (allocate, because third arg is always NULL). Then in
> loop we allocate sk_buffs, fill it with data and send. I mean first/last skb will be freed after uarg is already allocated and we
> don't touch it. I think i didn't understand Your question here.

The 'uarg' is referenced by all of the skb that contain data for the sendmsg().
So when the last one of them is freed the callback function is called.
The purpose of that callback is to 'undo' the zerocopy (page pinning etc).
But when you set uarg_to_msgzc(uarg)->zerocopy = 0 the callback does nothing.
So there is no point setting up the callback at all.

-- David

> 
> 
> >  
> >>  		if (can_zcopy)
> >>  			max_skb_len = min_t(u32, VIRTIO_VSOCK_MAX_PKT_BUF_SIZE,
> >>  					    (MAX_SKB_FRAGS * PAGE_SIZE));
> >> -
> >> -		if (info->msg->msg_flags & MSG_ZEROCOPY &&
> >> -		    info->op == VIRTIO_VSOCK_OP_RW) {
> >> -			uarg = info->msg->msg_ubuf;
> >> -
> >> -			if (!uarg) {
> >> -				uarg = msg_zerocopy_realloc(sk_vsock(vsk),
> >> -							    pkt_len, NULL, false);
> >> -				if (!uarg) {
> >> -					virtio_transport_put_credit(vvs, pkt_len);
> >> -					return -ENOMEM;
> >> -				}
> >> -
> >> -				if (!can_zcopy)
> >> -					uarg_to_msgzc(uarg)->zerocopy = 0;
> >> -
> >> -				have_uref = true;
> >> -			}
> >> -		}
> >>  	}
> >>  
> >>  	rest_len = pkt_len;
> >> @@ -365,14 +367,12 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> >>  
> >>  		skb = virtio_transport_alloc_skb(info, skb_len, can_zcopy,
> >>  						 src_cid, src_port,
> >> -						 dst_cid, dst_port);
> >> +						 dst_cid, dst_port, uarg);
> >>  		if (!skb) {
> >>  			ret = -ENOMEM;
> >>  			break;
> >>  		}
> >>  
> >> -		skb_zcopy_set(skb, uarg, NULL);  
> > Aren't you passing uarg through two function calls instead of doing it here.
> > Doesn't even make it clearer what is going on.  
> 
> 
> Agree, to simplify patch, uarg could be set earlier (without passing it to functions) I guess.
> 
> Thanks
> 
> 
> >
> > -- David
> >  
> >> -
> >>  		virtio_transport_inc_tx_pkt(vvs, skb);
> >>  
> >>  		ret = t_ops->send_pkt(skb, info->net);
> >> @@ -1178,7 +1178,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> >>  					   le64_to_cpu(hdr->dst_cid),
> >>  					   le32_to_cpu(hdr->dst_port),
> >>  					   le64_to_cpu(hdr->src_cid),
> >> -					   le32_to_cpu(hdr->src_port));
> >> +					   le32_to_cpu(hdr->src_port), NULL);
> >>  	if (!reply)
> >>  		return -ENOMEM;
> >>    


^ permalink raw reply

* Re: [PATCH v10 00/37] mm/virtio: skip redundant zeroing of host-zeroed pages
From: Lorenzo Stoakes @ 2026-06-08  9:17 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <cover.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:33:46AM -0400, Michael S. Tsirkin wrote:
> Further, on architectures with aliasing caches, upstream with init_on_alloc
> double-zeros user pages: once via kernel_init_pages() in
> post_alloc_hook, and again via clear_user_highpage() at the
> callsite (because user_alloc_needs_zeroing() returns true).
> This series eliminates that double-zeroing by moving the zeroing
> into the post_alloc_hook + propagating the "host
> already zeroed this page" information through the buddy allocator.
>
> For page reporting, VIRTIO_BALLOON_F_DEVICE_INIT_REPORTED (bit 6)
> is used. For the inflate/deflate path,
> VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE (bit 7) is used.
>
> Virtio spec: https://lore.kernel.org/all/cover.1778140241.git.mst@redhat.com
>
> Based on v7.1-rc6.  When applying on mm-unstable, two conflicts
> are expected:
> - kernel_init_pages() was renamed to clear_highpages_kasan_tagged()
>   in mm-unstable.  Use clear_highpages_kasan_tagged() in the
>   post_alloc_hook else branch.
> - FPI_PREPARED uses BIT(3) in mm-unstable.  Bump FPI_ZEROED to
>   BIT(4).
> Build-tested on mm-unstable at e9dd96806dbc:
> https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git zero-mm-unstable
>
> Patches 1-5: fixes/cleanups, dependencies of the zeroing patches.
> Patches 6-9: thread user_addr through page allocator, contig API,
>   and gigantic hugetlb allocation.
> Patches 10-16: folio_zero_user in post_alloc_hook, vma_alloc_zeroed
>   conversion, raw fault address threading.
> Patches 17-24: PG_zeroed flag, aliasing guard, buddy merge/split
>   tracking, FPI_ZEROED optimization, folio_put_zeroed.
> Patches 25-27: __GFP_ZERO callsite conversions (alloc_anon_folio,
>   vma_alloc_anon_folio_pmd) with memcg charge failure mitigation.
> Patches 28-29: hugetlb __GFP_ZERO + HPG_zeroed.
> Patches 30-35: page reporting zeroing (DEVICE_INIT_REPORTED),
>   disable indirect descriptors.
> Patches 36-37: inflate/deflate zeroing (DEVICE_INIT_ON_INFLATE).

This seems far too much for one series.

YOu're doing a bunch of mm stuff that seems relatively independent, then
putting the virtio stuff on top.

I think this should be broken out into separate series laying foundations
rather than doing it all in one go, which is also difficult for review
purposes.

Adding a new folio flag is contentious also for instance, we maybe want to
go bit-by-bit and ensure that each foundational element is acceptable
before doing the next bit rather than having it as part of a big series.

Looking through the changelog only adds to this feeling! Huge numbers of
changes, even relatively recently and I'm not sure all relevant maintainers
in mm have had a look through either.

Thanks, Lorenzo

>
> -------
>
> Performance with THP enabled on a 2GB VM, 1 vCPU, allocating
> 256MB of anonymous pages:
>
>   metric         baseline            optimized           delta
>   task-clock     232 +- 20 ms        51 +- 26 ms         -78%
>   cache-misses   1.20M +- 248K       288K +- 102K        -76%
>   instructions   16.3M +- 1.2M       13.8M +- 1.0M       -15%
>
> With hugetlb surplus pages:
>
>   metric         baseline            optimized           delta
>   task-clock     219 +- 23 ms        65 +- 34 ms         -70%
>   cache-misses   1.17M +- 391K       263K +- 36K         -78%
>   instructions   17.9M +- 1.2M       15.1M +- 724K       -16%
>
> Two flags track known-zero pages:
>   PG_zeroed (aliased to PG_private) marks buddy allocator pages that
>   are known to contain all zeros, either because the host zeroed
>   them during page reporting, or because they were freed via the
>   balloon deflate path.  It lives on free-list pages and is consumed
>   by post_alloc_hook() on allocation.
>   HPG_zeroed (stored in hugetlb folio->private bits) serves the same
>   purpose for hugetlb pool pages, which are kept in a pool and may
>   be zeroed long after buddy allocation, so PG_zeroed (consumed at
>   allocation time) cannot track their state.
>
> PG_zeroed lifecycle:
>
>   Sets PG_zeroed:
>   - page_reporting_drain: on reported pages when host zeroes them
>   - __free_pages_ok / __free_frozen_pages: when FPI_ZEROED is set
>     (balloon deflate path)
>   - buddy merge: on merged page if both buddies were zeroed
>   - expand(): propagate to split-off buddy sub-pages
>
>   Clears PG_zeroed:
>   - __free_pages_prepare: clears all PAGE_FLAGS_CHECK_AT_PREP flags
>     (PG_zeroed included), preventing PG_private aliasing leaks
>   - rmqueue_buddy / __rmqueue_pcplist: read-then-clear, passes
>     zeroed hint to prep_new_page -> post_alloc_hook
>   - __isolate_free_page: clear (compaction/page_reporting isolation)
>   - compaction, alloc_contig, split_free_frozen: clear before use
>   - buddy merge: clear both pages before merge, then conditionally
>     re-set on merged head if both were zeroed
>
> HPG_zeroed lifecycle (hugetlb pool pages, stored in folio->private):
>
>   Sets HPG_zeroed:
>   - alloc_surplus_hugetlb_folio: after buddy allocation with
>     __GFP_ZERO, mark pool page as known-zero
>
>   Clears HPG_zeroed:
>   - free_huge_folio: page was mapped to userspace, no longer
>     known-zero when it returns to the pool
>   - alloc_hugetlb_folio: cleared unconditionally on output
>   - alloc_hugetlb_folio_reserve: cleared after checking
>
> - The optimization is most effective with THP, where entire 2MB
>   pages are allocated directly from reported order-9+ buddy pages.
>   Without THP, only ~21% of order-0 allocations come from reported
>   pages due to low-order fragmentation.
> - Persistent hugetlb pool pages are not covered: when freed by
>   userspace they return to the hugetlb free pool, not the buddy
>   allocator, so they are never reported to the host.  Surplus
>   hugetlb pages are allocated from buddy and do benefit.
>
> - PG_zeroed is aliased to PG_private.  __free_pages_prepare() clears it
>   (preventing filesystem PG_private from leaking as false PG_zeroed).
>   FPI_ZEROED re-sets it after prepare for balloon deflate pages.
>   Is aliasing PG_private acceptable, or should a different bit be used?
>
> - With __GFP_ZERO, the folio is zeroed before mem_cgroup_charge().
>   If the charge fails (cgroup at limit), the zeroing work is wasted
>   and the folio is freed and retried at a smaller order.  Previously,
>   zeroing was done after a successful charge.  This is inherent to
>   the __GFP_ZERO approach.  Is this acceptable?
>
> - On architectures with aliasing caches, upstream with init_on_alloc
>   double-zeros user pages: once via kernel_init_pages() in
>   post_alloc_hook, and again via clear_user_highpage() at the
>   callsite (because user_alloc_needs_zeroing() returns true).
>   Our patches eliminate this by zeroing once via folio_zero_user()
>   in post_alloc_hook.  Not a critical fix (people who set init_on_alloc
>   know they are paying performance) but a nice cleanup anyway.
>
> Test program:
>
>   #include <stdio.h>
>   #include <stdlib.h>
>   #include <string.h>
>   #include <sys/mman.h>
>
>   #ifndef MADV_POPULATE_WRITE
>   #define MADV_POPULATE_WRITE 23
>   #endif
>   #ifndef MAP_HUGETLB
>   #define MAP_HUGETLB 0x40000
>   #endif
>
>   int main(int argc, char **argv)
>   {
>       unsigned long size;
>       int flags = MAP_PRIVATE | MAP_ANONYMOUS;
>       void *p;
>       int r;
>
>       if (argc < 2) {
>           fprintf(stderr, "usage: %s <size_mb> [huge]\n", argv[0]);
>           return 1;
>       }
>       size = atol(argv[1]) * 1024UL * 1024;
>       if (argc >= 3 && strcmp(argv[2], "huge") == 0)
>           flags |= MAP_HUGETLB;
>       p = mmap(NULL, size, PROT_READ | PROT_WRITE, flags, -1, 0);
>       if (p == MAP_FAILED) {
>           perror("mmap");
>           return 1;
>       }
>       r = madvise(p, size, MADV_POPULATE_WRITE);
>       if (r) {
>           perror("madvise");
>           return 1;
>       }
>       munmap(p, size);
>       return 0;
>   }
>
> Test script (bench.sh):
>
>   #!/bin/bash
>   # Usage: bench.sh <size_mb> <iterations> [huge]
>   # Feature negotiation (DEVICE_INIT_REPORTED/ON_INFLATE) is
>   # handled by QEMU command line flags,
>   SZ=${1:-256}; ITER=${2:-10}; HUGE=${3:-}
>   FLUSH=/sys/module/page_reporting/parameters/flush
>   CSV=/tmp/perf.csv
>   rmmod virtio_balloon 2>/dev/null
>   insmod /mnt/share/virtio_balloon.ko
>   echo 512 > $FLUSH
>   [ "$HUGE" = "huge" ] && echo $((SZ/2)) > /proc/sys/vm/nr_overcommit_hugepages
>   rm -f $CSV
>   echo "=== sz=${SZ}MB iter=$ITER $HUGE ==="
>   for i in $(seq 1 $ITER); do
>       echo 3 > /proc/sys/vm/drop_caches
>       echo 512 > $FLUSH
>       perf stat -e task-clock,instructions,cache-misses \
>           -x, -o $CSV --append -- /mnt/share/alloc_once $SZ $HUGE
>   done
>   [ "$HUGE" = "huge" ] && echo 0 > /proc/sys/vm/nr_overcommit_hugepages
>   rmmod virtio_balloon
>   awk -F, '/^#/||/^$/{next}{v=$1+0;e=$3;gsub(/ /,"",e);s[e]+=v;ss[e]+=v*v;n[e]++}
>   END{for(e in s){a=s[e]/n[e];d=sqrt(ss[e]/n[e]-a*a);printf "  %-16s %10.0f +- %8.0f (n=%d)\n",e,a,d,n[e]}}' $CSV
>
> Compile and run:
>   gcc -static -O2 -o alloc_once alloc_once.c
>   bash bench.sh 256 10            # regular pages
>   bash bench.sh 256 10 huge       # hugetlb surplus
>
> Note about Sashiko (sashiko.dev) false positives:
>   Sashiko's mm-alloc guideline says "Any optimization replacing
>   clear_user_highpage() with __GFP_ZERO is wrong on [cache-aliasing]
>   architectures". This is correct for mainline but not for this
>   series, which threads user_addr through the allocator so that
>   post_alloc_hook() calls folio_zero_user() with the dcache flush.
>   Suggested guideline update: add "unless the caller passes a
>   valid user address (i.e. not USER_ADDR_NONE) to vma_alloc_folio(),
>   alloc_contig_frozen_pages_user() etc., which reaches
>   post_alloc_hook() for the dcache flush".
>
> Pre-existing bugs found during review (not fixed, not made worse):
>   - do_swap_page() returns VM_FAULT_OOM on large-folio swapin race
>     instead of retrying.
>   - free_huge_folio() called with refcount==1 on
>     mem_cgroup_charge_hugetlb failure.
>   - memfd_alloc_folio() double-decrements resv_huge_pages on error.
>   - wait_event in virtballoon_free_page_report hangs on broken
>     virtqueue (pre-existing, same as old single-buffer code).
>   - tell_host() GFP_KERNEL under balloon_lock risks OOM deadlock.
>
> Changes since v9:
> - Fix W=1 kerneldoc warning on alloc_contig_frozen_pages_user_noprof.
> - Fix link error on !MMU configs (m68k, arm allnoconfig): move
>   folio_zero_user stub to new mm/folio_zero.h header.
> - Reorder patches: move PG_zeroed tracking and folio_put_zeroed
>   before __GFP_ZERO conversions, allowing folio_put_zeroed to
>   handle memcg charge failures.
> - Better handle memcg charge failures.
>
> Changes since v8 (address Sashiko v8 review findings):
> - Fix mempolicy interleave: combine vm_pgoff and VMA offset into
>   a single expression before shifting, fixing carry loss for
>   file-backed VMAs with unaligned vm_pgoff.
> - Fix memory-failure: wrap ClearPageHWPoison in retry path with
>   zone->lock (same race as TestSetPageHWPoison).
> - Fix stale comment: "folio_zero_user writes" -> "page zeroing"
>   in huge_memory.c __folio_mark_uptodate comment.
> - Drop rounddown_pow_of_two for page reporting capacity (no-op
>   for compiler optimization, halves batch size for non-power-of-2).
> - Reorder: move "mm: balloon: use put_page_zeroed" before
>   "virtio_balloon: implement DEVICE_INIT_ON_INFLATE" so the
>   ClearPageZeroed handling is in place before any page gets
>   the flag set.
> - Various commit log improvements (PowerPC note in aliasing
>   patch, memory-failure note about other HWPoison calls,
>   wording fixes).
>
> Changes since v7 (address Sashiko AI review findings):
> - Fix dcache flush on VIPT aliasing architectures: add
>   user_alloc_needs_zeroing() guard in post_alloc_hook to force
>   folio_zero_user for user pages when cache aliasing requires it.
>   Host-zeroed pages excluded (!zeroed).  Optimization preserved.
> - Fix folio_zero_user stub: replace macro with non-inline function
>   in mm/memory.c to avoid double-evaluation and missing include.
> - Fix C89 declaration-after-statement in free_huge_folio.
> - Fix CMA __GFP_ZERO: pass through to cma_alloc_frozen_compound
>   so HPG_zeroed accurately reflects whether page was zeroed.
> - Fix big-endian bitmap: use test_bit_le() for inflate_bitmap.
> - Fix migratepage: clear PageZeroed on old page before deflation.
> - Fix page_reporting flush: overflow-safe loop, add -EINTR on
>   signal, add code comment explaining double flush_delayed_work.
> - Add atomic ClearPageZeroed (CLEARPAGEFLAG) for balloon migration
>   path where zone->lock is not held.
> - Add VM_WARN_ON_ONCE for order>0 without __GFP_COMP in
>   post_alloc_hook (folio_zero_user requires compound metadata).
> - Add _noprof pattern for vma_alloc_zeroed_movable_folio to
>   preserve memory allocation profiling attribution.
> - Add PageReported propagation in split_large_buddy (was missing
>   from patch 2).
> - Add FPI_ZEROED guard: skip PageZeroed when page_poisoning
>   enabled and init_on_free disabled (poison overwrites zeroes).
> - Add DMA alignment comment for inflate_bitmap (ACCESS_PLATFORM
>   cleared, so not needed now).
> - Restore tell_host comment explaining vq buffer assumption.
> - Various code comments documenting design decisions.
> - Drop __GFP_ZERO from gather_surplus_pages: avoid shifting
>   zeroing from fault time to reservation time (mmap/fallocate).
>   Pool pages are zeroed at fault time via alloc_hugetlb_folio.
>   Fresh surplus allocations at fault time still benefit from
>   __GFP_ZERO + HPG_zeroed.
> - New patch: add alloc_contig_frozen_pages_user API with user_addr
>   for cache-friendly zeroing in the contiguous allocation path.
> - New patch: thread user_addr through gigantic hugetlb allocation
>   via alloc_contig_frozen_pages_user.
> - New patch: replace user_alloc_needs_zeroing() with aliasing-only
>   checks (cpu_dcache_is_aliasing || cpu_icache_is_aliasing) in the
>   post_alloc_hook guard.  Avoids redundant re-zero on non-aliasing.
> - New patch: serialize TestSetPageHWPoison with zone->lock in
>   memory_failure to fix pre-existing race with non-atomic buddy
>   flag operations (e.g. page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP).
> - New patch: disable VIRTIO_RING_F_INDIRECT_DESC in balloon to
>   prevent GFP_KERNEL allocation under balloon_lock (OOM deadlock).
> - New patch: skip kernel_init_pages for FPI_ZEROED when page
>   poisoning is not enabled (page already zero, skip redundant work).
>
> Also since v7 (address review by Gregory Price):
> - Drop from_pool bool in alloc_hugetlb_folio: use
>   folio_test_hugetlb_zeroed directly.  HPG_zeroed is set by
>   alloc_surplus_hugetlb_folio for fresh allocations, so the
>   check handles both pool and fresh pages.
> - Drop bool *zeroed output parameter from alloc_hugetlb_folio:
>   sink zeroing inside the function.  When __GFP_ZERO is set and
>   !folio_test_hugetlb_zeroed, call folio_zero_user internally.
> - Rename addr to user_addr in alloc_hugetlb_folio, align
>   internally with huge_page_mask.
> - Add Reviewed-by: Gregory Price tags on reviewed patches.
>
> New patches since v7:
> - mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
> - mm: add alloc_contig_frozen_pages_user for cache-friendly zeroing
> - mm: hugetlb: thread user_addr through gigantic page allocation
> - mm: page_alloc: use aliasing checks instead of
>   user_alloc_needs_zeroing
> - virtio_balloon: disable indirect descriptors
> - mm: page_alloc: skip kernel_init_pages for FPI_ZEROED when safe
>
> Changes since v6 (address review by Gregory Price):
> - Rework hugetlb: use gfp_t parameter instead of bool zero /
>   bool *zeroed.  Sink zeroing inside alloc_hugetlb_folio().
>   Pass raw fault address (user_addr) for cache-friendly zeroing
>   on both pool-page and fresh allocation
>   paths.  (Suggested by Gregory Price)
> - Reorder compaction_alloc_noprof() to call prep_compound_page
>   before post_alloc_hook for consistency.
>   (Suggested by Gregory Price)
> - Reorder: interleave fix first, PageReported propagation and
>   capacity fix moved to front as dependencies.
> - Add USER_ADDR_NONE comments in mmap.c and internal.h explaining why -1 is
>   never a valid userspace address.
> - Fix err uninitialized warning in virtballoon_free_page_report().
> - Lots of commit log tweaks.
>
> Also in v7:
> - Fix hugetlb pool page zeroing to use vmf->real_address
>   (the actual faulting subpage) instead of vmf->address
>   (hugepage-aligned), preserving cache-friendly zeroing
>   locality that upstream had at the callsite.
> - Remove dead/broken alloc_hugetlb_folio !CONFIG_HUGETLB_PAGE
>   stub (returned NULL but callers check IS_ERR).
>
> Changes since v5:
> - Rebased onto v7.1-rc2.
> - Split alloc_anon_folio and alloc_swap_folio raw fault address
>   changes into separate patches.
> - In virtio, move PAGE_POISON check for DEVICE_INIT_REPORTED
>   from probe() to validate(), clearing the feature instead of
>   just gating host_zeroes_pages.  Same for confidential
>   computing check.
> - Fix bisectability: FPI_ZEROED definition and usage now in
>   the same patch.
> - Lots of commit log tweaks.
> - Reorder: REPORTED before ON_INFLATE.
> - Kerneldoc fixes.
>
> Changes since v4:
> With virtio spec posted, update to latest spec:
> - Add VIRTIO_BALLOON_F_DEVICE_INIT_REPORTED (bit 6) for reporting.
> - Add VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE (bit 7) for inflate.
> - Per-page virtqueue submission, per-page used_len feedback.
> - Balloon migration preserves PageZeroed hint.
> - Page_reporting capacity bugfix for small virtqueues.
> - PG_zeroed propagation in split_large_buddy.
> - Disable both features for confidential computing guests.
> - Gate host_zeroes_pages on PAGE_POISON/poison_val: when PAGE_POISON
>   is negotiated with non-zero poison_val, device fills with poison
>   not zeros, so host_zeroes_pages must be false.
> - Disable ON_INFLATE when PAGE_POISON with non-zero poison_val.
> - Bound inflate bitmap reads by used_len from device.
> - Move ON_INFLATE poison_val check to validate() for proper
>   feature negotiation.
> - Fix NUMA interleave index for unaligned VMA start (new patch 1).
> - Drop vma_alloc_folio_user_addr: with the ilx fix, callers can
>   pass raw fault address to vma_alloc_folio directly.
> - Tested with DEBUG_VM, INIT_ON_ALLOC/FREE enabled.
>
> Changes since v3 (address review by Gregory Price and David Hildenbrand):
> - Keep user_addr threading internal: public APIs (__alloc_pages,
>   __folio_alloc, folio_alloc_mpol) are unchanged.  Only internal
>   functions (__alloc_frozen_pages_noprof, __alloc_pages_mpol) carry
>   user_addr.  This eliminates all API churn for external callers.
> - Add vma_alloc_folio_user_addr() (2/22) to separate NUMA policy
>   address from the zeroing hint address.  Fixes NUMA interleave
>   index corruption when passing unaligned fault address for
>   higher-order allocations.
> - Add per-page zeroed_bitmap to page_reporting_dev_info (17/22).
>   The driver's report() callback manages the bitmap.  Drain
>   checks it gated by the host_zeroes_pages static key.  This
>   matches the proposed virtio balloon extension at
>   https://lore.kernel.org/all/cover.1776874126.git.mst@redhat.com/
> - Clear PG_zeroed in __isolate_free_page() to prevent the aliased
>   PG_private flag from leaking to compaction/alloc_contig paths.
> - Do not exclude PG_zeroed from PAGE_FLAGS_CHECK_AT_PREP macro.
>   Instead, __free_pages_prepare() clears it (preventing filesystem
>   PG_private leaking as false PG_zeroed), and FPI_ZEROED sets it
>   after prepare.  Only buddy merge assertion is relaxed.
> - Initialize alloc_context.user_addr in alloc_pages_bulk_noprof.
> - Deflate and hugetlb changes are much smaller now.  Still, the
>   patchset can be merged gradually, if desired.
>
> Changes since v2 (address review by Gregory Price and David Hildenbrand):
> - v2 used pghint_t / vma_alloc_folio_hints API.  v3 switches to
>   threading user_addr through the page allocator and using __GFP_ZERO,
>   so post_alloc_hook() can use folio_zero_user() for cache-friendly
>   zeroing when the user fault address is known.
> - Use FPI_ZEROED to set PG_zeroed after __free_pages_prepare() instead
>   of runtime masking in __free_one_page (further refined in v4).
> - Drop redundant page_poisoning_enabled() check from mm core free
>   path, already guarded at feature negotiation time in
>   virtio_balloon_validate.  The balloon driver keeps its own
>   page_poisoning_enabled_static() check as defense in depth.
> - Split free_frozen_pages_zeroed and put_page_zeroed into separate
>   patches.  David Hildenbrand indicated he intends to rework balloon
>   pages to be frozen (no refcount), at which point put_page_zeroed
>   (21/22) can be dropped and the balloon can call
>   free_frozen_pages_zeroed directly.
> - Use HPG_zeroed flag (in hugetlb folio->private) for hugetlb pool
>   pages instead of PG_zeroed, since pool pages are zeroed long after
>   buddy allocation and PG_zeroed is consumed at allocation time.
> - syzbot CI found a PF_NO_COMPOUND BUG in the v2 pghint_t approach
>   where __ClearPageZeroed was called on compound hugetlb pages in
>   free_huge_folio.  The v3 HPG_zeroed approach avoids this.
> - Remove redundant arch vma_alloc_zeroed_movable_folio overrides
>   on x86, s390, m68k, and alpha (12/22). Suggested by David
>   Hildenbrand.
> - Updated benchmarking script to compute per-run avg +- stddev
>   via awk on CSV output.
>
> Changes v1->v2:
> - Replaced __GFP_PREZEROED with PG_zeroed page flag (aliased PG_private)
> - Added pghint_t type and vma_alloc_folio_hints() API
> - Track PG_zeroed across buddy merges and splits
> - Added post_alloc_hook integration (single consume/clear point)
> - Added hugetlb support (pool pages + memfd)
> - Added page_reporting flush parameter for deterministic testing
> - Added free_frozen_pages_hint/put_page_hint for balloon deflate path
> - Added try_to_claim_block PG_zeroed preservation
> - Updated perf numbers with per-iteration flush methodology
>
> Written with assistance from Claude (claude-opus-4-6).
> Reviewed by cursor-agent (GPT-5.4-xhigh).
> Everything manually read, patchset split and commit logs edited manually.
>
>
> Michael S. Tsirkin (37):
>   mm: mempolicy: fix interleave index calculation
>   mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
>   mm: page_alloc: propagate PageReported flag across buddy splits
>   mm: page_reporting: allow driver to set batch capacity
>   mm: hugetlb: remove dead alloc_hugetlb_folio stub
>   mm: move vma_alloc_folio_noprof to page_alloc.c
>   mm: thread user_addr through page allocator for cache-friendly zeroing
>   mm: add alloc_contig_frozen_pages_user for cache-friendly zeroing
>   mm: hugetlb: thread user_addr through gigantic page allocation
>   mm: add folio_zero_user stub for configs without THP/HUGETLBFS
>   mm: page_alloc: move prep_compound_page before post_alloc_hook
>   mm: use folio_zero_user for user pages in post_alloc_hook
>   mm: use __GFP_ZERO in vma_alloc_zeroed_movable_folio
>   mm: remove arch vma_alloc_zeroed_movable_folio overrides
>   mm: alloc_anon_folio: pass raw fault address to vma_alloc_folio
>   mm: alloc_swap_folio: pass raw fault address to vma_alloc_folio
>   mm: page_reporting: skip redundant zeroing of host-zeroed reported
>     pages
>   mm: page_alloc: use aliasing checks instead of
>     user_alloc_needs_zeroing
>   mm: page_alloc: clear PG_zeroed on buddy merge if not both zero
>   mm: page_alloc: preserve PG_zeroed in page_del_and_expand
>   mm: page_alloc: propagate PG_zeroed in split_large_buddy
>   mm: add free_frozen_pages_zeroed
>   mm: page_alloc: skip kernel_init_pages for FPI_ZEROED when safe
>   mm: add put_page_zeroed and folio_put_zeroed
>   mm: use __GFP_ZERO in alloc_anon_folio
>   mm: vma_alloc_anon_folio_pmd: pass raw fault address to
>     vma_alloc_folio
>   mm: use __GFP_ZERO in vma_alloc_anon_folio_pmd
>   mm: hugetlb: add gfp parameter and skip zeroing for zeroed pages
>   mm: memfd: skip zeroing for zeroed hugetlb pool pages
>   mm: page_reporting: add per-page zeroed bitmap for host feedback
>   virtio_balloon: submit reported pages as individual buffers
>   virtio_balloon: disable indirect descriptors
>   mm: page_reporting: add flush parameter with page budget
>   virtio_balloon: skip zeroing for host-zeroed reported pages
>   virtio_balloon: disable reporting zeroed optimization for confidential
>     guests
>   mm: balloon: use put_page_zeroed for zeroed balloon pages
>   virtio_balloon: implement VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE
>
>  arch/alpha/include/asm/page.h       |   3 -
>  arch/m68k/include/asm/page_no.h     |   3 -
>  arch/s390/include/asm/page.h        |   3 -
>  arch/x86/include/asm/page.h         |   3 -
>  drivers/virtio/virtio_balloon.c     | 177 ++++++++++++++---
>  fs/hugetlbfs/inode.c                |   3 +-
>  include/linux/cma.h                 |   3 +-
>  include/linux/gfp.h                 |  18 +-
>  include/linux/highmem.h             |  15 +-
>  include/linux/hugetlb.h             |  18 +-
>  include/linux/mm.h                  |  13 ++
>  include/linux/page-flags.h          |  11 ++
>  include/linux/page_reporting.h      |  13 ++
>  include/uapi/linux/virtio_balloon.h |   2 +
>  mm/balloon.c                        |  10 +-
>  mm/cma.c                            |   6 +-
>  mm/compaction.c                     |   9 +-
>  mm/folio_zero.h                     |  18 ++
>  mm/huge_memory.c                    |  16 +-
>  mm/hugetlb.c                        | 138 ++++++++-----
>  mm/hugetlb_cma.c                    |   4 +-
>  mm/internal.h                       |  22 ++-
>  mm/memfd.c                          |  14 +-
>  mm/memory-failure.c                 |  10 +
>  mm/memory.c                         |  19 +-
>  mm/mempolicy.c                      |  75 +++----
>  mm/mmap.c                           |   6 +
>  mm/page_alloc.c                     | 297 +++++++++++++++++++++++-----
>  mm/page_reporting.c                 |  99 ++++++++--
>  mm/page_reporting.h                 |  12 ++
>  mm/slub.c                           |   4 +-
>  mm/swap.c                           |  20 +-
>  32 files changed, 792 insertions(+), 272 deletions(-)
>  create mode 100644 mm/folio_zero.h
>
> --
> MST
>

^ permalink raw reply

* Re: [PATCH v10 10/37] mm: add folio_zero_user stub for configs without THP/HUGETLBFS
From: Lorenzo Stoakes @ 2026-06-08  9:12 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <60d48e3c6f03238f385cdb8e48f6d8ffd4cff387.1780906288.git.mst@redhat.com>

On Mon, Jun 08, 2026 at 04:36:02AM -0400, Michael S. Tsirkin wrote:
> folio_zero_user() is defined in mm/memory.c under
> CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_HUGETLBFS.  A subsequent patch
> will call it from post_alloc_hook() for all user page zeroing, so
> configs without THP or HUGETLBFS will need a stub.
>
> Add a stub that uses clear_user_highpages() with aligned
> addr_hint.

It's not a stub? You're doing default behaviour for !THP || hugetlbfs?

>
> Without THP/HUGETLBFS, only order-0 user pages are allocated, so
> the locality optimization in the real folio_zero_user() (zero near
> the faulting address last) is not needed.

The 'real' folio_zero_user()?

But they're both real? In what sense is this unreal?

This commit message is pretty confusing.

> This also matches what vma_alloc_zeroed_movable_folio currently does.

You're also not explaining why we need a random new file just for a single
define, this is really weird?

And please don't add new files without adding an entry to MAINTAINERS.

While we're here, can we just move this function somewhere else? memory.c is
like the junkyard of mm. Maybe util.c.

Anyway this seems like it belongs in mm/internal.h.

>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh
> ---
>  mm/folio_zero.h | 18 ++++++++++++++++++
>  mm/page_alloc.c |  1 +
>  2 files changed, 19 insertions(+)
>  create mode 100644 mm/folio_zero.h
>
> diff --git a/mm/folio_zero.h b/mm/folio_zero.h
> new file mode 100644
> index 000000000000..c135b3a34da8
> --- /dev/null
> +++ b/mm/folio_zero.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef MM_FOLIO_ZERO_H
> +#define MM_FOLIO_ZERO_H
> +
> +#include <linux/highmem.h>
> +
> +#if defined(CONFIG_TRANSPARENT_HUGEPAGE) || defined(CONFIG_HUGETLBFS)
> +void folio_zero_user(struct folio *folio, unsigned long addr_hint);
> +#else

No comment explaining what this does in comparison to the THP/hugetlbfs version?
Let's have a kdoc comment here.

> +static inline void folio_zero_user(struct folio *folio, unsigned long addr_hint)

So we have a folio (physical metadata) and an adress hint?

> +{
> +	unsigned long base = ALIGN_DOWN(addr_hint, folio_size(folio));

const please.

> +
> +	clear_user_highpages(&folio->page, base, folio_nr_pages(folio));
> +}
> +#endif
> +
> +#endif /* MM_FOLIO_ZERO_H */
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 6d3f284c607d..0943ab724032 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -17,6 +17,7 @@
>  #include <linux/stddef.h>
>  #include <linux/mm.h>
>  #include <linux/highmem.h>
> +#include "folio_zero.h"
>  #include <linux/interrupt.h>
>  #include <linux/jiffies.h>
>  #include <linux/compiler.h>
> --
> MST
>

Thanks, Lorenzo

^ permalink raw reply

* [PATCH v10 37/37] virtio_balloon: implement VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE
From: Michael S. Tsirkin @ 2026-06-08  8:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <cover.1780906288.git.mst@redhat.com>

When the device offers DEVICE_INIT_ON_INFLATE (bit 7), the device
initializes inflated pages and returns a per-page bitmap indicating
which pages were successfully initialized.

The driver appends a device-writable bitmap buffer to each inflate
descriptor chain via virtqueue_add_sgs. After the host acknowledges,
the driver checks bitmap bits (bounded by used_len) and marks pages
with SetPageZeroed.

tell_host() returns used_len from virtqueue_get_buf(). Bitmap reads
are bounded: fill_balloon() and virtballoon_migratepage() only trust
bits within the used_len range.

On deflate, release_pages_balloon checks PageZeroed per page and
uses put_page_zeroed for pages the host initialized, propagating
the zeroed hint to the buddy allocator.

If inflate_vq has fewer than 2 descriptors, probe fails with
-ENOSPC. If PAGE_POISON is negotiated with non-zero poison_val,
the feature is cleared in validate().

See the virtio spec change:
https://lore.kernel.org/all/9c69b992c3dd83dfef3db92cd86b2fd8a0730d48.1777731396.git.mst@redhat.com

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Assisted-by: Claude:claude-opus-4-6
Assisted-by: cursor-agent:GPT-5.4-xhigh
---
 drivers/virtio/virtio_balloon.c     | 107 ++++++++++++++++++++++++----
 include/linux/page-flags.h          |   1 +
 include/uapi/linux/virtio_balloon.h |   1 +
 3 files changed, 97 insertions(+), 12 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index bf1172ad5419..0cd4fd06ca2d 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -20,6 +20,7 @@
 #include <linux/mm.h>
 #include <linux/page_reporting.h>
 #include <linux/cc_platform.h>
+#include <asm-generic/bitops/le.h>
 
 /*
  * Balloon device works in 4K page units.  So each page is pointed to by
@@ -122,6 +123,13 @@ struct virtio_balloon {
 	struct virtqueue *reporting_vq;
 	struct page_reporting_dev_info pr_dev_info;
 
+	/* No DMA alignment needed: ACCESS_PLATFORM is cleared,
+	 * so virtio bypasses the DMA API.  If this ever changes,
+	 * add ____dma_from_device_aligned here.
+	 */
+	/* Bitmap returned by host for DEVICE_INIT_ON_INFLATE */
+	DECLARE_BITMAP(inflate_bitmap, VIRTIO_BALLOON_ARRAY_PFNS_MAX);
+
 	/* State for keeping the wakeup_source active while adjusting the balloon */
 	spinlock_t wakeup_lock;
 	bool processing_wakeup_event;
@@ -182,22 +190,33 @@ static void balloon_ack(struct virtqueue *vq)
 	wake_up(&vb->acked);
 }
 
-static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
+static unsigned int tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
 {
-	struct scatterlist sg;
+	struct scatterlist sg_out, sg_in;
+	struct scatterlist *sgs[] = { &sg_out, &sg_in };
 	unsigned int len;
 
-	sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
+	sg_init_one(&sg_out, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
 
 	/* We made sure the vq is large enough so we should always
 	 * be able to add one buffer to an empty queue.
 	 */
-	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
+	if (vq == vb->inflate_vq &&
+	    virtio_has_feature(vb->vdev,
+			       VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE)) {
+		unsigned int bitmap_bytes;
+
+		bitmap_bytes = DIV_ROUND_UP(vb->num_pfns, 8);
+		bitmap_zero(vb->inflate_bitmap, vb->num_pfns);
+		sg_init_one(&sg_in, vb->inflate_bitmap, bitmap_bytes);
+		virtqueue_add_sgs(vq, sgs, 1, 1, vb, GFP_KERNEL);
+	} else {
+		virtqueue_add_outbuf(vq, &sg_out, 1, vb, GFP_KERNEL);
+	}
 	virtqueue_kick(vq);
 
-	/* When host has read buffer, this completes via balloon_ack */
 	wait_event(vb->acked, virtqueue_get_buf(vq, &len));
-
+	return len;
 }
 
 static int virtballoon_free_page_report(struct page_reporting_dev_info *pr_dev_info,
@@ -300,8 +319,37 @@ static unsigned int fill_balloon(struct virtio_balloon *vb, size_t num)
 
 	num_allocated_pages = vb->num_pfns;
 	/* Did we get any? */
-	if (vb->num_pfns != 0)
-		tell_host(vb, vb->inflate_vq);
+	if (vb->num_pfns != 0) {
+		unsigned int used_len = tell_host(vb, vb->inflate_vq);
+
+		if (virtio_has_feature(vb->vdev,
+				       VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE)) {
+			unsigned int i;
+			unsigned int valid_bits = used_len * 8;
+
+			for (i = 0; i < vb->num_pfns;
+			     i += VIRTIO_BALLOON_PAGES_PER_PAGE) {
+				unsigned int pfn, j;
+				bool zeroed = true;
+
+				if (i + VIRTIO_BALLOON_PAGES_PER_PAGE > valid_bits)
+					break;
+				for (j = 0; j < VIRTIO_BALLOON_PAGES_PER_PAGE; j++) {
+					if (!test_bit_le(i + j, vb->inflate_bitmap)) {
+						zeroed = false;
+						break;
+					}
+				}
+				if (zeroed) {
+					pfn = virtio32_to_cpu(vb->vdev,
+							      vb->pfns[i]);
+					SetPageZeroed(pfn_to_page(pfn >>
+						(PAGE_SHIFT -
+						 VIRTIO_BALLOON_PFN_SHIFT)));
+				}
+			}
+		}
+	}
 	mutex_unlock(&vb->balloon_lock);
 
 	return num_allocated_pages;
@@ -314,7 +362,12 @@ static void release_pages_balloon(struct virtio_balloon *vb,
 
 	list_for_each_entry_safe(page, next, pages, lru) {
 		list_del(&page->lru);
-		put_page(page); /* balloon reference */
+		if (PageZeroed(page)) {
+			ClearPageZeroed(page);
+			put_page_zeroed(page);
+		} else {
+			put_page(page);
+		}
 	}
 }
 
@@ -861,8 +914,27 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
 	/* balloon's page migration 1st step  -- inflate "newpage" */
 	vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
 	set_page_pfns(vb, vb->pfns, newpage);
-	tell_host(vb, vb->inflate_vq);
+	{
+		unsigned int used_len = tell_host(vb, vb->inflate_vq);
 
+		if (virtio_has_feature(vb->vdev,
+				       VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE) &&
+		    used_len >= DIV_ROUND_UP(VIRTIO_BALLOON_PAGES_PER_PAGE, 8)) {
+			unsigned int j;
+			bool zeroed = true;
+
+			for (j = 0; j < VIRTIO_BALLOON_PAGES_PER_PAGE; j++) {
+				if (!test_bit_le(j, vb->inflate_bitmap)) {
+					zeroed = false;
+					break;
+				}
+			}
+			if (zeroed)
+				SetPageZeroed(newpage);
+		}
+	}
+
+	ClearPageZeroed(page);
 	/* balloon's page migration 2nd step -- deflate "page" */
 	vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
 	set_page_pfns(vb, vb->pfns, page);
@@ -966,6 +1038,12 @@ static int virtballoon_probe(struct virtio_device *vdev)
 	if (err)
 		goto out_free_vb;
 
+	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE) &&
+	    virtqueue_get_vring_size(vb->inflate_vq) < 2) {
+		err = -ENOSPC;
+		goto out_del_vqs;
+	}
+
 	if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
 		vb->vb_dev_info.adjust_managed_page_count = true;
 #ifdef CONFIG_BALLOON_MIGRATION
@@ -1191,11 +1269,15 @@ static int virtballoon_validate(struct virtio_device *vdev)
 
 	/* Device fills with poison_val, not zeros; disable zeroed hint */
 	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON) &&
-	    !want_init_on_free())
+	    !want_init_on_free()) {
 		__virtio_clear_bit(vdev, VIRTIO_BALLOON_F_DEVICE_INIT_REPORTED);
+		__virtio_clear_bit(vdev, VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE);
+	}
 
-	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
+	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
 		__virtio_clear_bit(vdev, VIRTIO_BALLOON_F_DEVICE_INIT_REPORTED);
+		__virtio_clear_bit(vdev, VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE);
+	}
 	/*
 	 * Balloon submits 1-2 sg entries max per buffer, virtqueue
 	 * sizes are 128+.  Disable indirect descriptors to avoid
@@ -1215,6 +1297,7 @@ static unsigned int features[] = {
 	VIRTIO_BALLOON_F_PAGE_POISON,
 	VIRTIO_BALLOON_F_REPORTING,
 	VIRTIO_BALLOON_F_DEVICE_INIT_REPORTED,
+	VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE,
 };
 
 static struct virtio_driver virtio_balloon_driver = {
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 9365d59ac1d6..caecf0fbf0e2 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -680,6 +680,7 @@ FOLIO_FLAG_FALSE(idle)
  * uses this to skip redundant zeroing in post_alloc_hook().
  */
 __PAGEFLAG(Zeroed, zeroed, PF_NO_COMPOUND)
+SETPAGEFLAG(Zeroed, zeroed, PF_NO_COMPOUND)
 CLEARPAGEFLAG(Zeroed, zeroed, PF_NO_COMPOUND)
 #define __PG_ZEROED (1UL << PG_zeroed)
 
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index 13074631f300..cbaf18e0b17c 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -38,6 +38,7 @@
 #define VIRTIO_BALLOON_F_PAGE_POISON	4 /* Guest is using page poisoning */
 #define VIRTIO_BALLOON_F_REPORTING	5 /* Page reporting virtqueue */
 #define VIRTIO_BALLOON_F_DEVICE_INIT_REPORTED	6 /* Device initializes reported pages */
+#define VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE	7 /* Device initializes pages on inflate */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
-- 
MST


^ permalink raw reply related

* [PATCH v10 36/37] mm: balloon: use put_page_zeroed for zeroed balloon pages
From: Michael S. Tsirkin @ 2026-06-08  8:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <cover.1780906288.git.mst@redhat.com>

When a balloon page marked PageZeroed is freed during migration,
use put_page_zeroed() to propagate the zeroed hint to the buddy
allocator. Previously the hint was silently lost via plain put_page().

No page has PageZeroed set yet; the next patch
(VIRTIO_BALLOON_F_DEVICE_INIT_ON_INFLATE) will set it on
pages the host has zeroed during inflate.
Note: during balloon migration, the migration core holds an
extra reference, so put_page_zeroed() will not be the final
put. The zeroed hint is lost in that case, which is
acceptable: it is a best-effort optimization.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Assisted-by: Claude:claude-opus-4-6
---
 mm/balloon.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/mm/balloon.c b/mm/balloon.c
index 96a8f1e20bc6..6c9dd8ab0c5d 100644
--- a/mm/balloon.c
+++ b/mm/balloon.c
@@ -324,7 +324,15 @@ static int balloon_page_migrate(struct page *newpage, struct page *page,
 	balloon_page_finalize(page);
 	spin_unlock_irqrestore(&balloon_pages_lock, flags);
 
-	put_page(page);
+	if (PageZeroed(page)) {
+		/* Atomic to serialize with memory_failure's
+		 * TestSetPageHWPoison; not under zone->lock here.
+		 */
+		ClearPageZeroed(page);
+		put_page_zeroed(page);
+	} else {
+		put_page(page);
+	}
 
 	return 0;
 }
-- 
MST


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox