Re: [PATCH v2 10/13] mm: make vm_area_desc utilise vma_flags_t only

["Liam R. Howlett" <Liam.Howlett@oracle.com>]

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [260122 16:06]:
> Now we have eliminated all uses of vm_area_desc->vm_flags, eliminate this
> field, and have mmap_prepare users utilise the vma_flags_t
> vm_area_desc->vma_flags field only.
> 
> As part of this change we alter is_shared_maywrite() to accept a
> vma_flags_t parameter, and introduce is_shared_maywrite_vm_flags() for use
> with legacy vm_flags_t flags.
> 
> We also update struct mmap_state to add a union between vma_flags and
> vm_flags temporarily until the mmap logic is also converted to using
> vma_flags_t.
> 
> Also update the VMA userland tests to reflect this change.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  include/linux/mm.h               |  9 +++++++--
>  include/linux/mm_types.h         |  5 +----
>  mm/filemap.c                     |  2 +-
>  mm/util.c                        |  2 +-
>  mm/vma.c                         | 11 +++++++----
>  mm/vma.h                         |  3 +--
>  tools/testing/vma/vma_internal.h |  9 +++++++--
>  7 files changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index e31f72a021ef..37e215de3343 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1290,15 +1290,20 @@ static inline bool vma_is_accessible(const struct vm_area_struct *vma)
>  	return vma->vm_flags & VM_ACCESS_FLAGS;
>  }
>  
> -static inline bool is_shared_maywrite(vm_flags_t vm_flags)
> +static inline bool is_shared_maywrite_vm_flags(vm_flags_t vm_flags)
>  {
>  	return (vm_flags & (VM_SHARED | VM_MAYWRITE)) ==
>  		(VM_SHARED | VM_MAYWRITE);
>  }
>  
> +static inline bool is_shared_maywrite(const vma_flags_t *flags)
> +{
> +	return vma_flags_test_all(flags, VMA_SHARED_BIT, VMA_MAYWRITE_BIT);
> +}
> +

Confusing git diff here, but looks okay.

>  static inline bool vma_is_shared_maywrite(const struct vm_area_struct *vma)
>  {
> -	return is_shared_maywrite(vma->vm_flags);
> +	return is_shared_maywrite(&vma->flags);
>  }
>  
>  static inline
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index cdac328b46dc..6d98ff6bc2e5 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -887,10 +887,7 @@ struct vm_area_desc {
>  	/* Mutable fields. Populated with initial state. */
>  	pgoff_t pgoff;
>  	struct file *vm_file;
> -	union {
> -		vm_flags_t vm_flags;
> -		vma_flags_t vma_flags;
> -	};
> +	vma_flags_t vma_flags;
>  	pgprot_t page_prot;
>  
>  	/* Write-only fields. */
> diff --git a/mm/filemap.c b/mm/filemap.c
> index ebd75684cb0a..6cd7974d4ada 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -4012,7 +4012,7 @@ int generic_file_readonly_mmap(struct file *file, struct vm_area_struct *vma)
>  
>  int generic_file_readonly_mmap_prepare(struct vm_area_desc *desc)
>  {
> -	if (is_shared_maywrite(desc->vm_flags))
> +	if (is_shared_maywrite(&desc->vma_flags))
>  		return -EINVAL;
>  	return generic_file_mmap_prepare(desc);
>  }
> diff --git a/mm/util.c b/mm/util.c
> index 97cae40c0209..b05ab6f97e11 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -1154,7 +1154,7 @@ int __compat_vma_mmap(const struct file_operations *f_op,
>  
>  		.pgoff = vma->vm_pgoff,
>  		.vm_file = vma->vm_file,
> -		.vm_flags = vma->vm_flags,
> +		.vma_flags = vma->flags,
>  		.page_prot = vma->vm_page_prot,
>  
>  		.action.type = MMAP_NOTHING, /* Default */
> diff --git a/mm/vma.c b/mm/vma.c
> index 39dcd9ddd4ba..be64f781a3aa 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -15,7 +15,10 @@ struct mmap_state {
>  	unsigned long end;
>  	pgoff_t pgoff;
>  	unsigned long pglen;
> -	vm_flags_t vm_flags;
> +	union {
> +		vm_flags_t vm_flags;
> +		vma_flags_t vma_flags;
> +	};
>  	struct file *file;
>  	pgprot_t page_prot;
>  
> @@ -2369,7 +2372,7 @@ static void set_desc_from_map(struct vm_area_desc *desc,
>  
>  	desc->pgoff = map->pgoff;
>  	desc->vm_file = map->file;
> -	desc->vm_flags = map->vm_flags;
> +	desc->vma_flags = map->vma_flags;
>  	desc->page_prot = map->page_prot;
>  }
>  
> @@ -2650,7 +2653,7 @@ static int call_mmap_prepare(struct mmap_state *map,
>  		map->file_doesnt_need_get = true;
>  		map->file = desc->vm_file;
>  	}
> -	map->vm_flags = desc->vm_flags;
> +	map->vma_flags = desc->vma_flags;
>  	map->page_prot = desc->page_prot;
>  	/* User-defined fields. */
>  	map->vm_ops = desc->vm_ops;
> @@ -2823,7 +2826,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
>  		return -EINVAL;
>  
>  	/* Map writable and ensure this isn't a sealed memfd. */
> -	if (file && is_shared_maywrite(vm_flags)) {
> +	if (file && is_shared_maywrite_vm_flags(vm_flags)) {
>  		int error = mapping_map_writable(file->f_mapping);
>  
>  		if (error)
> diff --git a/mm/vma.h b/mm/vma.h
> index bb7fa5d2bde2..062672df8a65 100644
> --- a/mm/vma.h
> +++ b/mm/vma.h
> @@ -286,8 +286,7 @@ static inline void set_vma_from_desc(struct vm_area_struct *vma,
>  	vma->vm_pgoff = desc->pgoff;
>  	if (desc->vm_file != vma->vm_file)
>  		vma_set_file(vma, desc->vm_file);
> -	if (desc->vm_flags != vma->vm_flags)
> -		vm_flags_set(vma, desc->vm_flags);
> +	vma->flags = desc->vma_flags;
>  	vma->vm_page_prot = desc->page_prot;
>  
>  	/* User-defined fields. */
> diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
> index 2b01794cbd61..2743f12ecf32 100644
> --- a/tools/testing/vma/vma_internal.h
> +++ b/tools/testing/vma/vma_internal.h
> @@ -1009,15 +1009,20 @@ static inline void vma_desc_clear_flags_mask(struct vm_area_desc *desc,
>  #define vma_desc_clear_flags(desc, ...) \
>  	vma_desc_clear_flags_mask(desc, mk_vma_flags(__VA_ARGS__))
>  
> -static inline bool is_shared_maywrite(vm_flags_t vm_flags)
> +static inline bool is_shared_maywrite_vm_flags(vm_flags_t vm_flags)
>  {
>  	return (vm_flags & (VM_SHARED | VM_MAYWRITE)) ==
>  		(VM_SHARED | VM_MAYWRITE);
>  }
>  
> +static inline bool is_shared_maywrite(const vma_flags_t *flags)
> +{
> +	return vma_flags_test_all(flags, VMA_SHARED_BIT, VMA_MAYWRITE_BIT);
> +}
> +
>  static inline bool vma_is_shared_maywrite(struct vm_area_struct *vma)
>  {
> -	return is_shared_maywrite(vma->vm_flags);
> +	return is_shared_maywrite(&vma->flags);
>  }
>  
>  static inline struct vm_area_struct *vma_next(struct vma_iterator *vmi)
> -- 
> 2.52.0
>