Re: [PATCH v2 1/7] userfaultfd: move core VMA manipulation logic to mm/userfaultfd.c

[Lorenzo Stoakes]

On Tue, Jul 09, 2024 at 12:40:12PM GMT, Vlastimil Babka wrote:
> On 7/4/24 9:27 PM, Lorenzo Stoakes wrote:
> > This patch forms part of a patch series intending to separate out VMA logic
> > and render it testable from userspace, which requires that core
> > manipulation functions be exposed in an mm/-internal header file.
> >
> > In order to do this, we must abstract APIs we wish to test, in this
> > instance functions which ultimately invoke vma_modify().
> >
> > This patch therefore moves all logic which ultimately invokes vma_modify()
> > to mm/userfaultfd.c, trying to transfer code at a functional granularity
> > where possible.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
>
> Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx>
>
> > --- a/include/linux/userfaultfd_k.h
> > +++ b/include/linux/userfaultfd_k.h
> > @@ -264,6 +264,25 @@ extern void userfaultfd_unmap_complete(struct mm_struct *mm,
> >  extern bool userfaultfd_wp_unpopulated(struct vm_area_struct *vma);
> >  extern bool userfaultfd_wp_async(struct vm_area_struct *vma);
> >
> > +extern void userfaultfd_reset_ctx(struct vm_area_struct *vma);
> > +
> > +extern struct vm_area_struct *userfaultfd_clear_vma(struct vma_iterator *vmi,
> > +						    struct vm_area_struct *prev,
> > +						    struct vm_area_struct *vma,
> > +						    unsigned long start,
> > +						    unsigned long end);
> > +
> > +int userfaultfd_register_range(struct userfaultfd_ctx *ctx,
> > +			       struct vm_area_struct *vma,
> > +			       unsigned long vm_flags,
> > +			       unsigned long start, unsigned long end,
> > +			       bool wp_async);
> > +
> > +extern void userfaultfd_release_new(struct userfaultfd_ctx *ctx);
> > +
> > +extern void userfaultfd_release_all(struct mm_struct *mm,
> > +				    struct userfaultfd_ctx *ctx);
> > +
>
> Nit: the externs are superfluous. AFAIU the tribal knowledge (or is it
> documented?), we don't add them even if other declarations around have them,
> but we don't also actively remove them unless the lines are touched for
> other reasons. So the declarations are inconsistent but slowly move towards
> no externs.

Oh right, I was just keeping it consistent with existing decls (apart from
uffd_register_range() for some reason). Am aware the extern's are
superfluous here so this was the only reason.

I think as it's a nitty thing let's keep it as-is for now, I can do a
follow up patch perhaps to correct.

>
> >  #else /* CONFIG_USERFAULTFD */
> >
> >  /* mm helpers */
>