Re: [PATCH resend v6 03/30] mm: thread user_addr through page allocator for cache-friendly zeroing

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, May 11, 2026 at 11:37:37AM -0400, Gregory Price wrote:
> On Mon, May 11, 2026 at 05:01:55AM -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.
> > 
> > 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
> 
> This is the nitty-est of all nits, but when doing this can we please
> prefer stack style?
> 
>   vma_alloc_folio 
>     folio_alloc_mpol 
>       __alloc_pages_mpol 
>         __alloc_frozen_pages 
>           get_page_from_freelist 
>             prep_new_page 
>               post_alloc_hook
> 
> Claude has a bad habit of writing changelog changes this way, and it's
> painful for a human to try to read.

Sure.

> > 
> > USER_ADDR_NONE ((unsigned long)-1) is used for non-user
> > allocations, since address 0 is a valid userspace mapping.
> > 
> 
> > +/*
> > + * Sentinel for user_addr: indicates a non-user allocation.
> > + * Cannot use 0 because address 0 is a valid userspace mapping.
> > + */
> > +#define USER_ADDR_NONE	((unsigned long)-1)
> 
> Ehm, hm.  Does -1 hold as a non-user address across all architectures?
> 
> What about in linear addressing / no VM mode?


this is used on a fault. I don't think there are any faults then?
But maybe FAULT_ADDR_NONE would be clearer.

> > 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
> >  
> 
> This change seems out of place unless i'm missing something.
> 

Don't remember. Could be from a change I reverted. I'll look.

> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index f24bf49be047..a999f3ead852 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -1806,7 +1806,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)
> 
>                        user_addr?  uaddr?

ok


> > @@ -1823,7 +1824,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);
> 
> Not on you, but the changes in hugetlb.c as a whole are :[
> 
> We do all of this to pass USER_ADDR_NONE all over the place, but the
> alternative is having a separate function specifically for user-land
> bound allocations.
> 
> So the trade off is:
>    a) churn the current interface for everyone
>    b) add a user_ variant and know people will just get it wrong

I was also explicitly asked not to proliferate too many new APIs.


> IIRC you said the consequence of getting wrong here is subtle corruption
> if a caller got it wrong, and this was related to cache flushing for the
> provided user address?

Yes.

> Stupid question: Does this not apply to kernel allocations as well? Or
> is it simply a matter of the cache having stale data that could leak,
> and therefore it's not a concern in-kernel?
> 
> ~Gregory


Not a concern since we zero through the kernel address.

-- 
MST