Re: [PATCH v2 05/13] mm: add basic VMA flag operation helper functions

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

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [260122 16:06]:
> Now we have the mk_vma_flags() macro helper which permits easy
> specification of any number of VMA flags, add helper functions which
> operate with vma_flags_t parameters.
> 
> This patch provides vma_flags_test[_mask](), vma_flags_set[_mask]() and
> vma_flags_clear[_mask]() respectively testing, setting and clearing flags
> with the _mask variants accepting vma_flag_t parameters, and the non-mask
> variants implemented as macros which accept a list of flags.
> 
> This allows us to trivially test/set/clear aggregate VMA flag values as
> necessary, for instance:
> 
> 	if (vma_flags_test(&flags, VMA_READ_BIT, VMA_WRITE_BIT))
> 		goto readwrite;
> 
> 	vma_flags_set(&flags, VMA_READ_BIT, VMA_WRITE_BIT);
> 
> 	vma_flags_clear(&flags, VMA_READ_BIT, VMA_WRITE_BIT);
> 
> We also add a function for testing that ALL flags are set for convenience,
> e.g.:
> 
> 	if (vma_flags_test_all(&flags, VMA_READ_BIT, VMA_MAYREAD_BIT)) {
> 		/* Both READ and MAYREAD flags set */
> 		...
> 	}
> 
> The compiler generates optimal assembly for each such that they behave as
> if the caller were setting the bitmap flags manually.
> 
> This is important for e.g. drivers which manipulate flag values rather than
> a VMA's specific flag values.
> 
> We also add helpers for testing, setting and clearing flags for VMA's and
> VMA descriptors to reduce boilerplate.
> 
> Also add the EMPTY_VMA_FLAGS define to aid initialisation of empty flags.
> 
> Finally, update the userland VMA tests to add the helpers there so they can
> be utilised as part of userland testing.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

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

> ---
>  include/linux/mm.h               | 165 +++++++++++++++++++++++++++++++
>  include/linux/mm_types.h         |   4 +-
>  tools/testing/vma/vma_internal.h | 147 +++++++++++++++++++++++----
>  3 files changed, 295 insertions(+), 21 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 32c3b5347dc6..fd93317193e0 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1059,6 +1059,171 @@ static inline vma_flags_t __mk_vma_flags(size_t count, const vma_flag_t *bits)
>  #define mk_vma_flags(...) __mk_vma_flags(COUNT_ARGS(__VA_ARGS__), \
>  					 (const vma_flag_t []){__VA_ARGS__})
>  
> +/*  Test each of to_test flags in flags, non-atomically. */
> +static __always_inline bool vma_flags_test_mask(const vma_flags_t *flags,
> +		vma_flags_t to_test)
> +{
> +	const unsigned long *bitmap = flags->__vma_flags;
> +	const unsigned long *bitmap_to_test = to_test.__vma_flags;
> +
> +	return bitmap_intersects(bitmap_to_test, bitmap, NUM_VMA_FLAG_BITS);
> +}
> +
> +/*
> + * Test whether any specified VMA flag is set, e.g.:
> + *
> + * if (vma_flags_test(flags, VMA_READ_BIT, VMA_MAYREAD_BIT)) { ... }
> + */
> +#define vma_flags_test(flags, ...) \
> +	vma_flags_test_mask(flags, mk_vma_flags(__VA_ARGS__))
> +
> +/* Test that ALL of the to_test flags are set, non-atomically. */
> +static __always_inline bool vma_flags_test_all_mask(const vma_flags_t *flags,
> +		vma_flags_t to_test)
> +{
> +	const unsigned long *bitmap = flags->__vma_flags;
> +	const unsigned long *bitmap_to_test = to_test.__vma_flags;
> +
> +	return bitmap_subset(bitmap_to_test, bitmap, NUM_VMA_FLAG_BITS);
> +}
> +
> +/*
> + * Test whether ALL specified VMA flags are set, e.g.:
> + *
> + * if (vma_flags_test_all(flags, VMA_READ_BIT, VMA_MAYREAD_BIT)) { ... }
> + */
> +#define vma_flags_test_all(flags, ...) \
> +	vma_flags_test_all_mask(flags, mk_vma_flags(__VA_ARGS__))
> +
> +/* Set each of the to_set flags in flags, non-atomically. */
> +static __always_inline void vma_flags_set_mask(vma_flags_t *flags, vma_flags_t to_set)
> +{
> +	unsigned long *bitmap = flags->__vma_flags;
> +	const unsigned long *bitmap_to_set = to_set.__vma_flags;
> +
> +	bitmap_or(bitmap, bitmap, bitmap_to_set, NUM_VMA_FLAG_BITS);
> +}
> +
> +/*
> + * Set all specified VMA flags, e.g.:
> + *
> + * vma_flags_set(&flags, VMA_READ_BIT, VMA_WRITE_BIT, VMA_EXEC_BIT);
> + */
> +#define vma_flags_set(flags, ...) \
> +	vma_flags_set_mask(flags, mk_vma_flags(__VA_ARGS__))
> +
> +/* Clear all of the to-clear flags in flags, non-atomically. */
> +static __always_inline void vma_flags_clear_mask(vma_flags_t *flags, vma_flags_t to_clear)
> +{
> +	unsigned long *bitmap = flags->__vma_flags;
> +	const unsigned long *bitmap_to_clear = to_clear.__vma_flags;
> +
> +	bitmap_andnot(bitmap, bitmap, bitmap_to_clear, NUM_VMA_FLAG_BITS);
> +}
> +
> +/*
> + * Clear all specified individual flags, e.g.:
> + *
> + * vma_flags_clear(&flags, VMA_READ_BIT, VMA_WRITE_BIT, VMA_EXEC_BIT);
> + */
> +#define vma_flags_clear(flags, ...) \
> +	vma_flags_clear_mask(flags, mk_vma_flags(__VA_ARGS__))
> +
> +/*
> + * Helper to test that ALL specified flags are set in a VMA.
> + *
> + * Note: appropriate locks must be held, this function does not acquire them for
> + * you.
> + */
> +static inline bool vma_test_all_flags_mask(const struct vm_area_struct *vma,
> +					   vma_flags_t flags)
> +{
> +	return vma_flags_test_all_mask(&vma->flags, flags);
> +}
> +
> +/*
> + * Helper macro for checking that ALL specified flags are set in a VMA, e.g.:
> + *
> + * if (vma_test_all_flags(vma, VMA_READ_BIT, VMA_MAYREAD_BIT) { ... }
> + */
> +#define vma_test_all_flags(vma, ...) \
> +	vma_test_all_flags_mask(vma, mk_vma_flags(__VA_ARGS__))
> +
> +/*
> + * Helper to set all VMA flags in a VMA.
> + *
> + * Note: appropriate locks must be held, this function does not acquire them for
> + * you.
> + */
> +static inline void vma_set_flags_mask(struct vm_area_struct *vma,
> +				      vma_flags_t flags)
> +{
> +	vma_flags_set_mask(&vma->flags, flags);
> +}
> +
> +/*
> + * Helper macro for specifying VMA flags in a VMA, e.g.:
> + *
> + * vma_set_flags(vma, VMA_IO_BIT, VMA_PFNMAP_BIT, VMA_DONTEXPAND_BIT,
> + * 		VMA_DONTDUMP_BIT);
> + *
> + * Note: appropriate locks must be held, this function does not acquire them for
> + * you.
> + */
> +#define vma_set_flags(vma, ...) \
> +	vma_set_flags_mask(vma, mk_vma_flags(__VA_ARGS__))
> +
> +/* Helper to test all VMA flags in a VMA descriptor. */
> +static inline bool vma_desc_test_flags_mask(const struct vm_area_desc *desc,
> +					    vma_flags_t flags)
> +{
> +	return vma_flags_test_mask(&desc->vma_flags, flags);
> +}
> +
> +/*
> + * Helper macro for testing VMA flags for an input pointer to a struct
> + * vm_area_desc object describing a proposed VMA, e.g.:
> + *
> + * if (vma_desc_test_flags(desc, VMA_IO_BIT, VMA_PFNMAP_BIT,
> + *		VMA_DONTEXPAND_BIT, VMA_DONTDUMP_BIT)) { ... }
> + */
> +#define vma_desc_test_flags(desc, ...) \
> +	vma_desc_test_flags_mask(desc, mk_vma_flags(__VA_ARGS__))
> +
> +/* Helper to set all VMA flags in a VMA descriptor. */
> +static inline void vma_desc_set_flags_mask(struct vm_area_desc *desc,
> +					   vma_flags_t flags)
> +{
> +	vma_flags_set_mask(&desc->vma_flags, flags);
> +}
> +
> +/*
> + * Helper macro for specifying VMA flags for an input pointer to a struct
> + * vm_area_desc object describing a proposed VMA, e.g.:
> + *
> + * vma_desc_set_flags(desc, VMA_IO_BIT, VMA_PFNMAP_BIT, VMA_DONTEXPAND_BIT,
> + * 		VMA_DONTDUMP_BIT);
> + */
> +#define vma_desc_set_flags(desc, ...) \
> +	vma_desc_set_flags_mask(desc, mk_vma_flags(__VA_ARGS__))
> +
> +/* Helper to clear all VMA flags in a VMA descriptor. */
> +static inline void vma_desc_clear_flags_mask(struct vm_area_desc *desc,
> +					     vma_flags_t flags)
> +{
> +	vma_flags_clear_mask(&desc->vma_flags, flags);
> +}
> +
> +/*
> + * Helper macro for clearing VMA flags for an input pointer to a struct
> + * vm_area_desc object describing a proposed VMA, e.g.:
> + *
> + * vma_desc_clear_flags(desc, VMA_IO_BIT, VMA_PFNMAP_BIT, VMA_DONTEXPAND_BIT,
> + * 		VMA_DONTDUMP_BIT);
> + */
> +#define vma_desc_clear_flags(desc, ...) \
> +	vma_desc_clear_flags_mask(desc, mk_vma_flags(__VA_ARGS__))
> +
>  static inline void vma_set_anonymous(struct vm_area_struct *vma)
>  {
>  	vma->vm_ops = NULL;
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 592ad065fa75..cdac328b46dc 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -844,7 +844,7 @@ struct mmap_action {
>  
>  	/*
>  	 * If specified, this hook is invoked when an error occurred when
> -	 * attempting the selection action.
> +	 * attempting the selected action.
>  	 *
>  	 * The hook can return an error code in order to filter the error, but
>  	 * it is not valid to clear the error here.
> @@ -868,6 +868,8 @@ typedef struct {
>  	DECLARE_BITMAP(__vma_flags, NUM_VMA_FLAG_BITS);
>  } vma_flags_t;
>  
> +#define EMPTY_VMA_FLAGS ((vma_flags_t){ })
> +
>  /*
>   * Describes a VMA that is about to be mmap()'ed. Drivers may choose to
>   * manipulate mutable fields which will cause those fields to be updated in the
> diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
> index ca4eb563b29b..2b01794cbd61 100644
> --- a/tools/testing/vma/vma_internal.h
> +++ b/tools/testing/vma/vma_internal.h
> @@ -21,7 +21,13 @@
>  
>  #include <stdlib.h>
>  
> +#ifdef __CONCAT
> +#undef __CONCAT
> +#endif
> +
> +#include <linux/args.h>
>  #include <linux/atomic.h>
> +#include <linux/bitmap.h>
>  #include <linux/list.h>
>  #include <linux/maple_tree.h>
>  #include <linux/mm.h>
> @@ -38,6 +44,8 @@ extern unsigned long dac_mmap_min_addr;
>  #define dac_mmap_min_addr	0UL
>  #endif
>  
> +#define ACCESS_PRIVATE(p, member) ((p)->member)
> +
>  #define VM_WARN_ON(_expr) (WARN_ON(_expr))
>  #define VM_WARN_ON_ONCE(_expr) (WARN_ON_ONCE(_expr))
>  #define VM_WARN_ON_VMG(_expr, _vmg) (WARN_ON(_expr))
> @@ -533,6 +541,8 @@ typedef struct {
>  	DECLARE_BITMAP(__vma_flags, NUM_VMA_FLAG_BITS);
>  } __private vma_flags_t;
>  
> +#define EMPTY_VMA_FLAGS ((vma_flags_t){ })
> +
>  struct mm_struct {
>  	struct maple_tree mm_mt;
>  	int map_count;			/* number of VMAs */
> @@ -882,6 +892,123 @@ static inline pgprot_t vm_get_page_prot(vm_flags_t vm_flags)
>  	return __pgprot(vm_flags);
>  }
>  
> +static inline void vma_flags_clear_all(vma_flags_t *flags)
> +{
> +	bitmap_zero(flags->__vma_flags, NUM_VMA_FLAG_BITS);
> +}
> +
> +static inline void vma_flag_set(vma_flags_t *flags, vma_flag_t bit)
> +{
> +	unsigned long *bitmap = flags->__vma_flags;
> +
> +	__set_bit((__force int)bit, bitmap);
> +}
> +
> +static inline vma_flags_t __mk_vma_flags(size_t count, const vma_flag_t *bits)
> +{
> +	vma_flags_t flags;
> +	int i;
> +
> +	vma_flags_clear_all(&flags);
> +	for (i = 0; i < count; i++)
> +		vma_flag_set(&flags, bits[i]);
> +	return flags;
> +}
> +
> +#define mk_vma_flags(...) __mk_vma_flags(COUNT_ARGS(__VA_ARGS__), \
> +					 (const vma_flag_t []){__VA_ARGS__})
> +
> +static __always_inline bool vma_flags_test_mask(const vma_flags_t *flags,
> +		vma_flags_t to_test)
> +{
> +	const unsigned long *bitmap = flags->__vma_flags;
> +	const unsigned long *bitmap_to_test = to_test.__vma_flags;
> +
> +	return bitmap_intersects(bitmap_to_test, bitmap, NUM_VMA_FLAG_BITS);
> +}
> +
> +#define vma_flags_test(flags, ...) \
> +	vma_flags_test_mask(flags, mk_vma_flags(__VA_ARGS__))
> +
> +static __always_inline bool vma_flags_test_all_mask(const vma_flags_t *flags,
> +		vma_flags_t to_test)
> +{
> +	const unsigned long *bitmap = flags->__vma_flags;
> +	const unsigned long *bitmap_to_test = to_test.__vma_flags;
> +
> +	return bitmap_subset(bitmap_to_test, bitmap, NUM_VMA_FLAG_BITS);
> +}
> +
> +#define vma_flags_test_all(flags, ...) \
> +	vma_flags_test_all_mask(flags, mk_vma_flags(__VA_ARGS__))
> +
> +static __always_inline void vma_flags_set_mask(vma_flags_t *flags, vma_flags_t to_set)
> +{
> +	unsigned long *bitmap = flags->__vma_flags;
> +	const unsigned long *bitmap_to_set = to_set.__vma_flags;
> +
> +	bitmap_or(bitmap, bitmap, bitmap_to_set, NUM_VMA_FLAG_BITS);
> +}
> +
> +#define vma_flags_set(flags, ...) \
> +	vma_flags_set_mask(flags, mk_vma_flags(__VA_ARGS__))
> +
> +static __always_inline void vma_flags_clear_mask(vma_flags_t *flags, vma_flags_t to_clear)
> +{
> +	unsigned long *bitmap = flags->__vma_flags;
> +	const unsigned long *bitmap_to_clear = to_clear.__vma_flags;
> +
> +	bitmap_andnot(bitmap, bitmap, bitmap_to_clear, NUM_VMA_FLAG_BITS);
> +}
> +
> +#define vma_flags_clear(flags, ...) \
> +	vma_flags_clear_mask(flags, mk_vma_flags(__VA_ARGS__))
> +
> +static inline bool vma_test_all_flags_mask(const struct vm_area_struct *vma,
> +					   vma_flags_t flags)
> +{
> +	return vma_flags_test_all_mask(&vma->flags, flags);
> +}
> +
> +#define vma_test_all_flags(vma, ...) \
> +	vma_test_all_flags_mask(vma, mk_vma_flags(__VA_ARGS__))
> +
> +static inline void vma_set_flags_mask(struct vm_area_struct *vma,
> +				      vma_flags_t flags)
> +{
> +	vma_flags_set_mask(&vma->flags, flags);
> +}
> +
> +#define vma_set_flags(vma, ...) \
> +	vma_set_flags_mask(vma, mk_vma_flags(__VA_ARGS__))
> +
> +static inline bool vma_desc_test_flags_mask(const struct vm_area_desc *desc,
> +					    vma_flags_t flags)
> +{
> +	return vma_flags_test_mask(&desc->vma_flags, flags);
> +}
> +
> +#define vma_desc_test_flags(desc, ...) \
> +	vma_desc_test_flags_mask(desc, mk_vma_flags(__VA_ARGS__))
> +
> +static inline void vma_desc_set_flags_mask(struct vm_area_desc *desc,
> +					   vma_flags_t flags)
> +{
> +	vma_flags_set_mask(&desc->vma_flags, flags);
> +}
> +
> +#define vma_desc_set_flags(desc, ...) \
> +	vma_desc_set_flags_mask(desc, mk_vma_flags(__VA_ARGS__))
> +
> +static inline void vma_desc_clear_flags_mask(struct vm_area_desc *desc,
> +					     vma_flags_t flags)
> +{
> +	vma_flags_clear_mask(&desc->vma_flags, flags);
> +}
> +
> +#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)
>  {
>  	return (vm_flags & (VM_SHARED | VM_MAYWRITE)) ==
> @@ -1540,31 +1667,11 @@ static inline void userfaultfd_unmap_complete(struct mm_struct *mm,
>  {
>  }
>  
> -#define ACCESS_PRIVATE(p, member) ((p)->member)
> -
> -#define bitmap_size(nbits)	(ALIGN(nbits, BITS_PER_LONG) / BITS_PER_BYTE)
> -
> -static __always_inline void bitmap_zero(unsigned long *dst, unsigned int nbits)
> -{
> -	unsigned int len = bitmap_size(nbits);
> -
> -	if (small_const_nbits(nbits))
> -		*dst = 0;
> -	else
> -		memset(dst, 0, len);
> -}
> -
>  static inline bool mm_flags_test(int flag, const struct mm_struct *mm)
>  {
>  	return test_bit(flag, ACCESS_PRIVATE(&mm->flags, __mm_flags));
>  }
>  
> -/* Clears all bits in the VMA flags bitmap, non-atomically. */
> -static inline void vma_flags_clear_all(vma_flags_t *flags)
> -{
> -	bitmap_zero(ACCESS_PRIVATE(flags, __vma_flags), NUM_VMA_FLAG_BITS);
> -}
> -
>  /*
>   * Copy value to the first system word of VMA flags, non-atomically.
>   *
> -- 
> 2.52.0
>