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

[Blake Caldwell <blake.caldwell@colorado.edu>]

[-- Attachment #1: Type: text/plain, Size: 3536 bytes --]


> On Jan 20, 2019, at 4:07 PM, Mike Rapoport <rppt@linux.ibm.com> wrote:
> 
> 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?

Only to show what has changed since this was last proposed, but yes, that
change to fs/userfaultfd.c should be squashed with patch 3. The purpose of
patch 4 will only be documenting UFFDIO_REMAP.

I will make those changes for the next revision. Thanks for looking this over.

> 
>> 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.


[-- Attachment #2: Type: text/html, Size: 15365 bytes --]