Re: [PATCH] fs/userfaultfd.c: simplify the calculation of new_flags

[Andrea Arcangeli]

Hello,

On Tue, Aug 06, 2019 at 01:38:59PM +0800, Wei Yang wrote:
> Finally new_flags equals old vm_flags *OR* vm_flags.
> 
> It is not necessary to mask them first.
> 
> Signed-off-by: Wei Yang <richardw.yang@xxxxxxxxxxxxxxx>
> ---
>  fs/userfaultfd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index ccbdbd62f0d8..653d8f7c453c 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1457,7 +1457,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
>  			start = vma->vm_start;
>  		vma_end = min(end, vma->vm_end);
>  
> -		new_flags = (vma->vm_flags & ~vm_flags) | vm_flags;
> +		new_flags = vma->vm_flags | vm_flags;
>  		prev = vma_merge(mm, prev, start, vma_end, new_flags,
>  				 vma->anon_vma, vma->vm_file, vma->vm_pgoff,
>  				 vma_policy(vma),

And then how do you clear the flags after the above?

It must be possible to clear the flags (from
UFFDIO_REGISTER_MODE_MISSING|UFFDIO_REGISTER_MODE_WP to only one set
or invert).

We have no WP support upstream yet, so maybe that's why it looks
superfluous in practice, but in theory it isn't because it would then
need to be reversed by Peter's (CC'ed) -wp patchset.

The register code has already the right placeholder to support -wp and
so it's better not to break them.

I would recommend reviewing the uffd-wp support and working on testing
the uffd-wp code instead of changing the above.

Thanks,
Andrea