mm: vma flag tweaks
Subsystems (15)
Lorenzo Stoakes (Oracle)
maintainercover mm: vma flag tweaks
The ongoing work around introducing non-system word VMA flags has introduced a number of helper functions and macros to make life easier when working with these flags and to make conversions from the legacy use of VM_xxx flags more straightforward.
This series improves these to reduce confusion as to what they do and to improve consistency and readability.
Firstly the series renames vma_flags_test() to vma_flags_test_any() to make it abundantly clear that this function tests whether any of the flags are set (as opposed to vma_flags_test_all()).
It then renames vma_desc_test_flags() to vma_desc_test_any() for the same reason. Note that we drop the 'flags' suffix here, as vma_desc_test_any_flags() would be cumbersome and 'test' implies a flag test.
Similarly, we rename vma_test_all_flags() to vma_test_all() for consistency.
Next, we have a couple of instances (erofs, zonefs) where we are now testing for vma_desc_test_any(desc, VMA_SHARED_BIT) && vma_desc_test_any(desc, VMA_MAYWRITE_BIT).
This is silly, so this series introduces vma_desc_test_all() so these callers can instead invoke vma_desc_test_all(desc, VMA_SHARED_BIT, VMA_MAYWRITE_BIT).
We then observe that quite a few instances of vma_flags_test_any() and vma_desc_test_any() are in fact only testing against a single flag.
Using the _any() variant here is just confusing - 'any' of single item reads strangely and is liable to cause confusion.
So in these instances the series reintroduces vma_flags_test() and vma_desc_test() as helpers which test against a single flag.
The fact that vma_flags_t is a struct and that vma_flag_t utilises sparse to avoid confusion with vm_flags_t makes it impossible for a user to misuse these helpers without it getting flagged somewhere.
The series also updates __mk_vma_flags() and functions invoked by it to explicitly mark them always inline to match expectation and to be consistent with other VMA flag helpers.
It also renames vma_flag_set() to vma_flags_set_flag() (a function only used by __mk_vma_flags()) to be consistent with other VMA flag helpers.
Finally it updates the VMA tests for each of these changes, and introduces explicit tests for vma_flags_test() and vma_desc_test() to assert that they behave as expected.
Lorenzo Stoakes (Oracle) (6):
mm: rename VMA flag helpers to be more readable
mm: add vma_desc_test_all() and use it
mm: always inline __mk_vma_flags() and invoked functions
mm: reintroduce vma_flags_test() as a singular flag test
mm: reintroduce vma_desc_test() as a singular flag test tools/testing/vma: add test for vma_flags_test(), vma_desc_test()
drivers/char/mem.c | 2 +- drivers/dax/device.c | 2 +- fs/erofs/data.c | 3 +- fs/hugetlbfs/inode.c | 2 +- fs/ntfs3/file.c | 2 +- fs/resctrl/pseudo_lock.c | 2 +- fs/zonefs/file.c | 3 +- include/linux/dax.h | 4 +- include/linux/hugetlb_inline.h | 2 +- include/linux/mm.h | 100 +++++++++++++++++++++-------- include/linux/mm_types.h | 2 +- mm/hugetlb.c | 12 ++-- mm/memory.c | 2 +- mm/secretmem.c | 2 +- tools/testing/vma/include/custom.h | 5 +- tools/testing/vma/include/dup.h | 48 ++++++++++---- tools/testing/vma/tests/vma.c | 58 +++++++++++++---- 17 files changed, 177 insertions(+), 74 deletions(-)
--
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH 1/6 mm: rename VMA flag helpers to be more readable
On reflection, it's confusing to have vma_flags_test() and vma_desc_test_flags() test whether any comma-separated VMA flag bit is set, while also having vma_flags_test_all() and vma_test_all_flags() separately test whether all flags are set.
Firstly, rename vma_flags_test() to vma_flags_test_any() to eliminate this confusion.
Secondly, since the VMA descriptor flag functions are becoming rather cumbersome, prefer vma_desc_test*() to vma_desc_test_flags*(), and also rename vma_desc_test_flags() to vma_desc_test_any().
Finally, rename vma_test_all_flags() to vma_test_all() to keep the VMA-specific helper consistent with the VMA descriptor naming convention and to help avoid confusion vs. vma_flags_test_all().
While we're here, also update whitespace to be consistent in helper functions.
--- drivers/char/mem.c | 2 +- drivers/dax/device.c | 2 +- fs/erofs/data.c | 4 +-- fs/hugetlbfs/inode.c | 2 +- fs/ntfs3/file.c | 2 +- fs/resctrl/pseudo_lock.c | 2 +- fs/zonefs/file.c | 4 +-- include/linux/dax.h | 4 +-- include/linux/hugetlb_inline.h | 2 +- include/linux/mm.h | 48 +++++++++++++++++---------------- mm/hugetlb.c | 14 +++++----- mm/memory.c | 2 +- mm/secretmem.c | 2 +- mm/shmem.c | 4 +-- tools/testing/vma/include/dup.h | 20 +++++++------- tools/testing/vma/tests/vma.c | 28 +++++++++---------- 16 files changed, 72 insertions(+), 70 deletions(-)
drivers/char/mem.c +1 -1
@@ -520,7 +520,7 @@ static int mmap_zero_prepare(struct vm_area_desc *desc)#ifndef CONFIG_MMU return -ENOSYS ;#endif -if (vma_desc_test_flags (desc,VMA_SHARED_BIT )) +if (vma_desc_test_any (desc,VMA_SHARED_BIT ))return shmem_zero_setup_desc (desc); desc->action.success_hook = mmap_zero_private_success;
drivers/dax/device.c +1 -1
@@ -24,7 +24,7 @@ static int __check_vma(struct dev_dax *dev_dax, vma_flags_t flags,return -ENXIO ;/* prevent private mappings from being established */ -if (!vma_flags_test (&flags,VMA_MAYSHARE_BIT )) { +if (!vma_flags_test_any (&flags,VMA_MAYSHARE_BIT )) {dev_info_ratelimited (dev,"%s: %s: fail, attempted private mapping\n" , current->comm, func);
fs/erofs/data.c +2 -2
@@ -473,8 +473,8 @@ static int erofs_file_mmap_prepare(struct vm_area_desc *desc)if (!IS_DAX (file_inode (desc->file)))return generic_file_readonly_mmap_prepare (desc); -if (vma_desc_test_flags (desc,VMA_SHARED_BIT ) && -vma_desc_test_flags (desc,VMA_MAYWRITE_BIT )) +if (vma_desc_test_any (desc,VMA_SHARED_BIT ) && +vma_desc_test_any (desc,VMA_MAYWRITE_BIT ))return -EINVAL ; desc->vm_ops = &erofs_dax_vm_ops;
fs/hugetlbfs/inode.c +1 -1
@@ -164,7 +164,7 @@ static int hugetlbfs_file_mmap_prepare(struct vm_area_desc *desc) goto out; ret =0 ; -if (vma_desc_test_flags (desc,VMA_WRITE_BIT ) && inode->i_size < len) +if (vma_desc_test_any (desc,VMA_WRITE_BIT ) && inode->i_size < len)i_size_write (inode, len); out:inode_unlock (inode);
fs/ntfs3/file.c +1 -1
@@ -276,7 +276,7 @@ static int ntfs_file_mmap_prepare(struct vm_area_desc *desc)struct file *file = desc->file;struct inode *inode =file_inode (file);struct ntfs_inode *ni =ntfs_i (inode); -const bool rw =vma_desc_test_flags (desc,VMA_WRITE_BIT ); +const bool rw =vma_desc_test_any (desc,VMA_WRITE_BIT );int err;/* Avoid any operation if inode is bad. */
fs/resctrl/pseudo_lock.c +1 -1
@@ -1044,7 +1044,7 @@ static int pseudo_lock_dev_mmap_prepare(struct vm_area_desc *desc)* Ensure changes are carried directly to the memory being mapped, * do not allow copy-on-write mapping. */ -if (!vma_desc_test_flags (desc,VMA_SHARED_BIT )) { +if (!vma_desc_test_any (desc,VMA_SHARED_BIT )) {mutex_unlock (&rdtgroup_mutex);return -EINVAL ; }
fs/zonefs/file.c +2 -2
@@ -333,8 +333,8 @@ static int zonefs_file_mmap_prepare(struct vm_area_desc *desc)* ordering between msync() and page cache writeback. */ if (zonefs_inode_is_seq (file_inode (file)) && -vma_desc_test_flags (desc,VMA_SHARED_BIT ) && -vma_desc_test_flags (desc,VMA_MAYWRITE_BIT )) +vma_desc_test_any (desc,VMA_SHARED_BIT ) && +vma_desc_test_any (desc,VMA_MAYWRITE_BIT ))return -EINVAL ;file_accessed (file);
include/linux/dax.h +2 -2
@@ -69,7 +69,7 @@ static inline bool daxdev_mapping_supported(const struct vm_area_desc *desc, const struct inode *inode, struct dax_device *dax_dev) { - if (!vma_desc_test_flags(desc, VMA_SYNC_BIT)) + if (!vma_desc_test_any(desc, VMA_SYNC_BIT)) return true;if (!IS_DAX (inode))return false;
@@ -115,7 +115,7 @@ static inline bool daxdev_mapping_supported(const struct vm_area_desc *desc,const struct inode *inode, struct dax_device *dax_dev) { - return !vma_desc_test_flags(desc, VMA_SYNC_BIT); + return !vma_desc_test_any(desc, VMA_SYNC_BIT); }static inline size_t dax_recovery_write (struct dax_device *dax_dev,pgoff_t pgoff,void *addr,size_t bytes,struct iov_iter *i)
include/linux/hugetlb_inline.h +1 -1
@@ -13,7 +13,7 @@ static inline bool is_vm_hugetlb_flags(vm_flags_t vm_flags)static inline bool is_vma_hugetlb_flags (const vma_flags_t *flags) { -return vma_flags_test (flags,VMA_HUGETLB_BIT ); +return vma_flags_test_any (flags,VMA_HUGETLB_BIT ); }#else
include/linux/mm.h +25 -23
128 lines not on this page · raw message
mm/hugetlb.c +7 -7
@@ -1194,7 +1194,7 @@ static void set_vma_resv_flags(struct vm_area_struct *vma, unsigned long flags)static void set_vma_desc_resv_map (struct vm_area_desc *desc,struct resv_map *map) {VM_WARN_ON_ONCE (!is_vma_hugetlb_flags (&desc->vma_flags)); -VM_WARN_ON_ONCE (vma_desc_test_flags (desc,VMA_MAYSHARE_BIT )); +VM_WARN_ON_ONCE (vma_desc_test_any (desc,VMA_MAYSHARE_BIT )); desc->private_data = map; }
@@ -1202,7 +1202,7 @@ static void set_vma_desc_resv_map(struct vm_area_desc *desc, struct resv_map *mastatic void set_vma_desc_resv_flags (struct vm_area_desc *desc,unsigned long flags) {VM_WARN_ON_ONCE (!is_vma_hugetlb_flags (&desc->vma_flags)); -VM_WARN_ON_ONCE (vma_desc_test_flags (desc,VMA_MAYSHARE_BIT )); +VM_WARN_ON_ONCE (vma_desc_test_any (desc,VMA_MAYSHARE_BIT )); desc->private_data = (void *)((unsigned long )desc->private_data | flags); }
@@ -6591,7 +6591,7 @@ long hugetlb_reserve_pages(struct inode *inode,* attempt will be made for VM_NORESERVE to allocate a page * without using reserves */ -if (vma_flags_test (&vma_flags,VMA_NORESERVE_BIT )) +if (vma_flags_test_any (&vma_flags,VMA_NORESERVE_BIT ))return 0 ;/*
@@ -6600,7 +6600,7 @@ long hugetlb_reserve_pages(struct inode *inode,* to reserve the full area even if read-only as mprotect() may be * called to make the mapping read-write. Assume !desc is a shm mapping */ -if (!desc ||vma_desc_test_flags (desc,VMA_MAYSHARE_BIT )) { +if (!desc ||vma_desc_test_any (desc,VMA_MAYSHARE_BIT )) {/* * resv_map can not be NULL as hugetlb_reserve_pages is only * called for inodes for which resv_maps were created (see
@@ -6634,7 +6634,7 @@ long hugetlb_reserve_pages(struct inode *inode,if (err <0 ) goto out_err; -if (desc && !vma_desc_test_flags (desc,VMA_MAYSHARE_BIT ) && h_cg) { +if (desc && !vma_desc_test_any (desc,VMA_MAYSHARE_BIT ) && h_cg) {/* For private mappings, the hugetlb_cgroup uncharge info hangs * of the resv_map. */
@@ -6671,7 +6671,7 @@ long hugetlb_reserve_pages(struct inode *inode,* consumed reservations are stored in the map. Hence, nothing * else has to be done for private mappings here */ -if (!desc ||vma_desc_test_flags (desc,VMA_MAYSHARE_BIT )) { +if (!desc ||vma_desc_test_any (desc,VMA_MAYSHARE_BIT )) { add =region_add (resv_map, from, to, regions_needed, h, h_cg);if (unlikely (add <0 )) {
@@ -6735,7 +6735,7 @@ long hugetlb_reserve_pages(struct inode *inode,hugetlb_cgroup_uncharge_cgroup_rsvd (hstate_index (h), chg *pages_per_huge_page (h), h_cg); out_err: -if (!desc ||vma_desc_test_flags (desc,VMA_MAYSHARE_BIT )) +if (!desc ||vma_desc_test_any (desc,VMA_MAYSHARE_BIT ))/* Only call region_abort if the region_chg succeeded but the * region_add failed or didn't run. */
mm/memory.c +1 -1
@@ -2969,7 +2969,7 @@ static int remap_pfn_range_internal(struct vm_area_struct *vma, unsigned long adif (WARN_ON_ONCE (!PAGE_ALIGNED (addr)))return -EINVAL ; -VM_WARN_ON_ONCE (!vma_test_all_flags_mask (vma,VMA_REMAP_FLAGS )); +VM_WARN_ON_ONCE (!vma_test_all_mask (vma,VMA_REMAP_FLAGS ));BUG_ON (addr >= end); pfn -= addr >>PAGE_SHIFT ;
mm/secretmem.c +1 -1
@@ -122,7 +122,7 @@ static int secretmem_mmap_prepare(struct vm_area_desc *desc) {const unsigned long len =vma_desc_size (desc); -if (!vma_desc_test_flags (desc,VMA_SHARED_BIT ,VMA_MAYSHARE_BIT )) +if (!vma_desc_test_any (desc,VMA_SHARED_BIT ,VMA_MAYSHARE_BIT ))return -EINVAL ;vma_desc_set_flags (desc,VMA_LOCKED_BIT ,VMA_DONTDUMP_BIT );
mm/shmem.c +2 -2
@@ -3086,7 +3086,7 @@ static struct inode *__shmem_get_inode(struct mnt_idmap *idmap,spin_lock_init (&info->lock);atomic_set (&info->stop_eviction,0 ); info->seals =F_SEAL_SEAL ; - info->flags =vma_flags_test (&flags,VMA_NORESERVE_BIT ) + info->flags =vma_flags_test_any (&flags,VMA_NORESERVE_BIT ) ?SHMEM_F_NORESERVE :0 ; info->i_crtime =inode_get_mtime (inode); info->fsflags = (dir == NULL) ? 0 :
@@ -5827,7 +5827,7 @@ static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, unsigned int i_flags) {const unsigned long shmem_flags = -vma_flags_test (&flags,VMA_NORESERVE_BIT ) ?SHMEM_F_NORESERVE :0 ; +vma_flags_test_any (&flags,VMA_NORESERVE_BIT ) ?SHMEM_F_NORESERVE :0 ;struct inode *inode;struct file *res;
tools/testing/vma/include/dup.h +10 -10
@@ -843,7 +843,7 @@ 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__}) -static __always_inline bool vma_flags_test_mask(const vma_flags_t *flags, +static __always_inline bool vma_flags_test_any_mask(const vma_flags_t *flags,vma_flags_t to_test) {const unsigned long *bitmap = flags->__vma_flags;
@@ -852,8 +852,8 @@ static __always_inline bool vma_flags_test_mask(const vma_flags_t *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__)) +#define vma_flags_test_any (flags, ...) \ + vma_flags_test_any_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)
@@ -889,14 +889,14 @@ static __always_inline void vma_flags_clear_mask(vma_flags_t *flags, vma_flags_t#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, +static inline bool vma_test_all_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__)) +#define vma_test_all (vma, ...) \ + vma_test_all_mask(vma, mk_vma_flags(__VA_ARGS__))static inline bool is_shared_maywrite_vm_flags (vm_flags_t vm_flags) {
@@ -913,14 +913,14 @@ static inline void vma_set_flags_mask(struct vm_area_struct *vma,#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, +static inline bool vma_desc_test_any_mask (const struct vm_area_desc *desc,vma_flags_t flags) { -return vma_flags_test_mask (&desc->vma_flags, flags); +return vma_flags_test_any_mask (&desc->vma_flags, flags); } -#define vma_desc_test_flags (desc, ...) \ - vma_desc_test_flags_mask(desc, mk_vma_flags(__VA_ARGS__)) +#define vma_desc_test_any (desc, ...) \ + vma_desc_test_any_mask(desc, mk_vma_flags(__VA_ARGS__))static inline void vma_desc_set_flags_mask (struct vm_area_desc *desc,vma_flags_t flags)
tools/testing/vma/tests/vma.c +14 -14
@@ -159,8 +159,8 @@ static bool test_vma_flags_word(void)return true; } -/* Ensure that vma_flags_test() and friends works correctly. */ -static bool test_vma_flags_test (void ) +/* Ensure that vma_flags_test_any() and friends works correctly. */ +static bool test_vma_flags_test_any (void ) {const vma_flags_t flags =mk_vma_flags (VMA_READ_BIT ,VMA_WRITE_BIT ,VMA_EXEC_BIT ,64 ,65 );
@@ -171,16 +171,16 @@ static bool test_vma_flags_test(void) desc.vma_flags = flags;#define do_test (...) \ - ASSERT_TRUE(vma_flags_test(&flags, __VA_ARGS__)); \ - ASSERT_TRUE(vma_desc_test_flags(&desc, __VA_ARGS__)) + ASSERT_TRUE(vma_flags_test_any(&flags, __VA_ARGS__)); \ + ASSERT_TRUE(vma_desc_test_any(&desc, __VA_ARGS__))#define do_test_all_true (...) \ ASSERT_TRUE(vma_flags_test_all(&flags, __VA_ARGS__)); \ - ASSERT_TRUE(vma_test_all_flags(&vma, __VA_ARGS__)) + ASSERT_TRUE(vma_test_all(&vma, __VA_ARGS__))#define do_test_all_false (...) \ ASSERT_FALSE(vma_flags_test_all(&flags, __VA_ARGS__)); \ - ASSERT_FALSE(vma_test_all_flags(&vma, __VA_ARGS__)) + ASSERT_FALSE(vma_test_all(&vma, __VA_ARGS__))/* * Testing for some flags that are present, some that are not - should
@@ -200,7 +200,7 @@ static bool test_vma_flags_test(void)* Check _mask variant. We don't need to test extensively as macro * helper is the equivalent. */ -ASSERT_TRUE (vma_flags_test_mask (&flags, flags)); +ASSERT_TRUE (vma_flags_test_any_mask (&flags, flags));ASSERT_TRUE (vma_flags_test_all_mask (&flags, flags));/* Single bits. */
@@ -268,9 +268,9 @@ static bool test_vma_flags_clear(void)vma_flags_clear_mask (&flags, mask);vma_flags_clear_mask (&vma.flags, mask);vma_desc_clear_flags_mask (&desc, mask); -ASSERT_FALSE (vma_flags_test (&flags,VMA_EXEC_BIT ,64 )); -ASSERT_FALSE (vma_flags_test (&vma.flags,VMA_EXEC_BIT ,64 )); -ASSERT_FALSE (vma_desc_test_flags (&desc,VMA_EXEC_BIT ,64 )); +ASSERT_FALSE (vma_flags_test_any (&flags,VMA_EXEC_BIT ,64 )); +ASSERT_FALSE (vma_flags_test_any (&vma.flags,VMA_EXEC_BIT ,64 )); +ASSERT_FALSE (vma_desc_test_any (&desc,VMA_EXEC_BIT ,64 ));/* Reset. */ vma_flags_set (&flags,VMA_EXEC_BIT ,64 );vma_set_flags (&vma,VMA_EXEC_BIT ,64 );
@@ -284,9 +284,9 @@ static bool test_vma_flags_clear(void)vma_flags_clear (&flags, __VA_ARGS__); \vma_flags_clear (&vma.flags, __VA_ARGS__); \vma_desc_clear_flags (&desc, __VA_ARGS__); \ -ASSERT_FALSE (vma_flags_test (&flags, __VA_ARGS__)); \ -ASSERT_FALSE (vma_flags_test (&vma.flags, __VA_ARGS__)); \ -ASSERT_FALSE (vma_desc_test_flags (&desc, __VA_ARGS__)); \ +ASSERT_FALSE (vma_flags_test_any (&flags, __VA_ARGS__)); \ +ASSERT_FALSE (vma_flags_test_any (&vma.flags, __VA_ARGS__)); \ +ASSERT_FALSE (vma_desc_test_any (&desc, __VA_ARGS__)); \vma_flags_set (&flags, __VA_ARGS__); \vma_set_flags (&vma, __VA_ARGS__); \vma_desc_set_flags (&desc, __VA_ARGS__)
@@ -334,6 +334,6 @@ static void run_vma_tests(int *num_tests, int *num_fail)TEST (vma_flags_unchanged);TEST (vma_flags_cleared);TEST (vma_flags_word); -TEST (vma_flags_test); +TEST (vma_flags_test_any);TEST (vma_flags_clear); }
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH 2/6 mm: add vma_desc_test_all() and use it
In reply to Lorenzo Stoakes (Oracle)erofs and zonefs are using vma_desc_test_any() twice to check whether all of VMA_SHARED_BIT and VMA_MAYWRITE_BIT are set, this is silly, so add vma_desc_test_all() to test all flags and update erofs and zonefs to use it.
While we're here, update the helper function comments to be more consistent.
Also add the same to the VMA test headers.
--- fs/erofs/data.c | 3 +-- fs/zonefs/file.c | 3 +-- include/linux/mm.h | 24 ++++++++++++++++++++---- tools/testing/vma/include/dup.h | 9 +++++++++ 4 files changed, 31 insertions(+), 8 deletions(-)
fs/erofs/data.c +1 -2
@@ -473,8 +473,7 @@ static int erofs_file_mmap_prepare(struct vm_area_desc *desc)if (!IS_DAX (file_inode (desc->file)))return generic_file_readonly_mmap_prepare (desc); -if (vma_desc_test_any (desc,VMA_SHARED_BIT ) && -vma_desc_test_any (desc,VMA_MAYWRITE_BIT )) +if (vma_desc_test_all (desc,VMA_SHARED_BIT ,VMA_MAYWRITE_BIT ))return -EINVAL ; desc->vm_ops = &erofs_dax_vm_ops;
fs/zonefs/file.c +1 -2
@@ -333,8 +333,7 @@ static int zonefs_file_mmap_prepare(struct vm_area_desc *desc)* ordering between msync() and page cache writeback. */ if (zonefs_inode_is_seq (file_inode (file)) && -vma_desc_test_any (desc,VMA_SHARED_BIT ) && -vma_desc_test_any (desc,VMA_MAYWRITE_BIT )) +vma_desc_test_all (desc,VMA_SHARED_BIT ,VMA_MAYWRITE_BIT ))return -EINVAL ;file_accessed (file);
include/linux/mm.h +20 -4
@@ -1178,7 +1178,7 @@ static inline void vma_set_flags_mask(struct vm_area_struct *vma,#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. */ +/* Helper to test any VMA flags in a VMA descriptor. */ static inline bool vma_desc_test_any_mask (const struct vm_area_desc *desc,vma_flags_t flags) {
@@ -1186,8 +1186,8 @@ static inline bool vma_desc_test_any_mask(const struct vm_area_desc *desc, }/* -* Helper macro for testing VMA flags for an input pointer to a struct -* vm_area_desc object describing a proposed VMA, e.g.: +* Helper macro for testing whether any VMA flags are set in a VMA descriptor, +* e.g.: * * if (vma_desc_test_any(desc, VMA_IO_BIT, VMA_PFNMAP_BIT, * VMA_DONTEXPAND_BIT, VMA_DONTDUMP_BIT)) { ... }
@@ -1195,6 +1195,22 @@ static inline bool vma_desc_test_any_mask(const struct vm_area_desc *desc,#define vma_desc_test_any (desc, ...) \ vma_desc_test_any_mask(desc, mk_vma_flags(__VA_ARGS__)) +/* Helper to test all VMA flags in a VMA descriptor. */ +static inline bool vma_desc_test_all_mask (const struct vm_area_desc *desc, +vma_flags_t flags) +{ +return vma_flags_test_all_mask (&desc->vma_flags, flags); +} + +/* +* Helper macro for testing whether ALL VMA flags are set in a VMA descriptor, +* e.g.: +* +* if (vma_desc_test_all(desc, VMA_READ_BIT, VMA_MAYREAD_BIT)) { ... } +*/ +#define vma_desc_test_all (desc, ...) \ + vma_desc_test_all_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)
@@ -1207,7 +1223,7 @@ static inline void vma_desc_set_flags_mask(struct vm_area_desc *desc,* 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); +* VMA_DONTDUMP_BIT); */ #define vma_desc_set_flags (desc, ...) \ vma_desc_set_flags_mask(desc, mk_vma_flags(__VA_ARGS__))
tools/testing/vma/include/dup.h +9 -0
@@ -922,6 +922,15 @@ static inline bool vma_desc_test_any_mask(const struct vm_area_desc *desc,#define vma_desc_test_any (desc, ...) \ vma_desc_test_any_mask(desc, mk_vma_flags(__VA_ARGS__)) +static inline bool vma_desc_test_all_mask (const struct vm_area_desc *desc, +vma_flags_t flags) +{ +return vma_flags_test_all_mask (&desc->vma_flags, flags); +} + +#define vma_desc_test_all (desc, ...) \ + vma_desc_test_all_mask(desc, mk_vma_flags(__VA_ARGS__)) +static inline void vma_desc_set_flags_mask (struct vm_area_desc *desc,vma_flags_t flags) {
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH 3/6 mm: always inline __mk_vma_flags() and invoked functions
In reply to Lorenzo Stoakes (Oracle)Be explicit about __mk_vma_flags() (which is used by the mk_vma_flags() macro) always being inline, as we rely on the compiler converting this function into meaningful.
Also update all of the functions __mk_vma_flags() ultimately invokes to be always inline too.
Note that test_bitmap_const_eval() asserts that the relevant bitmap functions result in build time constant values.
Additionally, vma_flag_set() operates on a vma_flags_t type, so it is inconsistently named versus other VMA flags functions.
We only use vma_flag_set() in __mk_vma_flags() so we don't need to worry about its new name being rather cumbersome, so rename it to vma_flags_set_flag() to disambiguate it from vma_flags_set().
Also update the VMA test headers to reflect the changes.
--- include/linux/mm.h | 8 +++++--- include/linux/mm_types.h | 2 +- tools/testing/vma/include/custom.h | 5 +++-- tools/testing/vma/include/dup.h | 5 +++-- 4 files changed, 12 insertions(+), 8 deletions(-)
include/linux/mm.h +5 -3
@@ -1031,21 +1031,23 @@ static inline bool vma_test_atomic_flag(struct vm_area_struct *vma, vma_flag_t b }/* Set an individual VMA flag in flags, non-atomically. */ -static inline void vma_flag_set (vma_flags_t *flags,vma_flag_t bit) +static __always_inline void vma_flags_set_flag(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) +static __always_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]); +vma_flags_set_flag (&flags, bits[i]);return flags; }
include/linux/mm_types.h +1 -1
@@ -1056,7 +1056,7 @@ struct vm_area_struct { } __randomize_layout;/* Clears all bits in the VMA flags bitmap, non-atomically. */ -static inline void vma_flags_clear_all (vma_flags_t *flags) +static __always_inline void vma_flags_clear_all(vma_flags_t *flags) {bitmap_zero (flags->__vma_flags,NUM_VMA_FLAG_BITS ); }
tools/testing/vma/include/custom.h +3 -2
@@ -102,7 +102,8 @@ static inline void vma_lock_init(struct vm_area_struct *vma, bool reset_refcnt)refcount_set (&vma->vm_refcnt,0 ); } -static inline vma_flags_t __mk_vma_flags (size_t count,const vma_flag_t *bits) +static __always_inline vma_flags_t __mk_vma_flags(size_t count, +const vma_flag_t *bits) {vma_flags_t flags;int i;
@@ -114,6 +115,6 @@ static inline vma_flags_t __mk_vma_flags(size_t count, const vma_flag_t *bits)vma_flags_clear_all (&flags);for (i =0 ; i < count; i++)if (bits[i] <NUM_VMA_FLAG_BITS ) -vma_flag_set (&flags, bits[i]); +vma_flags_set_flag (&flags, bits[i]);return flags; }
tools/testing/vma/include/dup.h +3 -2
@@ -780,12 +780,13 @@ static inline void vma_flags_clear_word(vma_flags_t *flags, unsigned long value) *bitmap &= ~value; } -static inline void vma_flags_clear_all (vma_flags_t *flags) +static __always_inline void vma_flags_clear_all(vma_flags_t *flags) {bitmap_zero (ACCESS_PRIVATE (flags, __vma_flags),NUM_VMA_FLAG_BITS ); } -static inline void vma_flag_set (vma_flags_t *flags,vma_flag_t bit) +static __always_inline void vma_flags_set_flag(vma_flags_t *flags, +vma_flag_t bit) {unsigned long *bitmap =ACCESS_PRIVATE (flags, __vma_flags);
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH 4/6 mm: reintroduce vma_flags_test() as a singular flag test
In reply to Lorenzo Stoakes (Oracle)Since we've now renamed vma_flags_test() to vma_flags_test_any() to be very clear as to what we are in fact testing, we now have the opportunity to bring vma_flags_test() back, but for explicitly testing a single VMA flag.
This is useful, as often flag tests are against a single flag, and vma_flags_test_any(flags, VMA_READ_BIT) reads oddly and potentially causes confusion.
We use sparse to enforce that users won't accidentally pass vm_flags_t to this function without it being flagged so this should make it harder to get this wrong.
Of course, passing vma_flags_t to the function is impossible, as it is a struct.
Also update the VMA tests to reflect this change.
--- include/linux/mm.h | 17 +++++++++++++++-- mm/hugetlb.c | 2 +- mm/shmem.c | 4 ++-- tools/testing/vma/include/dup.h | 8 ++++++++ 4 files changed, 26 insertions(+), 5 deletions(-)
include/linux/mm.h +15 -2
@@ -1051,6 +1051,19 @@ static __always_inline vma_flags_t __mk_vma_flags(size_t count,return flags; } +/* +* Test whether a specific VMA flag is set, e.g.: +* +* if (vma_flags_test(flags, VMA_READ_BIT)) { ... } +*/ +static __always_inline bool vma_flags_test(const vma_flags_t *flags, +vma_flag_t bit) +{ +const unsigned long *bitmap = flags->__vma_flags; + +return test_bit ((__force int )bit, bitmap); +} +/* * Helper macro which bitwise-or combines the specified input flags into a * vma_flags_t bitmap value. E.g.:
@@ -1957,8 +1970,8 @@ static inline bool vma_desc_is_cow_mapping(struct vm_area_desc *desc) {const vma_flags_t *flags = &desc->vma_flags; -return vma_flags_test_any (flags,VMA_MAYWRITE_BIT ) && - !vma_flags_test_any (flags,VMA_SHARED_BIT ); +return vma_flags_test (flags,VMA_MAYWRITE_BIT ) && + !vma_flags_test (flags,VMA_SHARED_BIT ); }#ifndef CONFIG_MMU
mm/hugetlb.c +1 -1
@@ -6591,7 +6591,7 @@ long hugetlb_reserve_pages(struct inode *inode,* attempt will be made for VM_NORESERVE to allocate a page * without using reserves */ -if (vma_flags_test_any (&vma_flags,VMA_NORESERVE_BIT )) +if (vma_flags_test (&vma_flags,VMA_NORESERVE_BIT ))return 0 ;/*
mm/shmem.c +2 -2
@@ -3086,7 +3086,7 @@ static struct inode *__shmem_get_inode(struct mnt_idmap *idmap,spin_lock_init (&info->lock);atomic_set (&info->stop_eviction,0 ); info->seals =F_SEAL_SEAL ; - info->flags =vma_flags_test_any (&flags,VMA_NORESERVE_BIT ) + info->flags =vma_flags_test (&flags,VMA_NORESERVE_BIT ) ?SHMEM_F_NORESERVE :0 ; info->i_crtime =inode_get_mtime (inode); info->fsflags = (dir == NULL) ? 0 :
@@ -5827,7 +5827,7 @@ static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, unsigned int i_flags) {const unsigned long shmem_flags = -vma_flags_test_any (&flags,VMA_NORESERVE_BIT ) ?SHMEM_F_NORESERVE :0 ; +vma_flags_test (&flags,VMA_NORESERVE_BIT ) ?SHMEM_F_NORESERVE :0 ;struct inode *inode;struct file *res;
tools/testing/vma/include/dup.h +8 -0
@@ -844,6 +844,14 @@ 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__}) +static __always_inline bool vma_flags_test(const vma_flags_t *flags, +vma_flag_t bit) +{ +const unsigned long *bitmap = flags->__vma_flags; + +return test_bit ((__force int )bit, bitmap); +} + static __always_inline bool vma_flags_test_any_mask(const vma_flags_t *flags,vma_flags_t to_test) {
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH 5/6 mm: reintroduce vma_desc_test() as a singular flag test
In reply to Lorenzo Stoakes (Oracle)Similar to vma_flags_test(), we have previously renamed vma_desc_test() to vma_desc_test_any(). Now that is in place, we can reintroduce vma_desc_test() to explicitly check for a single VMA flag.
As with vma_flags_test(), this is useful as often flag tests are against a single flag, and vma_desc_test_any(flags, VMA_READ_BIT) reads oddly and potentially causes confusion.
As with vma_flags_test() a combination of sparse and vma_flags_t being a struct means that users cannot misuse this function without it getting flagged.
Also update the VMA tests to reflect this change.
--- drivers/char/mem.c | 2 +- fs/hugetlbfs/inode.c | 2 +- fs/ntfs3/file.c | 2 +- fs/resctrl/pseudo_lock.c | 2 +- include/linux/dax.h | 4 ++-- include/linux/mm.h | 11 +++++++++++ mm/hugetlb.c | 12 ++++++------ tools/testing/vma/include/dup.h | 6 ++++++ 8 files changed, 29 insertions(+), 12 deletions(-)
drivers/char/mem.c +1 -1
@@ -520,7 +520,7 @@ static int mmap_zero_prepare(struct vm_area_desc *desc)#ifndef CONFIG_MMU return -ENOSYS ;#endif -if (vma_desc_test_any (desc,VMA_SHARED_BIT )) +if (vma_desc_test (desc,VMA_SHARED_BIT ))return shmem_zero_setup_desc (desc); desc->action.success_hook = mmap_zero_private_success;
fs/hugetlbfs/inode.c +1 -1
@@ -164,7 +164,7 @@ static int hugetlbfs_file_mmap_prepare(struct vm_area_desc *desc) goto out; ret =0 ; -if (vma_desc_test_any (desc,VMA_WRITE_BIT ) && inode->i_size < len) +if (vma_desc_test (desc,VMA_WRITE_BIT ) && inode->i_size < len)i_size_write (inode, len); out:inode_unlock (inode);
fs/ntfs3/file.c +1 -1
@@ -276,7 +276,7 @@ static int ntfs_file_mmap_prepare(struct vm_area_desc *desc)struct file *file = desc->file;struct inode *inode =file_inode (file);struct ntfs_inode *ni =ntfs_i (inode); -const bool rw =vma_desc_test_any (desc,VMA_WRITE_BIT ); +const bool rw =vma_desc_test (desc,VMA_WRITE_BIT );int err;/* Avoid any operation if inode is bad. */
fs/resctrl/pseudo_lock.c +1 -1
@@ -1044,7 +1044,7 @@ static int pseudo_lock_dev_mmap_prepare(struct vm_area_desc *desc)* Ensure changes are carried directly to the memory being mapped, * do not allow copy-on-write mapping. */ -if (!vma_desc_test_any (desc,VMA_SHARED_BIT )) { +if (!vma_desc_test (desc,VMA_SHARED_BIT )) {mutex_unlock (&rdtgroup_mutex);return -EINVAL ; }
include/linux/dax.h +2 -2
@@ -69,7 +69,7 @@ static inline bool daxdev_mapping_supported(const struct vm_area_desc *desc, const struct inode *inode, struct dax_device *dax_dev) { - if (!vma_desc_test_any(desc, VMA_SYNC_BIT)) + if (!vma_desc_test(desc, VMA_SYNC_BIT)) return true;if (!IS_DAX (inode))return false;
@@ -115,7 +115,7 @@ static inline bool daxdev_mapping_supported(const struct vm_area_desc *desc,const struct inode *inode, struct dax_device *dax_dev) { - return !vma_desc_test_any(desc, VMA_SYNC_BIT); + return !vma_desc_test(desc, VMA_SYNC_BIT); }static inline size_t dax_recovery_write (struct dax_device *dax_dev,pgoff_t pgoff,void *addr,size_t bytes,struct iov_iter *i)
include/linux/mm.h +11 -0
@@ -1193,6 +1193,17 @@ static inline void vma_set_flags_mask(struct vm_area_struct *vma,#define vma_set_flags (vma, ...) \ vma_set_flags_mask(vma, mk_vma_flags(__VA_ARGS__)) +/* +* Test whether a specific VMA flag is set in a VMA descriptor, e.g.: +* +* if (vma_desc_test(desc, VMA_READ_BIT)) { ... } +*/ +static __always_inline bool vma_desc_test(const struct vm_area_desc *desc, +vma_flag_t bit) +{ +return vma_flags_test (&desc->vma_flags, bit); +} +/* Helper to test any VMA flags in a VMA descriptor. */ static inline bool vma_desc_test_any_mask (const struct vm_area_desc *desc,vma_flags_t flags)
mm/hugetlb.c +6 -6
@@ -1194,7 +1194,7 @@ static void set_vma_resv_flags(struct vm_area_struct *vma, unsigned long flags)static void set_vma_desc_resv_map (struct vm_area_desc *desc,struct resv_map *map) {VM_WARN_ON_ONCE (!is_vma_hugetlb_flags (&desc->vma_flags)); -VM_WARN_ON_ONCE (vma_desc_test_any (desc,VMA_MAYSHARE_BIT )); +VM_WARN_ON_ONCE (vma_desc_test (desc,VMA_MAYSHARE_BIT )); desc->private_data = map; }
@@ -1202,7 +1202,7 @@ static void set_vma_desc_resv_map(struct vm_area_desc *desc, struct resv_map *mastatic void set_vma_desc_resv_flags (struct vm_area_desc *desc,unsigned long flags) {VM_WARN_ON_ONCE (!is_vma_hugetlb_flags (&desc->vma_flags)); -VM_WARN_ON_ONCE (vma_desc_test_any (desc,VMA_MAYSHARE_BIT )); +VM_WARN_ON_ONCE (vma_desc_test (desc,VMA_MAYSHARE_BIT )); desc->private_data = (void *)((unsigned long )desc->private_data | flags); }
@@ -6600,7 +6600,7 @@ long hugetlb_reserve_pages(struct inode *inode,* to reserve the full area even if read-only as mprotect() may be * called to make the mapping read-write. Assume !desc is a shm mapping */ -if (!desc ||vma_desc_test_any (desc,VMA_MAYSHARE_BIT )) { +if (!desc ||vma_desc_test (desc,VMA_MAYSHARE_BIT )) {/* * resv_map can not be NULL as hugetlb_reserve_pages is only * called for inodes for which resv_maps were created (see
@@ -6634,7 +6634,7 @@ long hugetlb_reserve_pages(struct inode *inode,if (err <0 ) goto out_err; -if (desc && !vma_desc_test_any (desc,VMA_MAYSHARE_BIT ) && h_cg) { +if (desc && !vma_desc_test (desc,VMA_MAYSHARE_BIT ) && h_cg) {/* For private mappings, the hugetlb_cgroup uncharge info hangs * of the resv_map. */
@@ -6671,7 +6671,7 @@ long hugetlb_reserve_pages(struct inode *inode,* consumed reservations are stored in the map. Hence, nothing * else has to be done for private mappings here */ -if (!desc ||vma_desc_test_any (desc,VMA_MAYSHARE_BIT )) { +if (!desc ||vma_desc_test (desc,VMA_MAYSHARE_BIT )) { add =region_add (resv_map, from, to, regions_needed, h, h_cg);if (unlikely (add <0 )) {
@@ -6735,7 +6735,7 @@ long hugetlb_reserve_pages(struct inode *inode,hugetlb_cgroup_uncharge_cgroup_rsvd (hstate_index (h), chg *pages_per_huge_page (h), h_cg); out_err: -if (!desc ||vma_desc_test_any (desc,VMA_MAYSHARE_BIT )) +if (!desc ||vma_desc_test (desc,VMA_MAYSHARE_BIT ))/* Only call region_abort if the region_chg succeeded but the * region_add failed or didn't run. */
tools/testing/vma/include/dup.h +6 -0
@@ -922,6 +922,12 @@ static inline void vma_set_flags_mask(struct vm_area_struct *vma,#define vma_set_flags (vma, ...) \ vma_set_flags_mask(vma, mk_vma_flags(__VA_ARGS__)) +static __always_inline bool vma_desc_test(const struct vm_area_desc *desc, +vma_flag_t bit) +{ +return vma_flags_test (&desc->vma_flags, bit); +} +static inline bool vma_desc_test_any_mask (const struct vm_area_desc *desc,vma_flags_t flags) {
Signature · 1 line
2.53.0
Lorenzo Stoakes (Oracle)
maintainerPATCH 6/6 tools/testing/vma: add test for vma_flags_test(), vma_desc_test()
In reply to Lorenzo Stoakes (Oracle)Now we have helpers which test singular VMA flags - vma_flags_test() and vma_desc_test() - add a test to explicitly assert that these behave as expected.
--- tools/testing/vma/tests/vma.c | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)
tools/testing/vma/tests/vma.c +36 -0
@@ -159,6 +159,41 @@ static bool test_vma_flags_word(void)return true; } +/* Ensure that vma_flags_test() and friends works correctly. */ +static bool test_vma_flags_test (void ) +{ +const vma_flags_t flags =mk_vma_flags (VMA_READ_BIT ,VMA_WRITE_BIT , +VMA_EXEC_BIT ,64 ,65 ); +struct vm_area_desc desc; + + desc.vma_flags = flags; + +#define do_test (_flag) \ + ASSERT_TRUE(vma_flags_test(&flags, _flag)); \ + ASSERT_TRUE(vma_desc_test(&desc, _flag)) + +#define do_test_false (_flag) \ + ASSERT_FALSE(vma_flags_test(&flags, _flag)); \ + ASSERT_FALSE(vma_desc_test(&desc, _flag)) + +do_test (VMA_READ_BIT ); +do_test (VMA_WRITE_BIT ); +do_test (VMA_EXEC_BIT ); +#if NUM_VMA_FLAG_BITS >64 +do_test (64 ); +do_test (65 ); +#endif +do_test_false (VMA_MAYWRITE_BIT ); +#if NUM_VMA_FLAG_BITS >64 +do_test_false (66 ); +#endif + +#undef do_test +#undef do_test_false + +return true; +} +/* Ensure that vma_flags_test_any() and friends works correctly. */ static bool test_vma_flags_test_any (void ) {
@@ -334,6 +369,7 @@ static void run_vma_tests(int *num_tests, int *num_fail)TEST (vma_flags_unchanged);TEST (vma_flags_cleared);TEST (vma_flags_word); +TEST (vma_flags_test);TEST (vma_flags_test_any);TEST (vma_flags_clear); }
Signature · 1 line
2.53.0
Re: [PATCH 4/6] mm: reintroduce vma_flags_test() as a singular flag test
In reply to Lorenzo Stoakes (Oracle)Quoted 7 lines · Lorenzo Stoakes (Oracle)
Since we've now renamed
vma_flags_test()tovma_flags_test_any()to be very clear as to what we are in fact testing, we now have the opportunity to bringvma_flags_test()back, but for explicitly testing a single VMA flag.This is useful, as often flag tests are against a single flag, and
vma_flags_test_any(flags, VMA_READ_BIT)reads oddly and potentially causes confusion.
Agreed
Signature · 3 lines
Cheers, David
Re: [PATCH 5/6] mm: reintroduce vma_desc_test() as a singular flag test
In reply to Lorenzo Stoakes (Oracle)Quoted 8 lines · Lorenzo Stoakes (Oracle)
Similar to
vma_flags_test(), we have previously renamedvma_desc_test()tovma_desc_test_any(). Now that is in place, we can reintroducevma_desc_test()to explicitly check for a single VMA flag.As with
vma_flags_test(), this is useful as often flag tests are against a single flag, andvma_desc_test_any(flags, VMA_READ_BIT)reads oddly and potentially causes confusion.
Quoted 5 lines
As with
vma_flags_test()a combination of sparse andvma_flags_tbeing a struct means that users cannot misuse this function without it getting flagged.Also update the VMA tests to reflect this change.
Signature · 3 lines
Cheers, David
David Hildenbrand (Arm)
maintainerRe: [PATCH 6/6] tools/testing/vma: add test for vma_flags_test(), vma_desc_test()
In reply to Lorenzo Stoakes (Oracle)Quoted 14 lines · Lorenzo Stoakes (Oracle)
Now we have helpers which test singular VMA flags -
vma_flags_test()andvma_desc_test()- add a test to explicitly assert that these behave as expected.Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>--- tools/testing/vma/tests/vma.c | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)tools/testing/vma/tests/vma.c +0
-0
tools/testing/vma/tests/vma.c static bool test_vma_flags_word(void) · line 166
Quoted 8 lines
tools/testing/vma/tests/vma.c +5
-0@@ -159,6 +159,41 @@ static bool test_vma_flags_word(void)return true; } +/* Ensure that vma_flags_test() and friends works correctly. */ +static bool test_vma_flags_test (void ) +{ +const vma_flags_t flags =mk_vma_flags (VMA_READ_BIT ,VMA_WRITE_BIT , +VMA_EXEC_BIT ,64 ,65 );
When already using numbers, I was wondering whether you'd want to stick to numbers only here.
tools/testing/vma/tests/vma.c static bool test_vma_flags_word(void) · line 167
Quoted 1 line
tools/testing/vma/tests/vma.c +1
-0+struct vm_area_desc desc;
struct vm_area_desc desc = {
.vma_flags = flags,
};
?
Signature · 3 lines
Cheers, David
Re: [PATCH 3/6] mm: always inline __mk_vma_flags() and invoked functions
In reply to Lorenzo Stoakes (Oracle)Quoted 10 lines · Lorenzo Stoakes (Oracle)
Be explicit about
__mk_vma_flags()(which is used by themk_vma_flags()macro) always being inline, as we rely on the compiler converting this function into meaningful.Also update all of the functions
__mk_vma_flags()ultimately invokes to be always inline too.Note that
test_bitmap_const_eval()asserts that the relevant bitmap functions result in build time constant values.
Quoted 8 lines
Additionally,
vma_flag_set()operates on avma_flags_ttype, so it is inconsistently named versus other VMA flags functions.We only use
vma_flag_set()in__mk_vma_flags()so we don't need to worry about its new name being rather cumbersome, so rename it tovma_flags_set_flag()to disambiguate it fromvma_flags_set().Also update the VMA test headers to reflect the changes.
Signature · 3 lines
Cheers, David
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH 6/6] tools/testing/vma: add test for vma_flags_test(), vma_desc_test()
In reply to David Hildenbrand (Arm)Quoted 18 lines · David Hildenbrand (Arm)
Quoted 17 lines · Lorenzo Stoakes (Oracle)
Now we have helpers which test singular VMA flags -
vma_flags_test()andvma_desc_test()- add a test to explicitly assert that these behave as expected.Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>--- tools/testing/vma/tests/vma.c | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)tools/testing/vma/tests/vma.c +0
-0@@ -159,6 +159,41 @@ static bool test_vma_flags_word(void)return true; }
Quoted 8 lines
tools/testing/vma/tests/vma.c static bool test_vma_flags_word(void) · line 166
Quoted 5 lines
tools/testing/vma/tests/vma.c +5
-0+/* Ensure that vma_flags_test() and friends works correctly. */ +static bool test_vma_flags_test (void ) +{ +const vma_flags_t flags =mk_vma_flags (VMA_READ_BIT ,VMA_WRITE_BIT , +VMA_EXEC_BIT ,64 ,65 );When already using numbers, I was wondering whether you'd want to stick to numbers only here.
Numbers are for flags > 64 bits, we currently don't define any, it's to make sure everything works at higher bitmap sizes, the tests currently set the bitmap size to 128 bits.
Quoted 9 lines
tools/testing/vma/tests/vma.c static bool test_vma_flags_word(void) · line 167
Quoted 2 lines
tools/testing/vma/tests/vma.c +1
-0+struct vm_area_desc desc;struct
vm_area_descdesc = {
.vma_flags = flags,
};?
Ack can do, fix-patch for Andrew below :)
Cheers, Lorenzo
----8<----
From 5cc64e6c1884aaf995ce6398e36d5844c246352d Mon Sep 17 00:00:00 2001
From: "Lorenzo Stoakes (Oracle)" <[email protected]>
Date: Thu, 5 Mar 2026 14:59:58 +0000
Subject: [PATCH] fix
--- tools/testing/vma/tests/vma.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
tools/testing/vma/tests/vma.c +3 -3
@@ -164,9 +164,9 @@ static bool test_vma_flags_test(void) {const vma_flags_t flags =mk_vma_flags (VMA_READ_BIT ,VMA_WRITE_BIT ,VMA_EXEC_BIT ,64 ,65 ); -struct vm_area_desc desc; - - desc.vma_flags = flags; +struct vm_area_desc desc = { + .vma_flags = flags, + };#define do_test (_flag) \ ASSERT_TRUE(vma_flags_test(&flags, _flag)); \
--
2.53.0
Re: [PATCH 6/6] tools/testing/vma: add test for vma_flags_test(), vma_desc_test()
Quoted 28 lines · Lorenzo Stoakes (Oracle)
Quoted 18 lines · David Hildenbrand (Arm)
Quoted 17 lines · Lorenzo Stoakes (Oracle)
Now we have helpers which test singular VMA flags -
vma_flags_test()andvma_desc_test()- add a test to explicitly assert that these behave as expected.Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>--- tools/testing/vma/tests/vma.c | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)tools/testing/vma/tests/vma.c +0
-0@@ -159,6 +159,41 @@ static bool test_vma_flags_word(void)return true; }Quoted 8 lines
tools/testing/vma/tests/vma.c static bool test_vma_flags_word(void) · line 166
Quoted 5 lines
tools/testing/vma/tests/vma.c +5
-0+/* Ensure that vma_flags_test() and friends works correctly. */ +static bool test_vma_flags_test (void ) +{ +const vma_flags_t flags =mk_vma_flags (VMA_READ_BIT ,VMA_WRITE_BIT , +VMA_EXEC_BIT ,64 ,65 );When already using numbers, I was wondering whether you'd want to stick to numbers only here.
Quoted 3 lines
Numbers are for flags > 64 bits, we currently don't define any, it's to make sure everything works at higher bitmap sizes, the tests currently set the bitmap size to 128 bits.
Right, I was just saying, purely using numbers would also work and make the numbers 64/65 not stick out as much :)
Quoted 9 lines
Quoted 9 lines
tools/testing/vma/tests/vma.c static bool test_vma_flags_word(void) · line 167
Quoted 3 lines
tools/testing/vma/tests/vma.c +1
-0+struct vm_area_desc desc;struct
vm_area_descdesc = {
.vma_flags = flags,
};
Quoted 3 lines
Quoted 1 line
?
Ack can do, fix-patch for Andrew below :)
In case I missed it:
Signature · 3 lines
Cheers, David
Andrew Morton
maintainerQuoted 7 lines · On Thu, 5 Mar 2026 10:50:13 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
The ongoing work around introducing non-system word VMA flags has introduced a number of helper functions and macros to make life easier when working with these flags and to make conversions from the legacy use of
VM_xxxflags more straightforward.This series improves these to reduce confusion as to what they do and to improve consistency and readability.
Thanks, I quietly added this (and the -fix) to mm.git's mm-new branch.
Andrew Morton
maintainerRe: [PATCH 2/6] mm: add vma_desc_test_all() and use it
In reply to Lorenzo Stoakes (Oracle)Quoted 9 lines · On Thu, 5 Mar 2026 10:50:15 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
erofs and zonefs are using
vma_desc_test_any()twice to check whether all ofVMA_SHARED_BITandVMA_MAYWRITE_BITare set, this is silly, so addvma_desc_test_all()to test all flags and update erofs and zonefs to use it.While we're here, update the helper function comments to be more consistent.
Also add the same to the VMA test headers.
fwiw, we have no review tags on this one.
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH 2/6] mm: add vma_desc_test_all() and use it
Quoted 7 lines · Andrew Morton
Quoted 5 lines · On Thu, 5 Mar 2026 10:50:15 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
erofs and zonefs are using
vma_desc_test_any()twice to check whether all ofVMA_SHARED_BITandVMA_MAYWRITE_BITare set, this is silly, so addvma_desc_test_all()to test all flags and update erofs and zonefs to use it.
Quoted 6 lines
Quoted 4 lines
While we're here, update the helper function comments to be more consistent.
Also add the same to the VMA test headers.
fwiw, we have no review tags on this one.
Based on the discussion we had about this previously I was under the impression if submitted by a maintainer that wasn't required?
I'll nag people, but I'm a bit surprised if this is why you haven't moved this to mm-stable, given how trivially obviously correct this patch is.
Lorenzo
Re: [PATCH 2/6] mm: add vma_desc_test_all() and use it
In reply to Lorenzo Stoakes (Oracle)Quoted 11 lines · Lorenzo Stoakes (Oracle)
erofs and zonefs are using
vma_desc_test_any()twice to check whether all ofVMA_SHARED_BITandVMA_MAYWRITE_BITare set, this is silly, so addvma_desc_test_all()to test all flags and update erofs and zonefs to use it.While we're here, update the helper function comments to be more consistent.
Also add the same to the VMA test headers.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>
I thought I saw David review all of the series and so focused on other stuff, didn't notice he skipped this one :)
Quoted 111 lines
--- fs/erofs/data.c | 3 +-- fs/zonefs/file.c | 3 +-- include/linux/mm.h | 24 ++++++++++++++++++++---- tools/testing/vma/include/dup.h | 9 +++++++++ 4 files changed, 31 insertions(+), 8 deletions(-)fs/erofs/data.c +1
-2@@ -473,8 +473,7 @@ static int erofs_file_mmap_prepare(struct vm_area_desc *desc)if (!IS_DAX (file_inode (desc->file)))return generic_file_readonly_mmap_prepare (desc); -if (vma_desc_test_any (desc,VMA_SHARED_BIT ) && -vma_desc_test_any (desc,VMA_MAYWRITE_BIT )) +if (vma_desc_test_all (desc,VMA_SHARED_BIT ,VMA_MAYWRITE_BIT ))return -EINVAL ; desc->vm_ops = &erofs_dax_vm_ops;fs/zonefs/file.c +1
-2@@ -333,8 +333,7 @@ static int zonefs_file_mmap_prepare(struct vm_area_desc *desc)* ordering between msync() and page cache writeback. */ if (zonefs_inode_is_seq (file_inode (file)) && -vma_desc_test_any (desc,VMA_SHARED_BIT ) && -vma_desc_test_any (desc,VMA_MAYWRITE_BIT )) +vma_desc_test_all (desc,VMA_SHARED_BIT ,VMA_MAYWRITE_BIT ))return -EINVAL ;file_accessed (file);include/linux/mm.h +20
-4@@ -1178,7 +1178,7 @@ static inline void vma_set_flags_mask(struct vm_area_struct *vma,#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. */ +/* Helper to test any VMA flags in a VMA descriptor. */ static inline bool vma_desc_test_any_mask (const struct vm_area_desc *desc,vma_flags_t flags) {@@ -1186,8 +1186,8 @@ static inline bool vma_desc_test_any_mask(const struct vm_area_desc *desc, }/* -* Helper macro for testing VMA flags for an input pointer to a struct -* vm_area_desc object describing a proposed VMA, e.g.: +* Helper macro for testing whether any VMA flags are set in a VMA descriptor, +* e.g.: * * if (vma_desc_test_any(desc, VMA_IO_BIT, VMA_PFNMAP_BIT, * VMA_DONTEXPAND_BIT, VMA_DONTDUMP_BIT)) { ... } @@ -1195,6 +1195,22 @@ static inline bool vma_desc_test_any_mask(const struct vm_area_desc *desc,#define vma_desc_test_any (desc, ...) \ vma_desc_test_any_mask(desc, mk_vma_flags(__VA_ARGS__)) +/* Helper to test all VMA flags in a VMA descriptor. */ +static inline bool vma_desc_test_all_mask (const struct vm_area_desc *desc, +vma_flags_t flags) +{ +return vma_flags_test_all_mask (&desc->vma_flags, flags); +} + +/* +* Helper macro for testing whether ALL VMA flags are set in a VMA descriptor, +* e.g.: +* +* if (vma_desc_test_all(desc, VMA_READ_BIT, VMA_MAYREAD_BIT)) { ... } +*/ +#define vma_desc_test_all (desc, ...) \ + vma_desc_test_all_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)@@ -1207,7 +1223,7 @@ static inline void vma_desc_set_flags_mask(struct vm_area_desc *desc,* 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); +* VMA_DONTDUMP_BIT); */ #define vma_desc_set_flags (desc, ...) \ vma_desc_set_flags_mask(desc, mk_vma_flags(__VA_ARGS__))tools/testing/vma/include/dup.h +9
-0@@ -922,6 +922,15 @@ static inline bool vma_desc_test_any_mask(const struct vm_area_desc *desc,#define vma_desc_test_any (desc, ...) \ vma_desc_test_any_mask(desc, mk_vma_flags(__VA_ARGS__)) +static inline bool vma_desc_test_all_mask (const struct vm_area_desc *desc, +vma_flags_t flags) +{ +return vma_flags_test_all_mask (&desc->vma_flags, flags); +} + +#define vma_desc_test_all (desc, ...) \ + vma_desc_test_all_mask(desc, mk_vma_flags(__VA_ARGS__)) +static inline void vma_desc_set_flags_mask (struct vm_area_desc *desc,vma_flags_t flags) {
Re: [PATCH 1/6] mm: rename VMA flag helpers to be more readable
In reply to Lorenzo Stoakes (Oracle)Quoted 16 lines · Lorenzo Stoakes (Oracle)
On reflection, it's confusing to have
vma_flags_test()andvma_desc_test_flags()test whether any comma-separated VMA flag bit is set, while also havingvma_flags_test_all()andvma_test_all_flags()separately test whether all flags are set.Firstly, rename
vma_flags_test()tovma_flags_test_any()to eliminate this confusion.Secondly, since the VMA descriptor flag functions are becoming rather cumbersome, prefer vma_desc_test*() to vma_desc_test_flags*(), and also rename
vma_desc_test_flags()tovma_desc_test_any().Finally, rename
vma_test_all_flags()tovma_test_all()to keep the VMA-specific helper consistent with the VMA descriptor naming convention and to help avoid confusion vs.vma_flags_test_all().
Quoted 6 lines
While we're here, also update whitespace to be consistent in helper functions.
Suggested-by Pedro Falcato <[email protected]>Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>---
Signature · 3 lines
Cheers, David
Re: [PATCH 2/6] mm: add vma_desc_test_all() and use it
In reply to Lorenzo Stoakes (Oracle)Quoted 5 lines · Lorenzo Stoakes (Oracle)
erofs and zonefs are using
vma_desc_test_any()twice to check whether all ofVMA_SHARED_BITandVMA_MAYWRITE_BITare set, this is silly, so addvma_desc_test_all()to test all flags and update erofs and zonefs to use it.
Quoted 7 lines
While we're here, update the helper function comments to be more consistent.
Also add the same to the VMA test headers.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>---
Nothing jumped at me
Signature · 3 lines
Cheers, David
Re: [PATCH 6/6] tools/testing/vma: add test for vma_flags_test(), vma_desc_test()
In reply to Lorenzo Stoakes (Oracle)Quoted 40 lines · Lorenzo Stoakes (Oracle)
Quoted 18 lines · David Hildenbrand (Arm)
Quoted 17 lines · Lorenzo Stoakes (Oracle)
Now we have helpers which test singular VMA flags -
vma_flags_test()andvma_desc_test()- add a test to explicitly assert that these behave as expected.Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>--- tools/testing/vma/tests/vma.c | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)tools/testing/vma/tests/vma.c +0
-0@@ -159,6 +159,41 @@ static bool test_vma_flags_word(void)return true; }Quoted 8 lines
tools/testing/vma/tests/vma.c static bool test_vma_flags_word(void) · line 166
Quoted 5 lines
tools/testing/vma/tests/vma.c +5
-0+/* Ensure that vma_flags_test() and friends works correctly. */ +static bool test_vma_flags_test (void ) +{ +const vma_flags_t flags =mk_vma_flags (VMA_READ_BIT ,VMA_WRITE_BIT , +VMA_EXEC_BIT ,64 ,65 );When already using numbers, I was wondering whether you'd want to stick to numbers only here.
Numbers are for flags > 64 bits, we currently don't define any, it's to make sure everything works at higher bitmap sizes, the tests currently set the bitmap size to 128 bits.
Quoted 8 lines
tools/testing/vma/tests/vma.c static bool test_vma_flags_word(void) · line 167
Quoted 2 lines
tools/testing/vma/tests/vma.c +1
-0+struct vm_area_desc desc;struct
vm_area_descdesc = {
.vma_flags = flags,
};
Quoted 3 lines
Quoted 1 line
?
Ack can do, fix-patch for Andrew below :)
Not sure if I provided a tag, but here it comes
Signature · 3 lines
Cheers, David
David Hildenbrand (Arm)
maintainerRe: [PATCH 2/6] mm: add vma_desc_test_all() and use it
In reply to Vlastimil Babka (SUSE)Quoted 9 lines · Vlastimil Babka (SUSE)
Quoted 8 lines · Lorenzo Stoakes (Oracle)
erofs and zonefs are using
vma_desc_test_any()twice to check whether all ofVMA_SHARED_BITandVMA_MAYWRITE_BITare set, this is silly, so addvma_desc_test_all()to test all flags and update erofs and zonefs to use it.While we're here, update the helper function comments to be more consistent.
Quoted 6 lines
Quoted 3 lines
Also add the same to the VMA test headers.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>I thought I saw David review all of the series and so focused on other stuff, didn't notice he skipped this one :)
I think I skipped it because it looked too mechanical when scanning and I was like "ofc I trust Lorenzo on that one blindly". So I missed to reply.
Tag provided now if it helps.
Signature · 3 lines
Cheers, David
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH 2/6] mm: add vma_desc_test_all() and use it
Quoted 14 lines · David Hildenbrand (Arm)
Quoted 13 lines · Vlastimil Babka (SUSE)
Quoted 11 lines · Lorenzo Stoakes (Oracle)
erofs and zonefs are using
vma_desc_test_any()twice to check whether all ofVMA_SHARED_BITandVMA_MAYWRITE_BITare set, this is silly, so addvma_desc_test_all()to test all flags and update erofs and zonefs to use it.While we're here, update the helper function comments to be more consistent.
Also add the same to the VMA test headers.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>
Quoted 7 lines
Quoted 2 lines
I thought I saw David review all of the series and so focused on other stuff, didn't notice he skipped this one :)
I think I skipped it because it looked too mechanical when scanning and I was like "ofc I trust Lorenzo on that one blindly". So I missed to reply.
Tag provided now if it helps.
Thanks guys! Appreciated.
Andrew Morton
maintainerRe: [PATCH 2/6] mm: add vma_desc_test_all() and use it
In reply to Lorenzo Stoakes (Oracle)Quoted 11 lines · On Wed, 25 Mar 2026 07:08:22 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
Quoted 10 lines · Andrew Morton
Quoted 8 lines · On Thu, 5 Mar 2026 10:50:15 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
erofs and zonefs are using
vma_desc_test_any()twice to check whether all ofVMA_SHARED_BITandVMA_MAYWRITE_BITare set, this is silly, so addvma_desc_test_all()to test all flags and update erofs and zonefs to use it.While we're here, update the helper function comments to be more consistent.
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Also add the same to the VMA test headers.
fwiw, we have no review tags on this one.
Based on the discussion we had about this previously I was under the impression if submitted by a maintainer that wasn't required?
Well, it's a gray area. Obviously it's better if people's stuff is checked by co-maintainers or by others.
I'm not inclined to make a fuss about it though (hence "fwiw"). Quite a lot of unreviewed maintainer-authored material ends up going upstream and I don't think that's causing much harm.
In a lot of cases this is pretty much unavoidable because the patch comes from a sole maintainer (SJ, Sergey, Ulad, Liam come to mind). But when the author has co-maintainers, perhaps those people could step up.
Quoted 2 lines
I'll nag people, but I'm a bit surprised if this is why you haven't moved this to mm-stable, given how trivially obviously correct this patch is.
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/pc/devel-series
shows my expected merging order. It looks like this one will be in the next batch ->mm-stable.
Re: [PATCH 1/6] mm: rename VMA flag helpers to be more readable
In reply to Lorenzo Stoakes (Oracle)Quoted 7 lines · Lorenzo Stoakes (Oracle)
On reflection, it's confusing to have
vma_flags_test()andvma_desc_test_flags()test whether any comma-separated VMA flag bit is set, while also havingvma_flags_test_all()andvma_test_all_flags()separately test whether all flags are set.Firstly, rename
vma_flags_test()tovma_flags_test_any()to eliminate this confusion.
Hmm. The names are getting longer. We should fix this One Day.
Quoted 6 lines
Secondly, since the VMA descriptor flag functions are becoming rather cumbersome, prefer vma_desc_test*() to vma_desc_test_flags*(), and also rename
vma_desc_test_flags()tovma_desc_test_any().
Quoted 6 lines
Finally, rename
vma_test_all_flags()tovma_test_all()to keep the VMA-specific helper consistent with the VMA descriptor naming convention and to help avoid confusion vs.vma_flags_test_all().While we're here, also update whitespace to be consistent in helper functions.
Extremely amazing patch! you were truly inspired!
Quoted 2 lines
Suggested-by Pedro Falcato <[email protected]>Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>
Signature · 1 line
Pedro
Re: [PATCH 2/6] mm: add vma_desc_test_all() and use it
In reply to Lorenzo Stoakes (Oracle)Quoted 11 lines · Lorenzo Stoakes (Oracle)
erofs and zonefs are using
vma_desc_test_any()twice to check whether all ofVMA_SHARED_BITandVMA_MAYWRITE_BITare set, this is silly, so addvma_desc_test_all()to test all flags and update erofs and zonefs to use it.While we're here, update the helper function comments to be more consistent.
Also add the same to the VMA test headers.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>
Signature · 1 line
Pedro
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH 1/6] mm: rename VMA flag helpers to be more readable
In reply to Pedro FalcatoQuoted 10 lines · Pedro Falcato
Quoted 7 lines · Lorenzo Stoakes (Oracle)
On reflection, it's confusing to have
vma_flags_test()andvma_desc_test_flags()test whether any comma-separated VMA flag bit is set, while also havingvma_flags_test_all()andvma_test_all_flags()separately test whether all flags are set.Firstly, rename
vma_flags_test()tovma_flags_test_any()to eliminate this confusion.Hmm. The names are getting longer. We should fix this One Day.
:) yeha it's a bit of a trade-off still. But for now keeping it (fairly) straightforward.
Quoted 14 lines
Quoted 7 lines
Secondly, since the VMA descriptor flag functions are becoming rather cumbersome, prefer vma_desc_test*() to vma_desc_test_flags*(), and also rename
vma_desc_test_flags()tovma_desc_test_any().Quoted 6 lines
Finally, rename
vma_test_all_flags()tovma_test_all()to keep the VMA-specific helper consistent with the VMA descriptor naming convention and to help avoid confusion vs.vma_flags_test_all().While we're here, also update whitespace to be consistent in helper functions.
Quoted 7 lines
Extremely amazing patch! you were truly inspired!
Quoted 2 lines
Suggested-by Pedro Falcato <[email protected]>Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>Reviewed-by Pedro Falcato <[email protected]>
:) thanks, I don't know who could have suggested such an amazing idea, well I won't read the tags to check + spoil the surprise... :>)
Quoted 3 lines
--
Pedro
Cheers, Lorenzo
Re: [PATCH 3/6] mm: always inline __mk_vma_flags() and invoked functions
In reply to Lorenzo Stoakes (Oracle)Quoted 3 lines · Lorenzo Stoakes (Oracle)
Be explicit about
__mk_vma_flags()(which is used by themk_vma_flags()macro) always being inline, as we rely on the compiler converting this function into meaningful.
meaningful what?
Quoted 10 lines
Also update all of the functions
__mk_vma_flags()ultimately invokes to be always inline too.Note that
test_bitmap_const_eval()asserts that the relevant bitmap functions result in build time constant values.Additionally,
vma_flag_set()operates on avma_flags_ttype, so it is inconsistently named versus other VMA flags functions.
Quoted 7 lines
We only use
vma_flag_set()in__mk_vma_flags()so we don't need to worry about its new name being rather cumbersome, so rename it tovma_flags_set_flag()to disambiguate it fromvma_flags_set().Also update the VMA test headers to reflect the changes.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>
Is there an actual difference in codegen here? On -O2.
Signature · 1 line
Pedro
Re: [PATCH 4/6] mm: reintroduce vma_flags_test() as a singular flag test
In reply to Lorenzo Stoakes (Oracle)Quoted 12 lines · Lorenzo Stoakes (Oracle)
Since we've now renamed
vma_flags_test()tovma_flags_test_any()to be very clear as to what we are in fact testing, we now have the opportunity to bringvma_flags_test()back, but for explicitly testing a single VMA flag.This is useful, as often flag tests are against a single flag, and
vma_flags_test_any(flags, VMA_READ_BIT)reads oddly and potentially causes confusion.We use sparse to enforce that users won't accidentally pass
vm_flags_tto this function without it being flagged so this should make it harder to get this wrong.
Quoted 6 lines
Of course, passing
vma_flags_tto the function is impossible, as it is a struct.Also update the VMA tests to reflect this change.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>
This is a lot nicer, though I am wondering if there is any difference in codegen as well...
Signature · 1 line
Pedro
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH 3/6] mm: always inline __mk_vma_flags() and invoked functions
In reply to Pedro FalcatoQuoted 5 lines · Pedro Falcato
Quoted 3 lines · Lorenzo Stoakes (Oracle)
Be explicit about
__mk_vma_flags()(which is used by themk_vma_flags()macro) always being inline, as we rely on the compiler converting this function into meaningful.meaningful what?
'into the equivalent compile-time constant code' probably fine.
Andrew - could you update that if there's time?
Quoted 15 lines
Quoted 15 lines
Also update all of the functions
__mk_vma_flags()ultimately invokes to be always inline too.Note that
test_bitmap_const_eval()asserts that the relevant bitmap functions result in build time constant values.Additionally,
vma_flag_set()operates on avma_flags_ttype, so it is inconsistently named versus other VMA flags functions.We only use
vma_flag_set()in__mk_vma_flags()so we don't need to worry about its new name being rather cumbersome, so rename it tovma_flags_set_flag()to disambiguate it fromvma_flags_set().
Quoted 7 lines
Quoted 3 lines
Also update the VMA test headers to reflect the changes.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>Reviewed-by Pedro Falcato <[email protected]>Is there an actual difference in codegen here? On -O2.
No, this is more about consistency.
Quoted 3 lines
--
Pedro
Thanks, Lorenzo
Re: [PATCH 5/6] mm: reintroduce vma_desc_test() as a singular flag test
In reply to Lorenzo Stoakes (Oracle)Quoted 8 lines · Lorenzo Stoakes (Oracle)
Similar to
vma_flags_test(), we have previously renamedvma_desc_test()tovma_desc_test_any(). Now that is in place, we can reintroducevma_desc_test()to explicitly check for a single VMA flag.As with
vma_flags_test(), this is useful as often flag tests are against a single flag, andvma_desc_test_any(flags, VMA_READ_BIT)reads oddly and potentially causes confusion.
Quoted 7 lines
As with
vma_flags_test()a combination of sparse andvma_flags_tbeing a struct means that users cannot misuse this function without it getting flagged.Also update the VMA tests to reflect this change.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>
Similarly LGTM.
Signature · 1 line
Pedro
Re: [PATCH 6/6] tools/testing/vma: add test for vma_flags_test(), vma_desc_test()
In reply to Lorenzo Stoakes (Oracle)Quoted 5 lines · Lorenzo Stoakes (Oracle)
Now we have helpers which test singular VMA flags -
vma_flags_test()andvma_desc_test()- add a test to explicitly assert that these behave as expected.Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>
Signature · 1 line
Pedro
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH 4/6] mm: reintroduce vma_flags_test() as a singular flag test
In reply to Pedro FalcatoQuoted 16 lines · Pedro Falcato
Quoted 15 lines · Lorenzo Stoakes (Oracle)
Since we've now renamed
vma_flags_test()tovma_flags_test_any()to be very clear as to what we are in fact testing, we now have the opportunity to bringvma_flags_test()back, but for explicitly testing a single VMA flag.This is useful, as often flag tests are against a single flag, and
vma_flags_test_any(flags, VMA_READ_BIT)reads oddly and potentially causes confusion.We use sparse to enforce that users won't accidentally pass
vm_flags_tto this function without it being flagged so this should make it harder to get this wrong.Of course, passing
vma_flags_tto the function is impossible, as it is a struct.
Quoted 8 lines
Quoted 3 lines
Also update the VMA tests to reflect this change.
Signed-off-by Lorenzo Stoakes (Oracle) <[email protected]>Reviewed-by Pedro Falcato <[email protected]>This is a lot nicer, though I am wondering if there is any difference in codegen as well...
Not that I could tell, this was more about cromulence.
Quoted 3 lines
--
Pedro
Thanks, Lorenzo
Andrew Morton
maintainerRe: [PATCH 3/6] mm: always inline __mk_vma_flags() and invoked functions
In reply to Lorenzo Stoakes (Oracle)Quoted 10 lines · On Wed, 25 Mar 2026 14:58:14 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
Quoted 5 lines · Pedro Falcato
Quoted 3 lines · Lorenzo Stoakes (Oracle)
Be explicit about
__mk_vma_flags()(which is used by themk_vma_flags()macro) always being inline, as we rely on the compiler converting this function into meaningful.meaningful what?
'into the equivalent compile-time constant code' probably fine.
Andrew - could you update that if there's time?
np
: Be explicit about __mk_vma_flags() (which is used by the mk_vma_flags() : macro) always being inline, as we rely on the compiler converting this : function into the equivalent compile-time constant code.
what does "compile-time constant code" actually mean? That constants within the code are evaluated at compile-time?
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH 2/6] mm: add vma_desc_test_all() and use it
In reply to Andrew MortonQuoted 15 lines · Andrew Morton
Quoted 13 lines · On Wed, 25 Mar 2026 07:08:22 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
Quoted 12 lines · Andrew Morton
Quoted 9 lines · On Thu, 5 Mar 2026 10:50:15 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
erofs and zonefs are using
vma_desc_test_any()twice to check whether all ofVMA_SHARED_BITandVMA_MAYWRITE_BITare set, this is silly, so addvma_desc_test_all()to test all flags and update erofs and zonefs to use it.While we're here, update the helper function comments to be more consistent.
Also add the same to the VMA test headers.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
fwiw, we have no review tags on this one.
Based on the discussion we had about this previously I was under the impression if submitted by a maintainer that wasn't required?
Well, it's a gray area. Obviously it's better if people's stuff is checked by co-maintainers or by others.
OK that contradicts the previous conversation we had where you had to convince me that this was ok (which I ended up agreeing with), rather than being a grey area.
We had quite a long conversation about maintainers are in a trusted role so a co-maintainer would have called it out it was wrong and etc.
But sure it'd be nice to get review, obviously I agree with that.
Quoted 4 lines
I'm not inclined to make a fuss about it though (hence "fwiw"). Quite a lot of unreviewed maintainer-authored material ends up going upstream and I don't think that's causing much harm.
I think you need to be a lot clearer in communicating these things while the process remains undocumented.
In this case for instance, I took that to mean that you required review, the 'fwiw' doesn't really make it clear that this was optional, especially given this patch is so trivial.
Quoted 5 lines
In a lot of cases this is pretty much unavoidable because the patch comes from a sole maintainer (SJ, Sergey, Ulad, Liam come to mind). But when the author has co-maintainers, perhaps those people could step up.
Right.
Quoted 8 lines
Quoted 3 lines
I'll nag people, but I'm a bit surprised if this is why you haven't moved this to mm-stable, given how trivially obviously correct this patch is.
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/pc/devel-series
shows my expected merging order. It looks like this one will be in the next batch ->mm-stable.
I'll definietly need some decoding of that to understand where each batch is?
To me it reads as a bunch of inscrutible #'s and file names...
Thanks, Lorenzo
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH 3/6] mm: always inline __mk_vma_flags() and invoked functions
In reply to Andrew MortonQuoted 17 lines · Andrew Morton
Quoted 10 lines · On Wed, 25 Mar 2026 14:58:14 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
Quoted 5 lines · Pedro Falcato
Quoted 3 lines · Lorenzo Stoakes (Oracle)
Be explicit about
__mk_vma_flags()(which is used by themk_vma_flags()macro) always being inline, as we rely on the compiler converting this function into meaningful.meaningful what?
'into the equivalent compile-time constant code' probably fine.
Andrew - could you update that if there's time?
np
Quoted 6 lines
: Be explicit about
__mk_vma_flags()(which is used by themk_vma_flags(): macro) always being inline, as we rely on the compiler converting this : function into the equivalent compile-time constant code.what does "compile-time constant code" actually mean? That constants within the code are evaluated at compile-time?
Yeah, so effectively the compiler rewrites:
x = mk_vma_flags(VMA_READ_BIT, VMA_WRITE_BIT);
To:
x = (1UL << VMA_READ_BIT) | (1UL << VMA_WRITE_BIT);
And then:
x = 3;
Various efforts at checking generated assembly has confirmed this.
Maybe 'into an inline constant value' is better?
Thanks, Lorenzo
Andrew Morton
maintainerRe: [PATCH 3/6] mm: always inline __mk_vma_flags() and invoked functions
Quoted 1 line · On Wed, 25 Mar 2026 16:23:53 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
Maybe 'into an inline constant value' is better?
<bikeshedbikeshed>
How about
: Be explicit about __mk_vma_flags() (which is used by the mk_vma_flags() : macro) always being inline, as we rely on the compiler turning all : constants into compile-time ones.
Lorenzo Stoakes (Oracle)
maintainerRe: [PATCH 3/6] mm: always inline __mk_vma_flags() and invoked functions
Quoted 4 lines · Andrew Morton
Quoted 1 line · On Wed, 25 Mar 2026 16:23:53 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
Maybe 'into an inline constant value' is better?
Quoted 8 lines
<bikeshedbikeshed>
How about
: Be explicit about
__mk_vma_flags()(which is used by themk_vma_flags(): macro) always being inline, as we rely on the compiler turning all : constants into compile-time ones.
Well I think that loses the meaning a bit.
Something like:
Be explicit about __mk_vma_flags() (which is used by the mk_vma_flags() -macro) always being inline, as we rely on the compiler converting this -function into meaningful.
+macro) always being inline, as we rely on the compiler to evaluate the +loop in this function and determine that it can replace the code with the +an equivalent constant value, e.g. that:
+
+__mk_vma_flags(2, (const vma_flag_t []){ VMA_WRITE_BIT, VMA_EXEC_BIT }); +
+Can be replaced with:
+
+(1UL << VMA_WRITE_BIT) | (1UL << VMA_EXEC_BIT)
+
+= (1UL << 1) | (1UL << 2) = 6
+
+Most likely an 'inline' will suffice for this, but be explicit as we can +be.
Should verbosely cover that off.
Thanks, Lorenzo
Andrew Morton
maintainerRe: [PATCH 3/6] mm: always inline __mk_vma_flags() and invoked functions
Quoted 27 lines · On Wed, 25 Mar 2026 18:44:13 +0000 "Lorenzo Stoakes (Oracle)" <[email protected]>…
Quoted 4 lines
: Be explicit about
__mk_vma_flags()(which is used by themk_vma_flags(): macro) always being inline, as we rely on the compiler turning all : constants into compile-time ones.Well I think that loses the meaning a bit.
Something like:
Be explicit about
__mk_vma_flags()(which is used by themk_vma_flags()(quoted diff) +14
-2-macro) always being inline, as we rely on the compiler converting this -function into meaningful. +macro) always being inline, as we rely on the compiler to evaluate the +loop in this function and determine that it can replace the code with the +an equivalent constant value, e.g. that: + +__mk_vma_flags(2, (const vma_flag_t []){ VMA_WRITE_BIT, VMA_EXEC_BIT }); + +Can be replaced with: + +(1UL << VMA_WRITE_BIT) | (1UL << VMA_EXEC_BIT) + += (1UL << 1) | (1UL << 2) = 6 + +Most likely an 'inline' will suffice for this, but be explicit as we can +be.
Quoted 1 line
Should verbosely cover that off.
ok ;)