Re: [PATCH 4/4] userfaultfd: change the direction for UFFDIO_REMAP to out

[Mike Rapoport <rppt@linux.ibm.com>]

Hi,

On Sat, Jan 12, 2019 at 12:36:29AM +0000, Blake Caldwell wrote:
> Moving a page out of a userfaultfd registered region and into a userland
> anonymous vma is needed by the use case of uncooperatively limiting the
> resident size of the userfaultfd region. Reverse the direction of the
> original userfaultfd_remap() to the out direction. Now after memory has
> been removed, subsequent accesses will generate uffdio page fault events.

It took me a while but better late then never :)

Why did you keep this as a separate patch? If the primary use case for
UFFDIO_REMAP to move pages out of userfaultfd region, why not make it so
from the beginning?

> Signed-off-by: Blake Caldwell <blake.caldwell@colorado.edu>
> ---
>  Documentation/admin-guide/mm/userfaultfd.rst | 10 ++++++++++
>  fs/userfaultfd.c                             |  6 +++---
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/admin-guide/mm/userfaultfd.rst b/Documentation/admin-guide/mm/userfaultfd.rst
> index 5048cf6..714af49 100644
> --- a/Documentation/admin-guide/mm/userfaultfd.rst
> +++ b/Documentation/admin-guide/mm/userfaultfd.rst
> @@ -108,6 +108,16 @@ UFFDIO_COPY. They're atomic as in guaranteeing that nothing can see an
>  half copied page since it'll keep userfaulting until the copy has
>  finished.
> 
> +To move pages out of a userfault registered region and into a user vma
> +the UFFDIO_REMAP ioctl can be used. This is only possible for the
> +"OUT" direction. For the "IN" direction, UFFDIO_COPY is preferred
> +since UFFDIO_REMAP requires a TLB flush on the source range at a
> +greater penalty than copying the page. With
> +UFFDIO_REGISTER_MODE_MISSING set, subsequent accesses to the same
> +region will generate a page fault event. This allows non-cooperative
> +removal of memory in a userfaultfd registered vma, effectively
> +limiting the amount of resident memory in such a region.
> +
>  QEMU/KVM
>  ========
> 
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index cf68cdb..8099da2 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1808,10 +1808,10 @@ static int userfaultfd_remap(struct userfaultfd_ctx *ctx,
>  			   sizeof(uffdio_remap)-sizeof(__s64)))
>  		goto out;
> 
> -	ret = validate_range(ctx->mm, uffdio_remap.dst, uffdio_remap.len);
> +	ret = validate_range(current->mm, uffdio_remap.dst, uffdio_remap.len);
>  	if (ret)
>  		goto out;
> -	ret = validate_range(current->mm, uffdio_remap.src, uffdio_remap.len);
> +	ret = validate_range(ctx->mm, uffdio_remap.src, uffdio_remap.len);
>  	if (ret)
>  		goto out;
>  	ret = -EINVAL;
> @@ -1819,7 +1819,7 @@ static int userfaultfd_remap(struct userfaultfd_ctx *ctx,
>  				  UFFDIO_REMAP_MODE_DONTWAKE))
>  		goto out;
> 
> -	ret = remap_pages(ctx->mm, current->mm,
> +	ret = remap_pages(current->mm, ctx->mm,
>  			  uffdio_remap.dst, uffdio_remap.src,
>  			  uffdio_remap.len, uffdio_remap.mode);
>  	if (unlikely(put_user(ret, &user_uffdio_remap->remap)))
> -- 
> 1.8.3.1
> 

-- 
Sincerely yours,
Mike.