On Thu, Feb 18, 2021 at 04:48:19PM -0800, Axel Rasmussen wrote:
[...]
> @@ -1290,14 +1299,20 @@ static int userfaultfd_register(struct
> userfaultfd_ctx *ctx,
> ret = -EINVAL;
> if (!uffdio_register.mode)
> goto out;
> - if (uffdio_register.mode & ~(UFFDIO_REGISTER_MODE_MISSING|
> - UFFDIO_REGISTER_MODE_WP))
> + if (uffdio_register.mode & ~UFFD_API_REGISTER_MODES)
> goto out;
> vm_flags = 0;
> if (uffdio_register.mode & UFFDIO_REGISTER_MODE_MISSING)
> vm_flags |= VM_UFFD_MISSING;
> if (uffdio_register.mode & UFFDIO_REGISTER_MODE_WP)
> vm_flags |= VM_UFFD_WP;
> + if (uffdio_register.mode & UFFDIO_REGISTER_MODE_MINOR) {
> + /* VM_UFFD_MINOR == VM_NONE if this arch doesn't support it. */
How about check CONFIG_HAVE_ARCH_USERFAULTFD_MINOR below directly instead of
commenting?
> + ret = -EINVAL;
Should be able to drop this line too since ret is -EINVAL already?
> + if (!VM_UFFD_MINOR)
> + goto out;
> + vm_flags |= VM_UFFD_MINOR;
> + }
[...]
> diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
> index 67018d367b9f..a743a0f9ebde 100644
> --- a/include/trace/events/mmflags.h
> +++ b/include/trace/events/mmflags.h
> @@ -137,6 +137,12 @@ IF_HAVE_PG_ARCH_2(PG_arch_2, "arch_2"
> )
> #define IF_HAVE_VM_SOFTDIRTY(flag,name)
> #endif
>
> +#ifdef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
> +# define IF_HAVE_UFFD_MINOR(flag, name) {flag, name},
> +#else
> +# define IF_HAVE_UFFD_MINOR(flag, name)
> +#endif
> +
> #define __def_vmaflag_names \
> {VM_READ, "read" }, \
> {VM_WRITE, "write" }, \
> @@ -148,6 +154,7 @@ IF_HAVE_PG_ARCH_2(PG_arch_2, "arch_2"
> )
> {VM_MAYSHARE, "mayshare" }, \
> {VM_GROWSDOWN, "growsdown" }, \
> {VM_UFFD_MISSING, "uffd_missing" }, \
> +IF_HAVE_UFFD_MINOR(VM_UFFD_MINOR, "uffd_minor" ) \
> {VM_PFNMAP, "pfnmap" }, \
> {VM_DENYWRITE, "denywrite" }, \
> {VM_UFFD_WP, "uffd_wp" }, \
> @@ -169,7 +176,7 @@ IF_HAVE_VM_SOFTDIRTY(VM_SOFTDIRTY, "softdirty"
> ) \
> {VM_MIXEDMAP, "mixedmap" }, \
> {VM_HUGEPAGE, "hugepage" }, \
> {VM_NOHUGEPAGE, "nohugepage" }, \
> - {VM_MERGEABLE, "mergeable" } \
> + {VM_MERGEABLE, "mergeable" }
This change seems irrelevant.
If you agree with above comments, please feel free to add:
Reviewed-by: Peter Xu <[email protected]>
Thanks,
--
Peter Xu
