Re: [RFC PATCH v2 2/5] userfaultfd: introduce access-likely mode for copy/wp operations

From: Peter Xu <peterx@redhat.com>
To: David Hildenbrand <david@redhat.com>
Cc: Nadav Amit <nadav.amit@gmail.com>,
	linux-mm@kvack.org, Nadav Amit <namit@vmware.com>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Hugh Dickins <hughd@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Mike Rapoport <rppt@linux.ibm.com>
Subject: Re: [RFC PATCH v2 2/5] userfaultfd: introduce access-likely mode for copy/wp operations
Date: Tue, 21 Jun 2022 11:42:57 -0400	[thread overview]
Message-ID: <YrHnAZDTkXFQ2CMB@xz-m1.local> (raw)
In-Reply-To: <506888c0-c257-e2a8-9540-823acdd403db@redhat.com>

On Tue, Jun 21, 2022 at 10:48:51AM +0200, David Hildenbrand wrote:
> > @@ -1871,6 +1877,7 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg)
> >  	struct uffdio_continue uffdio_continue;
> >  	struct uffdio_continue __user *user_uffdio_continue;
> >  	struct userfaultfd_wake_range range;
> > +	uffd_flags_t uffd_flags;
> >  
> >  	user_uffdio_continue = (struct uffdio_continue __user *)arg;
> >  
> > @@ -1898,10 +1905,12 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg)
> >  	if (uffdio_continue.mode & ~UFFDIO_CONTINUE_MODE_DONTWAKE)
> >  		goto out;
> >  
> > +	uffd_flags = UFFD_FLAGS_ACCESS_LIKELY;
> 
> Can we add a comment why that makes sense? I think I know why -- someone
> is stuck waiting for that continue to happen :)

I think we shouldn't apply it by default for CONTINUE at all, at least not
sololy for CONTINUE.  CONTINUE can be used similarly as COPY at least in VM
migration use case, afaict, so we can proactively install pgtables even if
the page was not faulted.

[...]

> > diff --git a/include/uapi/linux/userfaultfd.h b/include/uapi/linux/userfaultfd.h
> > index 005e5e306266..d9c8ce9ba777 100644
> > --- a/include/uapi/linux/userfaultfd.h
> > +++ b/include/uapi/linux/userfaultfd.h
> > @@ -38,7 +38,8 @@
> >  			   UFFD_FEATURE_MINOR_HUGETLBFS |	\
> >  			   UFFD_FEATURE_MINOR_SHMEM |		\
> >  			   UFFD_FEATURE_EXACT_ADDRESS |		\
> > -			   UFFD_FEATURE_WP_HUGETLBFS_SHMEM)
> > +			   UFFD_FEATURE_WP_HUGETLBFS_SHMEM |	\
> > +			   UFFD_FEATURE_ACCESS_HINTS)

Is the access_hint feature gonna cover the next dirty bit patch?  If so I'd
suggest we add the feature declaration in a separate patch after all bits
ready.

Thanks,

-- 
Peter Xu