Re: [PATCH v2 03/15] userfaultfd: introduce mfill_get_pmd() helper.

[Mike Rapoport <rppt@kernel.org>]

On Fri, Mar 20, 2026 at 01:55:29PM +0100, David Hildenbrand (Arm) wrote:
> On 3/6/26 18:18, Mike Rapoport wrote:
> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> 
> Nit: "." at the end of the patch subject
 
Oops :/
 
> > +static int mfill_get_pmd(struct mfill_state *state)
> > +{
> > +	struct mm_struct *dst_mm = state->ctx->mm;
> > +	pmd_t *dst_pmd;
> > +	pmd_t dst_pmdval;
> 
> I'd just have both on a single line.

Can do :) 

> > +	/*
> > +	 * If the dst_pmd is THP don't override it and just be strict.
> > +	 * (This includes the case where the PMD used to be THP and
> > +	 * changed back to none after __pte_alloc().)
> > +	 */
> > +	if (unlikely(!pmd_present(dst_pmdval) || pmd_trans_huge(dst_pmdval)))
> 
> Can we directly switch to pmd_leaf() while touching that?

You mean instead of pmd_trans_huge()?
Yeah, sure.

> > @@ -809,41 +838,15 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx,
> >  	while (state.src_addr < src_start + len) {
> >  		VM_WARN_ON_ONCE(state.dst_addr >= dst_start + len);
> >  
> > -		pmd_t dst_pmdval;
> > -
> > -		dst_pmd = mm_alloc_pmd(dst_mm, state.dst_addr);
> > -		if (unlikely(!dst_pmd)) {
> > -			err = -ENOMEM;
> > +		err = mfill_get_pmd(&state);
> > +		if (err)
> 
> 
> It's a bit odd that a "get" function doesn't return a PMD pointer but
> instead stores it in the state.
> 
> Maybe more like "mfill_prepare_pmd" ? But actually you want to have a
> pte table.
> 
> mfill_prepare_pte_table() or alternatively mfill_alloc_pte_table() /
> mfill_alloc_dst_pte_table()

As it actually allocates the pte table once in 512 times, I'd prefer
mfill_establish_pmd().
 
> -- 
> Cheers,
> David

-- 
Sincerely yours,
Mike.