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)" <[email protected]>
>
> 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.
