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
