convert more vm_flags_t users to vma_flags_t
Subsystems (33)
Lorenzo Stoakes
maintainercover convert more vm_flags_t users to vma_flags_t
This series makes further progress in converting usage of the deprecated vm_flags_t type to its replacement, vma_flags_t.
It focuses on mm, though updates some users of mm APIs also.
It updates:
* The core do_mmap() code path for VMA mapping.
* Unmapped area logic.
* The usage of mm->def_vma_flags.
* VMA page protection bit logic.
* General usage of VMA flags in core mm code, mlock, mprotect, mremap.
REVIEWER/MERGER NOTE: This is dependent on [0], which must be merged before this.
[0]:https://lore.kernel.org/linux-mm/[email protected]/
Lorenzo Stoakes (13):
mm: introduce vma_flags_can_grow() and vma_can_grow() mm/vma: update do_mmap() to use vma_flags_t
mm: convert __get_unmapped_area() to use vma_flags_t
mm: update generic_get_unmapped_area[_topdown]() to use vma_flags_t
mm: prefer mm->def_vma_flags in mm logic
mm/vma: convert vm_pgprot_modify() to use vma_flags_t and rename mm/vma: rename vma_get_page_prot to vma_flags_to_page_prot
mm: introduce vma_get_page_prot() and use it
mm/vma: update create_init_stack_vma() to use vma_flags_t mm/vma: convert miscellaneous uses of VMA flags in core mm mm/mlock: convert mlock code to use vma_flags_t mm/mprotect: convert mprotect code to use vma_flags_t mm/mremap: convert mremap code to use vma_flags_t
arch/mips/kernel/vdso.c | 4 +- arch/powerpc/mm/book3s64/slice.c | 6 +- drivers/gpu/drm/drm_gem.c | 2 +- drivers/gpu/drm/drm_gem_dma_helper.c | 2 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_gem.c | 6 +- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 12 +- drivers/gpu/drm/msm/msm_gem.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +- drivers/gpu/drm/omapdrm/omap_gem.c | 6 +- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 2 +- drivers/gpu/drm/tegra/gem.c | 2 +- drivers/gpu/drm/virtio/virtgpu_vram.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 2 +- drivers/gpu/drm/xe/xe_device.c | 2 +- drivers/gpu/drm/xe/xe_mmio_gem.c | 2 +- drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +- drivers/video/fbdev/core/fb_io_fops.c | 2 +- fs/aio.c | 2 +- fs/hugetlbfs/inode.c | 3 +- include/linux/huge_mm.h | 10 +- include/linux/memfd.h | 6 +- include/linux/mm.h | 51 ++++-- include/linux/sched/mm.h | 16 +- ipc/shm.c | 3 +- mm/debug.c | 2 +- mm/huge_memory.c | 21 +-- mm/memfd.c | 15 +- mm/mlock.c | 97 ++++++------ mm/mmap.c | 164 +++++++++++--------- mm/mprotect.c | 16 +- mm/mremap.c | 38 ++--- mm/nommu.c | 3 +- mm/util.c | 10 +- mm/vma.c | 27 ++-- mm/vma.h | 8 +- mm/vma_exec.c | 20 ++- sound/core/memalloc.c | 2 +- tools/testing/vma/include/dup.h | 22 ++- 41 files changed, 339 insertions(+), 261 deletions(-)
--
2.54.0
Lorenzo Stoakes
maintainerPATCH 1/13 mm: introduce vma_flags_can_grow() and vma_can_grow()
These test whether the VMA has stack sematics, i.e. is able to grow upwards or downwards depending on the architecture.
In order to account for arches which do not support upward-growing stacks, introduce VMA_GROWSUP whose definition depends on the architecture supporting it, and use vma_flags_test_single_mask() in vma_flags_can_grow() to account for this.
Update the VMA userland tests to reflect the changes
No functional change intended.
--- include/linux/mm.h | 21 ++++++++++++++++++--- tools/testing/vma/include/dup.h | 4 ++++ 2 files changed, 22 insertions(+), 3 deletions(-)
include/linux/mm.h +18 -3
@@ -472,6 +472,7 @@ enum {#define VM_SAO INIT_VM_FLAG(SAO)#elif defined(CONFIG_PARISC )#define VM_GROWSUP INIT_VM_FLAG(GROWSUP) +#define VMA_GROWSUP mk_vma_flags(VMA_GROWSUP_BIT)#elif defined(CONFIG_SPARC64 )#define VM_SPARC_ADI INIT_VM_FLAG(SPARC_ADI)#define VM_ARCH_CLEAR INIT_VM_FLAG(ARCH_CLEAR)
@@ -483,6 +484,7 @@ enum {#endif #ifndef VM_GROWSUP #define VM_GROWSUP VM_NONE +#define VMA_GROWSUP EMPTY_VMA_FLAGS#endif #ifdef CONFIG_ARM64_MTE #define VM_MTE INIT_VM_FLAG(MTE)
@@ -1563,11 +1565,24 @@ static inline bool vma_is_initial_stack(const struct vm_area_struct *vma) vma->vm_end >= vma->vm_mm->start_stack; } -static inline bool vma_is_temporary_stack (const struct vm_area_struct *vma) +static inline bool vma_flags_can_grow (const vma_flags_t *flags) { -int maybe_stack = vma->vm_flags & (VM_GROWSDOWN |VM_GROWSUP ); +if (vma_flags_test_single_mask (flags,VMA_GROWSUP )) +return true; +if (vma_flags_test (flags,VMA_GROWSDOWN_BIT )) +return true; + +return false; +} -if (!maybe_stack) +static inline bool vma_can_grow (const struct vm_area_struct *vma) +{ +return vma_flags_can_grow (&vma->flags); +} + +static inline bool vma_is_temporary_stack (const struct vm_area_struct *vma) +{ +if (!vma_can_grow (vma))return false; if ((vma->vm_flags & VM_STACK_INCOMPLETE_SETUP) ==
tools/testing/vma/include/dup.h +4 -0
@@ -245,8 +245,10 @@ enum {#define VM_STACK INIT_VM_FLAG(STACK)#ifdef CONFIG_STACK_GROWS_UP #define VM_STACK_EARLY INIT_VM_FLAG(STACK_EARLY) +#define VMA_STACK_EARLY mk_vma_flags(VMA_STACK_EARLY_BIT)#else #define VM_STACK_EARLY VM_NONE +#define VMA_STACK_EARLY EMPTY_VMA_FLAGS#endif #ifdef CONFIG_ARCH_HAS_PKEYS #define VM_PKEY_SHIFT ((__force int)VMA_HIGH_ARCH_0_BIT)
@@ -315,6 +317,8 @@ enum {/* Bits set in the VMA until the stack is in its final location */ #define VM_STACK_INCOMPLETE_SETUP (VM_RAND_READ | VM_SEQ_READ | VM_STACK_EARLY) +#define VMA_STACK_INCOMPLETE_SETUP append_vma_flags( \ + VMA_STACK_EARLY, VMA_RAND_READ_BIT, VMA_SEQ_READ_BIT)#define TASK_EXEC_BIT ((current->personality & READ_IMPLIES_EXEC) ? \ VM_EXEC_BIT : VM_READ_BIT)
Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerPATCH 2/13 mm/vma: update do_mmap() to use vma_flags_t
In reply to Lorenzo StoakesThe core do_mmap() function accepts a vm_flags_t parameter which it then manipulates before passing to mmap_region() to do the heavy lifting of the memory mapping.
Update do_mmap() to instead accept a vma_flags_t parameter, and adjust all the logic within do_mmap() to manipulate this instead.
This is as part of the ongoing effort to convert VMA flags from a system word size to a bitmap type which allows us to unrestrict the number of VMA flags, as well as gain control over how VMA flag manipulation occurs.
We do not cascade these changes to all functions which accept vm_flags_t, but rather use vma_flags_to_legacy() where necessary, specifically deferring converting calc_vm_prot_bits(), calc_vm_flag_bits() and __get_unmapped_area() to vma_flags_t.
Also utilise the new vma_flags_can_grow() predicate which correctly handles the case of architectures without upward growing stacks.
As part of this change, introduce VMA_SHADOW_STACK so we can correctly handle the case of the shadow stack not being defined.
No functional change intended.
--- arch/mips/kernel/vdso.c | 4 +-- fs/aio.c | 2 +- include/linux/memfd.h | 6 ++-- include/linux/mm.h | 6 ++-- ipc/shm.c | 3 +- mm/memfd.c | 15 ++++----- mm/mmap.c | 67 ++++++++++++++++++++++++----------------- mm/nommu.c | 3 +- mm/util.c | 10 +++--- mm/vma.c | 7 ++--- mm/vma.h | 2 +- 11 files changed, 69 insertions(+), 56 deletions(-)
arch/mips/kernel/vdso.c +2 -2
@@ -91,8 +91,8 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)/* Map delay slot emulation page */ base =do_mmap (NULL ,STACK_TOP ,PAGE_SIZE ,PROT_READ |PROT_EXEC , -MAP_ANONYMOUS |MAP_PRIVATE |MAP_FIXED ,0 ,0 , &unused, -NULL ); +MAP_ANONYMOUS |MAP_PRIVATE |MAP_FIXED , +EMPTY_VMA_FLAGS ,0 , &unused,NULL );if (IS_ERR_VALUE (base)) { ret = base; goto out;
fs/aio.c +1 -1
@@ -604,7 +604,7 @@ static int aio_setup_ring(struct kioctx *ctx, unsigned int nr_events) ctx->mmap_base =do_mmap (ctx->aio_ring_file,0 , ctx->mmap_size,PROT_READ |PROT_WRITE , -MAP_SHARED ,0 ,0 , &unused,NULL ); +MAP_SHARED ,EMPTY_VMA_FLAGS ,0 , &unused,NULL );mmap_write_unlock (mm);if (IS_ERR ((void *)ctx->mmap_base)) { ctx->mmap_size =0 ;
include/linux/memfd.h +3 -3
@@ -14,9 +14,9 @@ struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx);* to sealing, or 0 otherwise. * * We also update VMA flags if appropriate by manipulating the VMA flags pointed -* to by vm_flags_ptr. +* to by vma_flags_ptr. */ -int memfd_check_seals_mmap (struct file *file,vm_flags_t *vm_flags_ptr); +int memfd_check_seals_mmap (struct file *file,vma_flags_t *vma_flags_ptr);struct file *memfd_alloc_file (const char *name,unsigned int flags);int memfd_get_seals (struct file *file);int memfd_add_seals (struct file *file,unsigned int seals);
@@ -30,7 +30,7 @@ static inline struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx)return ERR_PTR (-EINVAL ); }static inline int memfd_check_seals_mmap (struct file *file, -vm_flags_t *vm_flags_ptr) +vma_flags_t *vma_flags_ptr) {return 0 ; }
include/linux/mm.h +4 -2
@@ -463,9 +463,11 @@ enum {#if defined(CONFIG_X86_USER_SHADOW_STACK ) || defined(CONFIG_ARM64_GCS ) || \ defined(CONFIG_RISCV_USER_CFI )#define VM_SHADOW_STACK INIT_VM_FLAG(SHADOW_STACK) +#define VMA_SHADOW_STACK mk_vma_flags(VMA_SHADOW_STACK_BIT)#define VMA_STARTGAP_FLAGS mk_vma_flags(VMA_GROWSDOWN_BIT, VMA_SHADOW_STACK_BIT)#else #define VM_SHADOW_STACK VM_NONE +#define VMA_SHADOW_STACK EMPTY_VMA_FLAGS#define VMA_STARTGAP_FLAGS mk_vma_flags(VMA_GROWSDOWN_BIT)#endif #if defined(CONFIG_PPC64 )
@@ -4167,9 +4169,9 @@ get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,return __get_unmapped_area (file, addr, len, pgoff, flags,0 ); } -extern unsigned long do_mmap (struct file *file,unsigned long addr, +unsigned long do_mmap (struct file *file,unsigned long addr,unsigned long len,unsigned long prot,unsigned long flags, -vm_flags_t vm_flags,unsigned long pgoff,unsigned long *populate, +vma_flags_t vma_flags,unsigned long pgoff,unsigned long *populate,struct list_head *uf); extern int do_vmi_munmap(struct vma_iterator *vmi, struct mm_struct *mm, unsigned long start, size_t len, struct list_head *uf,
ipc/shm.c +2 -1
@@ -1661,7 +1661,8 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, goto invalid; } - addr =do_mmap (file, addr, size, prot, flags,0 ,0 , &populate,NULL ); + addr =do_mmap (file, addr, size, prot, flags,EMPTY_VMA_FLAGS ,0 , + &populate,NULL ); *raddr = addr; err =0 ;if (IS_ERR_VALUE (addr))
mm/memfd.c +6 -9
@@ -369,39 +369,36 @@ static inline bool is_write_sealed(unsigned int seals)return seals & (F_SEAL_WRITE |F_SEAL_FUTURE_WRITE ); } -static int check_write_seal (vm_flags_t *vm_flags_ptr) +static int check_write_seal (vma_flags_t *vma_flags_ptr) { -vm_flags_t vm_flags = *vm_flags_ptr; -vm_flags_t mask = vm_flags & (VM_SHARED |VM_WRITE ); -/* If a private mapping then writability is irrelevant. */ -if (!(mask &VM_SHARED )) +if (!vma_flags_test (vma_flags_ptr,VMA_SHARED_BIT ))return 0 ;/* * New PROT_WRITE and MAP_SHARED mmaps are not allowed when * write seals are active. */ -if (mask &VM_WRITE ) +if (vma_flags_test (vma_flags_ptr,VMA_WRITE_BIT ))return -EPERM ;/* * This is a read-only mapping, disallow mprotect() from making a * write-sealed mapping writable in future. */ - *vm_flags_ptr &= ~VM_MAYWRITE ; +vma_flags_clear (vma_flags_ptr,VMA_MAYWRITE_BIT );return 0 ; } -int memfd_check_seals_mmap (struct file *file,vm_flags_t *vm_flags_ptr) +int memfd_check_seals_mmap (struct file *file,vma_flags_t *vma_flags_ptr) {int err =0 ;unsigned int *seals_ptr =memfd_file_seals_ptr (file);unsigned int seals = seals_ptr ? *seals_ptr :0 ;if (is_write_sealed (seals)) - err =check_write_seal (vm_flags_ptr); + err =check_write_seal (vma_flags_ptr);return err; }
mm/mmap.c +40 -27
172 lines not on this page · raw message
mm/nommu.c +2 -1
@@ -1014,11 +1014,12 @@ unsigned long do_mmap(struct file *file,unsigned long len, unsigned long prot, unsigned long flags, - vm_flags_t vm_flags, + vma_flags_t vma_flags, unsigned long pgoff, unsigned long *populate, struct list_head *uf) { + vm_flags_t vm_flags = vma_flags_to_legacy(vma_flags);struct vm_area_struct *vma;struct vm_region *region;struct rb_node *rb;
mm/util.c +5 -5
@@ -578,8 +578,8 @@ unsigned long vm_mmap_pgoff(struct file *file, unsigned long addr,if (!ret) {if (mmap_write_lock_killable (mm))return -EINTR ; - ret =do_mmap (file, addr, len, prot, flag,0 , pgoff, &populate, - &uf); + ret =do_mmap (file, addr, len, prot, flag,EMPTY_VMA_FLAGS , pgoff, + &populate, &uf);mmap_write_unlock (mm);userfaultfd_unmap_complete (mm, &uf);if (populate)
@@ -627,20 +627,20 @@ EXPORT_SYMBOL(vm_mmap);unsigned long vm_mmap_shadow_stack (unsigned long addr,unsigned long len,unsigned long flags) { +vma_flags_t vma_flags =VMA_SHADOW_STACK ;struct mm_struct *mm = current->mm;unsigned long ret, unused; -vm_flags_t vm_flags =VM_SHADOW_STACK ; flags |=MAP_ANONYMOUS |MAP_PRIVATE ;if (addr) flags |=MAP_FIXED_NOREPLACE ;if (IS_ENABLED (CONFIG_TRANSPARENT_HUGEPAGE )) - vm_flags |=VM_NOHUGEPAGE ; +vma_flags_set (&vma_flags,VMA_NOHUGEPAGE_BIT );mmap_write_lock (mm); ret =do_mmap (NULL , addr, len,PROT_READ |PROT_WRITE , flags, - vm_flags,0 , &unused,NULL ); + vma_flags,0 , &unused,NULL );mmap_write_unlock (mm);return ret;
mm/vma.c +3 -4
@@ -2881,7 +2881,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,* file to be mapped, otherwise NULL. * @addr: The page-aligned address at which to perform the mapping. * @len: The page-aligned, non-zero, length of the mapping. -* @vm_flags: The VMA flags which should be applied to the mapping. +* @vma_flags: The VMA flags which should be applied to the mapping. * @pgoff: If @file is specified, the page offset into the file, if not then * the virtual page offset in memory of the anonymous mapping. * @uf: Optionally, a pointer to a list head used for tracking userfaultfd unmap
@@ -2891,12 +2891,11 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,* been performed. */ unsigned long mmap_region (struct file *file,unsigned long addr, -unsigned long len,vm_flags_t vm_flags, +unsigned long len,vma_flags_t vma_flags,unsigned long pgoff,struct list_head *uf) {unsigned long ret;bool writable_file_mapping = false; -const vma_flags_t vma_flags =legacy_to_vma_flags (vm_flags);mmap_assert_write_locked (current->mm);
@@ -2905,7 +2904,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,return -EACCES ;/* Allow architectures to sanity-check the vm_flags. */ -if (!arch_validate_flags (vm_flags)) +if (!arch_validate_flags (vma_flags_to_legacy (vma_flags)))return -EINVAL ;/* Map writable and ensure this isn't a sealed memfd. */
mm/vma.h +1 -1
@@ -498,7 +498,7 @@ int mm_take_all_locks(struct mm_struct *mm);void mm_drop_all_locks (struct mm_struct *mm);unsigned long mmap_region (struct file *file,unsigned long addr, -unsigned long len,vm_flags_t vm_flags,unsigned long pgoff, +unsigned long len,vma_flags_t vma_flags,unsigned long pgoff,struct list_head *uf); int do_brk_flags(struct vma_iterator *vmi, struct vm_area_struct *brkvma,
Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerPATCH 3/13 mm: convert __get_unmapped_area() to use vma_flags_t
In reply to Lorenzo StoakesUpdate __get_unmapped_area() to be parameterised by vma_flags_t rather than vm_flags_t as part of the effort to move VMA flags from a system word to a bitmap.
We cascade the changes up to arch_get_unmapped_area_topdown() and arch_get_unmapped_area(), where, for now, we use vma_flags_to_legacy() in order to propagate the VMA flags.
No functional change intended.
--- fs/hugetlbfs/inode.c | 3 ++- include/linux/huge_mm.h | 10 +++++----- include/linux/mm.h | 6 ++++-- include/linux/sched/mm.h | 12 ++++++------ mm/huge_memory.c | 21 ++++++++++++--------- mm/mmap.c | 27 ++++++++++++++------------- 6 files changed, 43 insertions(+), 36 deletions(-)
fs/hugetlbfs/inode.c +2 -1
@@ -184,7 +184,8 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,if (addr) addr0 =ALIGN (addr,huge_page_size (h)); -return mm_get_unmapped_area_vmflags (file, addr0, len, pgoff, flags,0 ); +return mm_get_unmapped_area_vmaflags (file, addr0, len, pgoff, flags, +EMPTY_VMA_FLAGS ); }/*
include/linux/huge_mm.h +5 -5
@@ -391,9 +391,9 @@ static inline bool thp_disabled_by_hw(void)unsigned long thp_get_unmapped_area (struct file *filp,unsigned long addr,unsigned long len,unsigned long pgoff,unsigned long flags); -unsigned long thp_get_unmapped_area_vmflags (struct file *filp,unsigned long addr, +unsigned long thp_get_unmapped_area_vmaflags (struct file *filp,unsigned long addr,unsigned long len,unsigned long pgoff,unsigned long flags, -vm_flags_t vm_flags); +vma_flags_t vma_flags);enum split_type {SPLIT_TYPE_UNIFORM ,
@@ -615,9 +615,9 @@ static inline unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma,#define thp_get_unmapped_area NULLstatic inline unsigned long -thp_get_unmapped_area_vmflags (struct file *filp,unsigned long addr, -unsigned long len,unsigned long pgoff, -unsigned long flags,vm_flags_t vm_flags) +thp_get_unmapped_area_vmaflags (struct file *filp,unsigned long addr, +unsigned long len,unsigned long pgoff, +unsigned long flags,vma_flags_t vma_flags) {return 0 ; }
include/linux/mm.h +4 -2
@@ -4160,13 +4160,15 @@ unsigned long randomize_page(unsigned long start, unsigned long range);unsigned long __get_unmapped_area (struct file *file,unsigned long addr,unsigned long len, -unsigned long pgoff,unsigned long flags,vm_flags_t vm_flags); +unsigned long pgoff,unsigned long flags, +vma_flags_t vma_flags);static inline unsigned long get_unmapped_area (struct file *file,unsigned long addr,unsigned long len,unsigned long pgoff,unsigned long flags) { -return __get_unmapped_area (file, addr, len, pgoff, flags,0 ); +return __get_unmapped_area (file, addr, len, pgoff, flags, +EMPTY_VMA_FLAGS ); } unsigned long do_mmap(struct file *file, unsigned long addr,
include/linux/sched/mm.h +6 -6
@@ -193,12 +193,12 @@ unsigned long mm_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags); -unsigned long mm_get_unmapped_area_vmflags (struct file *filp, -unsigned long addr, -unsigned long len, -unsigned long pgoff, -unsigned long flags, -vm_flags_t vm_flags); +unsigned long mm_get_unmapped_area_vmaflags (struct file *filp, +unsigned long addr, +unsigned long len, +unsigned long pgoff, +unsigned long flags, +vma_flags_t vma_flags); unsigned long generic_get_unmapped_area(struct file *filp, unsigned long addr,
mm/huge_memory.c +12 -9
@@ -1196,7 +1196,7 @@ static inline bool is_transparent_hugepage(const struct folio *folio)static unsigned long __thp_get_unmapped_area (struct file *filp,unsigned long addr,unsigned long len,loff_t off,unsigned long flags,unsigned long size, -vm_flags_t vm_flags) +vma_flags_t vma_flags) {loff_t off_end = off + len;loff_t off_align =round_up (off, size);
@@ -1212,8 +1212,9 @@ static unsigned long __thp_get_unmapped_area(struct file *filp,if (len_pad < len || (off + len_pad) < off)return 0 ; - ret =mm_get_unmapped_area_vmflags (filp, addr, len_pad, - off >>PAGE_SHIFT , flags, vm_flags); + ret =mm_get_unmapped_area_vmaflags (filp, addr, len_pad, + off >>PAGE_SHIFT , flags, + vma_flags);/* * The failure might be due to length padding. The caller will retry
@@ -1238,25 +1239,27 @@ static unsigned long __thp_get_unmapped_area(struct file *filp,return ret; } -unsigned long thp_get_unmapped_area_vmflags (struct file *filp,unsigned long addr, +unsigned long thp_get_unmapped_area_vmaflags (struct file *filp,unsigned long addr,unsigned long len,unsigned long pgoff,unsigned long flags, -vm_flags_t vm_flags) +vma_flags_t vma_flags) {unsigned long ret;loff_t off = (loff_t )pgoff <<PAGE_SHIFT ; - ret =__thp_get_unmapped_area (filp, addr, len, off, flags,PMD_SIZE , vm_flags); + ret =__thp_get_unmapped_area (filp, addr, len, off, flags,PMD_SIZE , + vma_flags);if (ret)return ret; -return mm_get_unmapped_area_vmflags (filp, addr, len, pgoff, flags, - vm_flags); +return mm_get_unmapped_area_vmaflags (filp, addr, len, pgoff, flags, + vma_flags); }unsigned long thp_get_unmapped_area (struct file *filp,unsigned long addr,unsigned long len,unsigned long pgoff,unsigned long flags) { -return thp_get_unmapped_area_vmflags (filp, addr, len, pgoff, flags,0 ); +return thp_get_unmapped_area_vmaflags (filp, addr, len, pgoff, flags, +EMPTY_VMA_FLAGS ); }EXPORT_SYMBOL_GPL (thp_get_unmapped_area);
mm/mmap.c +14 -13
@@ -411,8 +411,7 @@ unsigned long do_mmap(struct file *file, unsigned long addr,/* Obtain the address to map to. we verify (or select) it and ensure * that it represents a valid section of the address space. */ - addr =__get_unmapped_area (file, addr, len, pgoff, flags, -vma_flags_to_legacy (vma_flags)); + addr =__get_unmapped_area (file, addr, len, pgoff, flags, vma_flags);if (IS_ERR_VALUE (addr))return addr;
@@ -812,19 +811,20 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr, }#endif -unsigned long mm_get_unmapped_area_vmflags (struct file *filp,unsigned long addr, -unsigned long len,unsigned long pgoff, -unsigned long flags,vm_flags_t vm_flags) +unsigned long mm_get_unmapped_area_vmaflags (struct file *filp,unsigned long addr, +unsigned long len,unsigned long pgoff, +unsigned long flags,vma_flags_t vma_flags) {if (mm_flags_test (MMF_TOPDOWN , current->mm))return arch_get_unmapped_area_topdown (filp, addr, len, pgoff, - flags, vm_flags); -return arch_get_unmapped_area (filp, addr, len, pgoff, flags, vm_flags); + flags,vma_flags_to_legacy (vma_flags)); +return arch_get_unmapped_area (filp, addr, len, pgoff, flags, +vma_flags_to_legacy (vma_flags)); }unsigned long __get_unmapped_area (struct file *file,unsigned long addr,unsigned long len, -unsigned long pgoff,unsigned long flags,vm_flags_t vm_flags) +unsigned long pgoff,unsigned long flags,vma_flags_t vma_flags) {unsigned long (*get_area)(struct file *,unsigned long ,unsigned long ,unsigned long ,unsigned long )
@@ -859,11 +859,11 @@ __get_unmapped_area(struct file *file, unsigned long addr, unsigned long len, && !addr /* no hint */ && IS_ALIGNED(len, PMD_SIZE)) { /* Ensures that larger anonymous mappings are THP aligned. */ - addr = thp_get_unmapped_area_vmflags(file, addr, len, - pgoff, flags, vm_flags); + addr = thp_get_unmapped_area_vmaflags(file, addr, len, + pgoff, flags, vma_flags); } else { - addr =mm_get_unmapped_area_vmflags (file, addr, len, - pgoff, flags, vm_flags); + addr =mm_get_unmapped_area_vmaflags (file, addr, len, + pgoff, flags, vma_flags); }if (IS_ERR_VALUE (addr))return addr;
@@ -881,7 +881,8 @@ unsigned long mm_get_unmapped_area(struct file *file, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags) { - return mm_get_unmapped_area_vmflags(file, addr, len, pgoff, flags, 0); + return mm_get_unmapped_area_vmaflags(file, addr, len, pgoff, flags, + EMPTY_VMA_FLAGS); }EXPORT_SYMBOL (mm_get_unmapped_area);
Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerPATCH 4/13 mm: update generic_get_unmapped_area[_topdown]() to use vma_flags_t
In reply to Lorenzo StoakesAs part of the changes converting VMA flags from a system word size to a bitmap, extend this change to generic_get_unmapped_area() and generic_get_unmapped_area_topdown(), which also allows us to convert stack_guard_placement() as well.
We retain arch_get_unmapped_area() and arch_get_unmapped_area_topdown() as-is for now, using legacy_to_vma_flags() as necessary to do so.
No functional change intended.
--- arch/powerpc/mm/book3s64/slice.c | 6 ++++-- include/linux/sched/mm.h | 4 ++-- mm/mmap.c | 16 ++++++++-------- 3 files changed, 14 insertions(+), 12 deletions(-)
arch/powerpc/mm/book3s64/slice.c +4 -2
@@ -659,7 +659,8 @@ unsigned long arch_get_unmapped_area(struct file *filp,unsigned int psize;if (radix_enabled ()) -return generic_get_unmapped_area (filp, addr, len, pgoff, flags, vm_flags); +return generic_get_unmapped_area (filp, addr, len, pgoff, flags, +legacy_to_vma_flags (vm_flags));if (filp &&is_file_hugepages (filp)) psize =file_to_psize (filp);
@@ -679,7 +680,8 @@ unsigned long arch_get_unmapped_area_topdown(struct file *filp,unsigned int psize;if (radix_enabled ()) -return generic_get_unmapped_area_topdown (filp, addr0, len, pgoff, flags, vm_flags); +return generic_get_unmapped_area_topdown (filp, addr0, len, + pgoff, flags,legacy_to_vma_flags (vm_flags));if (filp &&is_file_hugepages (filp)) psize =file_to_psize (filp);
include/linux/sched/mm.h +2 -2
@@ -203,11 +203,11 @@ unsigned long mm_get_unmapped_area_vmaflags(struct file *filp,unsigned long generic_get_unmapped_area (struct file *filp,unsigned long addr,unsigned long len,unsigned long pgoff, -unsigned long flags,vm_flags_t vm_flags); +unsigned long flags,vma_flags_t vma_flags);unsigned long generic_get_unmapped_area_topdown (struct file *filp,unsigned long addr,unsigned long len,unsigned long pgoff, -unsigned long flags,vm_flags_t vm_flags); +unsigned long flags,vma_flags_t vma_flags);#else static inline void arch_pick_mmap_layout (struct mm_struct *mm,const struct rlimit *rlim_stack) {}
mm/mmap.c +8 -8
@@ -657,9 +657,9 @@ SYSCALL_DEFINE1(old_mmap, struct mmap_arg_struct __user *, arg)* Determine if the allocation needs to ensure that there is no * existing mapping within it's guard gaps, for use as start_gap. */ -static inline unsigned long stack_guard_placement (vm_flags_t vm_flags) +static inline unsigned long stack_guard_placement (vma_flags_t vma_flags) { -if (vm_flags &VM_SHADOW_STACK ) +if (vma_flags_test_single_mask (&vma_flags,VMA_SHADOW_STACK ))return PAGE_SIZE ;return 0 ;
@@ -701,7 +701,7 @@ unsigned long vm_unmapped_area(struct vm_unmapped_area_info *info)unsigned long generic_get_unmapped_area (struct file *filp,unsigned long addr,unsigned long len,unsigned long pgoff, -unsigned long flags,vm_flags_t vm_flags) +unsigned long flags,vma_flags_t vma_flags) {struct mm_struct *mm = current->mm;struct vm_area_struct *vma, *prev;
@@ -726,7 +726,7 @@ generic_get_unmapped_area(struct file *filp, unsigned long addr, info.length = len; info.low_limit = mm->mmap_base; info.high_limit = mmap_end; - info.start_gap =stack_guard_placement (vm_flags); + info.start_gap =stack_guard_placement (vma_flags);if (filp &&is_file_hugepages (filp)) info.align_mask =huge_page_mask_align (filp);return vm_unmapped_area (&info);
@@ -739,7 +739,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long flags, vm_flags_t vm_flags) {return generic_get_unmapped_area (filp, addr, len, pgoff, flags, - vm_flags); +legacy_to_vma_flags (vm_flags)); }#endif
@@ -750,7 +750,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,unsigned long generic_get_unmapped_area_topdown (struct file *filp,unsigned long addr,unsigned long len,unsigned long pgoff, -unsigned long flags,vm_flags_t vm_flags) +unsigned long flags,vma_flags_t vma_flags) {struct vm_area_struct *vma, *prev;struct mm_struct *mm = current->mm;
@@ -778,7 +778,7 @@ generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr, info.length = len; info.low_limit =PAGE_SIZE ; info.high_limit =arch_get_mmap_base (addr, mm->mmap_base); - info.start_gap =stack_guard_placement (vm_flags); + info.start_gap =stack_guard_placement (vma_flags);if (filp &&is_file_hugepages (filp)) info.align_mask =huge_page_mask_align (filp); addr =vm_unmapped_area (&info);
@@ -807,7 +807,7 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr, unsigned long flags, vm_flags_t vm_flags) {return generic_get_unmapped_area_topdown (filp, addr, len, pgoff, flags, - vm_flags); +legacy_to_vma_flags (vm_flags)); }#endif
Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerPATCH 5/13 mm: prefer mm->def_vma_flags in mm logic
In reply to Lorenzo StoakesCurrently mm->def_flags (of type vm_flags_t) is union'd with mm->def_vma_flags (of type vma_flags_t).
As part of the effort to convert vm_flags_t usage to vma_flags_t (in order to no longer be arbitrarily limited to a system word size for VMA flags), prefer mm->def_vma_flags to mm->def_flags throughout the mm logic.
No functional change intended.
--- mm/debug.c | 2 +- mm/mlock.c | 13 +++++++------ mm/mmap.c | 11 ++++++----- mm/vma.c | 4 ++-- 4 files changed, 16 insertions(+), 14 deletions(-)
mm/debug.c +1 -1
@@ -226,7 +226,7 @@ void dump_mm(const struct mm_struct *mm) mm->numa_next_scan, mm->numa_scan_offset, mm->numa_scan_seq, #endifatomic_read (&mm->tlb_flush_pending), - mm->def_flags, &mm->def_flags +vma_flags_to_legacy (mm->def_vma_flags), &mm->def_vma_flags ); }EXPORT_SYMBOL (dump_mm);
mm/mlock.c +7 -6
@@ -699,26 +699,27 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)/* * Take the MCL_* flags passed into mlockall (or 0 if called from munlockall) -* and translate into the appropriate modifications to mm->def_flags and/or the -* flags for all current VMAs. +* and translate into the appropriate modifications to mm->def_vma_flags and/or +* the flags for all current VMAs. * * There are a couple of subtleties with this. If mlockall() is called multiple * times with different flags, the values do not necessarily stack. If mlockall * is called once including the MCL_FUTURE flag and then a second time without -* it, VM_LOCKED and VM_LOCKONFAULT will be cleared from mm->def_flags. +* it, VM_LOCKED and VM_LOCKONFAULT will be cleared from mm->def_vma_flags. */ static int apply_mlockall_flags (int flags) {VMA_ITERATOR (vmi, current->mm,0 ); +struct mm_struct *mm = current->mm;struct vm_area_struct *vma, *prev =NULL ;vm_flags_t to_add =0 ; - current->mm->def_flags &= ~VM_LOCKED_MASK ; +vma_flags_clear_mask (&mm->def_vma_flags,VMA_LOCKED_MASK );if (flags &MCL_FUTURE ) { - current->mm->def_flags |=VM_LOCKED ; +vma_flags_set (&mm->def_vma_flags,VMA_LOCKED_BIT );if (flags &MCL_ONFAULT ) - current->mm->def_flags |=VM_LOCKONFAULT ; +vma_flags_set (&mm->def_vma_flags,VMA_LOCKONFAULT_BIT );if (!(flags &MCL_CURRENT )) goto out;
mm/mmap.c +6 -5
@@ -102,15 +102,16 @@ void vma_set_page_prot(struct vm_area_struct *vma)*/ static int check_brk_limits (unsigned long addr,unsigned long len) { +const struct mm_struct *mm = current->mm; +const bool is_def_locked = +vma_flags_test (&mm->def_vma_flags,VMA_LOCKED_BIT );unsigned long mapped_addr; mapped_addr =get_unmapped_area (NULL , addr, len,0 ,MAP_FIXED );if (IS_ERR_VALUE (mapped_addr))return mapped_addr; -return mlock_future_ok (current->mm, - current->mm->def_flags &VM_LOCKED , len) - ?0 : -EAGAIN ; +return mlock_future_ok (mm, is_def_locked, len) ?0 : -EAGAIN ; } SYSCALL_DEFINE1(brk, unsigned long, brk)
@@ -197,7 +198,7 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) goto out; mm->brk = brk; -if (mm->def_flags &VM_LOCKED ) +if (vma_flags_test (&mm->def_vma_flags,VMA_LOCKED_BIT )) populate = true; success:
@@ -1247,7 +1248,7 @@ int vm_brk_flags(unsigned long addr, unsigned long request, bool is_exec) vma =vma_prev (&vmi); ret =do_brk_flags (&vmi, vma, addr, len, vma_flags); - populate = ((mm->def_flags &VM_LOCKED ) !=0 ); + populate =vma_flags_test (&mm->def_vma_flags,VMA_LOCKED_BIT );mmap_write_unlock (mm);userfaultfd_unmap_complete (mm, &uf);if (populate && !ret)
mm/vma.c +2 -2
@@ -3424,7 +3424,8 @@ struct vm_area_struct *__install_special_mapping(if (unlikely (vma ==NULL ))return ERR_PTR (-ENOMEM ); - vm_flags |= mm->def_flags |VM_DONTEXPAND ; +vma_set_range (vma, addr, addr + len,0 ); + vm_flags |=vma_flags_to_legacy (mm->def_vma_flags) |VM_DONTEXPAND ;if (pgtable_supports_soft_dirty ()) vm_flags |=VM_SOFTDIRTY ;vm_flags_init (vma, vm_flags & ~VM_LOCKED_MASK );
@@ -3432,7 +3433,6 @@ struct vm_area_struct *__install_special_mapping( vma->vm_ops = ops; vma->vm_private_data = priv; -vma_set_range (vma, addr, addr + len,0 ); ret =insert_vm_struct (mm, vma);if (ret)
Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerPATCH 6/13 mm/vma: convert vm_pgprot_modify() to use vma_flags_t and rename
In reply to Lorenzo StoakesUpdate vm_pgprot_modify() to use the new VMA flags type vma_flags_t, and rename to vma_pgprot_modify() accordingly.
This is part of the ongoing work to convert vm_flags_t to vma_flags_t, in order to eliminate the arbitrary limit of the number of bits in a system word on available VMA flags.
Update VMA userland tests accordingly, updating vma_set_page_prot() to no longer inline vma_pgprot_modify(), rather we can simply define vma_pgprot_modify() as a static inline function and the tests will pick it up from vma.h.
No functional change intended.
--- mm/mmap.c | 8 ++++---- mm/vma.c | 2 +- mm/vma.h | 6 ++++-- tools/testing/vma/include/dup.h | 12 +++++------- 4 files changed, 14 insertions(+), 14 deletions(-)
mm/mmap.c +4 -4
@@ -80,13 +80,13 @@ core_param(ignore_rlimit_data, ignore_rlimit_data, bool, 0644);/* Update vma->vm_page_prot to reflect vma->vm_flags. */ void vma_set_page_prot (struct vm_area_struct *vma) { -vm_flags_t vm_flags = vma->vm_flags; +vma_flags_t vma_flags = vma->flags;pgprot_t vm_page_prot; - vm_page_prot =vm_pgprot_modify (vma->vm_page_prot, vm_flags); + vm_page_prot =vma_pgprot_modify (vma->vm_page_prot, vma_flags);if (vma_wants_writenotify (vma, vm_page_prot)) { - vm_flags &= ~VM_SHARED ; - vm_page_prot =vm_pgprot_modify (vm_page_prot, vm_flags); +vma_flags_clear (&vma_flags,VMA_SHARED_BIT ); + vm_page_prot =vma_pgprot_modify (vm_page_prot, vma_flags); }/* remove_protection_ptes reads vma->vm_page_prot without mmap_lock */ WRITE_ONCE (vma->vm_page_prot, vm_page_prot);
mm/vma.c +1 -1
@@ -2179,7 +2179,7 @@ bool vma_wants_writenotify(struct vm_area_struct *vma, pgprot_t vm_page_prot)/* The open routine did something to the protections that pgprot_modify * won't preserve? */ if (pgprot_val (vm_page_prot) != -pgprot_val (vm_pgprot_modify (vm_page_prot, vma->vm_flags))) +pgprot_val (vma_pgprot_modify (vm_page_prot, vma->flags)))return false;/*
mm/vma.h +4 -2
@@ -522,9 +522,11 @@ static inline bool vma_wants_manual_pte_write_upgrade(struct vm_area_struct *vma }#ifdef CONFIG_MMU -static inline pgprot_t vm_pgprot_modify (pgprot_t oldprot,vm_flags_t vm_flags) +static inline pgprot_t vma_pgprot_modify (pgprot_t oldprot,vma_flags_t vma_flags) { -return pgprot_modify (oldprot,vm_get_page_prot (vm_flags)); +const pgprot_t prot =vma_get_page_prot (vma_flags); + +return pgprot_modify (oldprot, prot); }#endif
tools/testing/vma/include/dup.h +5 -7
@@ -1434,20 +1434,18 @@ struct vm_area_struct *vma_iter_next_range(struct vma_iterator *vmi) }bool vma_wants_writenotify (struct vm_area_struct *vma,pgprot_t vm_page_prot); +static inline pgprot_t vma_pgprot_modify (pgprot_t oldprot,vma_flags_t vma_flags);/* Update vma->vm_page_prot to reflect vma->vm_flags. */ static inline void vma_set_page_prot (struct vm_area_struct *vma) { -vm_flags_t vm_flags = vma->vm_flags; +vma_flags_t vma_flags = vma->flags;pgprot_t vm_page_prot; -/* testing: we inline vm_pgprot_modify() to avoid clash with vma.h. */ - vm_page_prot =pgprot_modify (vma->vm_page_prot,vm_get_page_prot (vm_flags)); - + vm_page_prot =vma_pgprot_modify (vma->vm_page_prot, vma_flags);if (vma_wants_writenotify (vma, vm_page_prot)) { - vm_flags &= ~VM_SHARED ; -/* testing: we inline vm_pgprot_modify() to avoid clash with vma.h. */ - vm_page_prot =pgprot_modify (vm_page_prot,vm_get_page_prot (vm_flags)); +vma_flags_clear (&vma_flags,VMA_SHARED_BIT ); + vm_page_prot =vma_pgprot_modify (vm_page_prot, vma_flags); }/* remove_protection_ptes reads vma->vm_page_prot without mmap_lock */ WRITE_ONCE (vma->vm_page_prot, vm_page_prot);
Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerPATCH 7/13 mm/vma: rename vma_get_page_prot to vma_flags_to_page_prot
In reply to Lorenzo StoakesHaving vma_get_page_prot() refer to VMA flags and vma_set_page_prot() refer to a VMA is confusing.
Rename vma_get_page_prot() to vma_flags_to_page_prot() to resolve this confusion.
No functional change intended.
--- include/linux/mm.h | 4 ++-- mm/vma.c | 2 +- mm/vma.h | 2 +- tools/testing/vma/include/dup.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-)
include/linux/mm.h +2 -2
@@ -4568,7 +4568,7 @@ static inline bool range_in_vma_desc(const struct vm_area_desc *desc,#ifdef CONFIG_MMU pgprot_t vm_get_page_prot (vm_flags_t vm_flags); -static inline pgprot_t vma_get_page_prot (vma_flags_t vma_flags) +static inline pgprot_t vma_flags_to_page_prot (vma_flags_t vma_flags) {const vm_flags_t vm_flags =vma_flags_to_legacy (vma_flags);
@@ -4581,7 +4581,7 @@ static inline pgprot_t vm_get_page_prot(vm_flags_t vm_flags) {return __pgprot (0 ); } -static inline pgprot_t vma_get_page_prot (vma_flags_t vma_flags) +static inline pgprot_t vma_flags_to_page_prot (vma_flags_t vma_flags) {return __pgprot (0 ); }
mm/vma.c +1 -1
@@ -56,7 +56,7 @@ struct mmap_state { .pglen = PHYS_PFN(len_), \ .vma_flags = vma_flags_, \ .file = file_, \ - .page_prot = vma_get_page_prot(vma_flags_), \ + .page_prot = vma_flags_to_page_prot(vma_flags_), \ }#define VMG_MMAP_STATE (name, map_, vma_) \
mm/vma.h +1 -1
@@ -524,7 +524,7 @@ static inline bool vma_wants_manual_pte_write_upgrade(struct vm_area_struct *vma#ifdef CONFIG_MMU static inline pgprot_t vma_pgprot_modify (pgprot_t oldprot,vma_flags_t vma_flags) { -const pgprot_t prot =vma_get_page_prot (vma_flags); +const pgprot_t prot =vma_flags_to_page_prot (vma_flags);return pgprot_modify (oldprot, prot); }
tools/testing/vma/include/dup.h +1 -1
@@ -1555,7 +1555,7 @@ static inline int get_sysctl_max_map_count(void)#define pgtable_supports_soft_dirty () IS_ENABLED(CONFIG_MEM_SOFT_DIRTY)#endif -static inline pgprot_t vma_get_page_prot (vma_flags_t vma_flags) +static inline pgprot_t vma_flags_to_page_prot (vma_flags_t vma_flags) {const vm_flags_t vm_flags =vma_flags_to_legacy (vma_flags);
Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerPATCH 8/13 mm: introduce vma_get_page_prot() and use it
In reply to Lorenzo StoakesThere's a large number of vm_get_page_prot(vma->vm_flags) invocations. Make life easier by introducing vma_get_page_prot() parameterised by the VMA.
This also makes converting vm_get_page_prot() to vma_flags_t easier.
Also update the userland VMA tests to reflect the change.
No functional change intended.
--- drivers/gpu/drm/drm_gem.c | 2 +- drivers/gpu/drm/drm_gem_dma_helper.c | 2 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_gem.c | 6 +++--- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 12 ++++++------ drivers/gpu/drm/msm/msm_gem.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +- drivers/gpu/drm/omapdrm/omap_gem.c | 6 +++--- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 2 +- drivers/gpu/drm/tegra/gem.c | 2 +- drivers/gpu/drm/virtio/virtgpu_vram.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 2 +- drivers/gpu/drm/xe/xe_device.c | 2 +- drivers/gpu/drm/xe/xe_mmio_gem.c | 2 +- drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +- drivers/video/fbdev/core/fb_io_fops.c | 2 +- include/linux/mm.h | 10 +++++++++- mm/vma.c | 2 +- mm/vma_exec.c | 2 +- sound/core/memalloc.c | 2 +- tools/testing/vma/include/dup.h | 4 ++++ 23 files changed, 43 insertions(+), 31 deletions(-)
drivers/gpu/drm/drm_gem.c +1 -1
@@ -1252,7 +1252,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, }vm_flags_set (vma,VM_IO |VM_PFNMAP |VM_DONTEXPAND |VM_DONTDUMP ); - vma->vm_page_prot =pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_writecombine (vma_get_page_prot (vma)); vma->vm_page_prot =pgprot_decrypted (vma->vm_page_prot); }
drivers/gpu/drm/drm_gem_dma_helper.c +1 -1
@@ -540,7 +540,7 @@ int drm_gem_dma_mmap(struct drm_gem_dma_object *dma_obj, struct vm_area_struct *vm_flags_mod (vma,VM_DONTDUMP |VM_DONTEXPAND ,VM_PFNMAP );if (dma_obj->map_noncoherent) { - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); ret = dma_mmap_pages(drm_dev_dma_dev(dma_obj->base.dev), vma, vma->vm_end - vma->vm_start,
drivers/gpu/drm/drm_gem_shmem_helper.c +1 -1
@@ -764,7 +764,7 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_structreturn ret;vm_flags_set (vma,VM_PFNMAP |VM_DONTEXPAND |VM_DONTDUMP ); - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma);if (shmem->map_wc) vma->vm_page_prot =pgprot_writecombine (vma->vm_page_prot);
drivers/gpu/drm/etnaviv/etnaviv_gem.c +1 -1
@@ -133,7 +133,7 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,vm_flags_set (vma,VM_PFNMAP |VM_DONTEXPAND |VM_DONTDUMP ); - vm_page_prot =vm_get_page_prot (vma->vm_flags); + vm_page_prot =vma_get_page_prot (vma);if (etnaviv_obj->flags &ETNA_BO_WC ) { vma->vm_page_prot =pgprot_writecombine (vm_page_prot);
drivers/gpu/drm/exynos/exynos_drm_gem.c +3 -3
@@ -377,13 +377,13 @@ static int exynos_drm_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct/* non-cachable as default. */ if (exynos_gem->flags &EXYNOS_BO_CACHABLE ) - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma);else if (exynos_gem->flags &EXYNOS_BO_WC ) vma->vm_page_prot = -pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); +pgprot_writecombine (vma_get_page_prot (vma));else vma->vm_page_prot = -pgprot_noncached (vm_get_page_prot (vma->vm_flags)); +pgprot_noncached (vma_get_page_prot (vma)); ret =exynos_drm_gem_mmap_buffer (exynos_gem, vma);if (ret)
drivers/gpu/drm/i915/gem/i915_gem_mman.c +6 -6
@@ -112,7 +112,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data, vma =find_vma (mm, addr);if (vma &&__vma_matches (vma, obj->base.filp, addr, args->size)) vma->vm_page_prot = -pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); +pgprot_writecombine (vma_get_page_prot (vma));else addr = -ENOMEM ;mmap_write_unlock (mm);
@@ -1024,7 +1024,7 @@ i915_gem_object_mmap(struct drm_i915_gem_object *obj,fput (anon);if (obj->ops->mmap_ops) { - vma->vm_page_prot =pgprot_decrypted (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_decrypted (vma_get_page_prot (vma)); vma->vm_ops = obj->ops->mmap_ops; vma->vm_private_data = obj->base.vma_node.driver_private;return 0 ;
@@ -1035,7 +1035,7 @@ i915_gem_object_mmap(struct drm_i915_gem_object *obj,switch (mmo->mmap_type) {case I915_MMAP_TYPE_WC : vma->vm_page_prot = -pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); +pgprot_writecombine (vma_get_page_prot (vma)); vma->vm_ops = &vm_ops_cpu;break ;
@@ -1043,19 +1043,19 @@ i915_gem_object_mmap(struct drm_i915_gem_object *obj,GEM_WARN_ON (1 ); fallthrough;case I915_MMAP_TYPE_WB : - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); vma->vm_ops = &vm_ops_cpu;break ;case I915_MMAP_TYPE_UC : vma->vm_page_prot = -pgprot_noncached (vm_get_page_prot (vma->vm_flags)); +pgprot_noncached (vma_get_page_prot (vma)); vma->vm_ops = &vm_ops_cpu;break ;case I915_MMAP_TYPE_GTT : vma->vm_page_prot = -pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); +pgprot_writecombine (vma_get_page_prot (vma)); vma->vm_ops = &vm_ops_gtt;break ; }
drivers/gpu/drm/msm/msm_gem.c +1 -1
@@ -1125,7 +1125,7 @@ static int msm_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_structstruct msm_gem_object *msm_obj =to_msm_bo (obj);vm_flags_set (vma,VM_PFNMAP |VM_DONTEXPAND |VM_DONTDUMP ); - vma->vm_page_prot =msm_gem_pgprot (msm_obj,vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =msm_gem_pgprot (msm_obj,vma_get_page_prot (vma));return 0 ; }
drivers/gpu/drm/nouveau/nouveau_gem.c +1 -1
@@ -55,7 +55,7 @@ static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf) goto error_unlock;nouveau_bo_del_io_reserve_lru (bo); - prot =vm_get_page_prot (vma->vm_flags); + prot =vma_get_page_prot (vma); ret =ttm_bo_vm_fault_reserved (vmf, prot,TTM_BO_VM_NUM_PREFAULT );nouveau_bo_add_io_reserve_lru (bo);if (ret ==VM_FAULT_RETRY && !(vmf->flags &FAULT_FLAG_RETRY_NOWAIT ))
drivers/gpu/drm/omapdrm/omap_fbdev.c +1 -1
@@ -84,7 +84,7 @@ static int omap_fbdev_pan_display(struct fb_var_screeninfo *var, struct fb_infostatic int omap_fbdev_fb_mmap (struct fb_info *info,struct vm_area_struct *vma) { - vma->vm_page_prot =pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_writecombine (vma_get_page_prot (vma));return fb_deferred_io_mmap (info, vma); }
drivers/gpu/drm/omapdrm/omap_gem.c +3 -3
@@ -538,9 +538,9 @@ static int omap_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_strucvm_flags_set (vma,VM_DONTEXPAND |VM_DONTDUMP |VM_IO |VM_MIXEDMAP );if (omap_obj->flags &OMAP_BO_WC ) { - vma->vm_page_prot =pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_writecombine (vma_get_page_prot (vma)); }else if (omap_obj->flags &OMAP_BO_UNCACHED ) { - vma->vm_page_prot =pgprot_noncached (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_noncached (vma_get_page_prot (vma)); }else {/* * We do have some private objects, at least for scanout buffers
@@ -558,7 +558,7 @@ static int omap_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struc vma->vm_pgoff -=drm_vma_node_start (&obj->vma_node);vma_set_file (vma, obj->filp); - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); } vma->vm_page_prot =pgprot_decrypted (vma->vm_page_prot);
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +1 -1
@@ -255,7 +255,7 @@ static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,*/ vm_flags_mod (vma,VM_IO |VM_DONTEXPAND |VM_DONTDUMP ,VM_PFNMAP ); - vma->vm_page_prot =pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_writecombine (vma_get_page_prot (vma)); vma->vm_page_prot =pgprot_decrypted (vma->vm_page_prot);if (rk_obj->pages)
drivers/gpu/drm/tegra/gem.c +1 -1
@@ -602,7 +602,7 @@ int __tegra_gem_mmap(struct drm_gem_object *gem, struct vm_area_struct *vma) vma->vm_pgoff = vm_pgoff; }else { -pgprot_t prot =vm_get_page_prot (vma->vm_flags); +pgprot_t prot =vma_get_page_prot (vma);vm_flags_mod (vma,VM_MIXEDMAP ,VM_PFNMAP );
drivers/gpu/drm/virtio/virtgpu_vram.c +1 -1
@@ -55,7 +55,7 @@ static int virtio_gpu_vram_mmap(struct drm_gem_object *obj, vma->vm_pgoff -=drm_vma_node_start (&obj->vma_node);vm_flags_set (vma,VM_MIXEDMAP |VM_DONTEXPAND ); - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); vma->vm_page_prot =pgprot_decrypted (vma->vm_page_prot); vma->vm_ops = &virtio_gpu_vram_vm_ops;
drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c +1 -1
@@ -481,7 +481,7 @@ vm_fault_t vmw_bo_vm_fault(struct vm_fault *vmf)if (vbo->dirty && vbo->dirty->method ==VMW_BO_DIRTY_MKWRITE ) prot =vm_get_page_prot (vma->vm_flags & ~VM_SHARED );else - prot =vm_get_page_prot (vma->vm_flags); + prot =vma_get_page_prot (vma); ret =ttm_bo_vm_fault_reserved (vmf, prot, num_prefault);if (ret ==VM_FAULT_RETRY && !(vmf->flags &FAULT_FLAG_RETRY_NOWAIT ))
drivers/gpu/drm/xe/xe_device.c +1 -1
@@ -281,7 +281,7 @@ static vm_fault_t barrier_fault(struct vm_fault *vmf)pgprot_t prot;int idx; - prot =vm_get_page_prot (vma->vm_flags); + prot =vma_get_page_prot (vma);if (drm_dev_enter (dev, &idx)) {unsigned long pfn;
drivers/gpu/drm/xe/xe_mmio_gem.c +1 -1
@@ -149,7 +149,7 @@ static int xe_mmio_gem_mmap(struct drm_gem_object *base, struct vm_area_struct */* Set vm_pgoff (used as a fake buffer offset by DRM) to 0 */ vma->vm_pgoff =0 ; - vma->vm_page_prot =pgprot_noncached (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_noncached (vma_get_page_prot (vma));vm_flags_set (vma,VM_IO |VM_PFNMAP |VM_DONTEXPAND |VM_DONTDUMP |VM_DONTCOPY |VM_NORESERVE );
drivers/gpu/drm/xen/xen_drm_front_gem.c +1 -1
@@ -80,7 +80,7 @@ static int xen_drm_front_gem_object_mmap(struct drm_gem_object *gem_obj,* which is mapped as Normal Inner Write-Back Outer Write-Back * Inner-Shareable. */ - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma);/* * vm_operations_struct.fault handler will be called if CPU access
drivers/video/fbdev/core/fb_io_fops.c +1 -1
@@ -161,7 +161,7 @@ int fb_io_mmap(struct fb_info *info, struct vm_area_struct *vma) len = info->fix.mmio_len; } - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); vma->vm_page_prot =pgprot_framebuffer (vma->vm_page_prot, vma->vm_start, vma->vm_end, start);
include/linux/mm.h +9 -1
@@ -4575,6 +4575,10 @@ static inline pgprot_t vma_flags_to_page_prot(vma_flags_t vma_flags) return vm_get_page_prot(vm_flags); } +static inline pgprot_t vma_get_page_prot (const struct vm_area_struct *vma) +{ +return vma_flags_to_page_prot (vma->flags); +}void vma_set_page_prot (struct vm_area_struct *vma);#else static inline pgprot_t vm_get_page_prot (vm_flags_t vm_flags)
@@ -4585,9 +4589,13 @@ static inline pgprot_t vma_flags_to_page_prot(vma_flags_t vma_flags) {return __pgprot (0 ); } +static inline pgprot_t vma_get_page_prot (const struct vm_area_struct *vma) +{ +return __pgprot (0 ); +}static inline void vma_set_page_prot (struct vm_area_struct *vma) { - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); }#endif
mm/vma.c +1 -1
@@ -3429,7 +3429,7 @@ struct vm_area_struct *__install_special_mapping(if (pgtable_supports_soft_dirty ()) vm_flags |=VM_SOFTDIRTY ;vm_flags_init (vma, vm_flags & ~VM_LOCKED_MASK ); - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); vma->vm_ops = ops; vma->vm_private_data = priv;
mm/vma_exec.c +1 -1
@@ -146,7 +146,7 @@ int create_init_stack_vma(struct mm_struct *mm, struct vm_area_struct **vmap,if (pgtable_supports_soft_dirty ()) flags |=VM_SOFTDIRTY ;vm_flags_init (vma, flags); - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); err =insert_vm_struct (mm, vma);if (err)
sound/core/memalloc.c +1 -1
@@ -851,7 +851,7 @@ static void snd_dma_noncoherent_free(struct snd_dma_buffer *dmab)static int snd_dma_noncoherent_mmap (struct snd_dma_buffer *dmab,struct vm_area_struct *area) { - area->vm_page_prot =vm_get_page_prot (area->vm_flags); + area->vm_page_prot =vma_get_page_prot (area);return dma_mmap_pages (dmab->dev.dev, area, area->vm_end - area->vm_start,virt_to_page (dmab->area));
tools/testing/vma/include/dup.h +4 -0
@@ -1577,3 +1577,7 @@ static inline pgoff_t linear_page_index(const struct vm_area_struct *vma, pgoff +=vma_start_pgoff (vma);return pgoff; } +static inline pgprot_t vma_get_page_prot (const struct vm_area_struct *vma) +{ +return vma_flags_to_page_prot (vma->flags); +}
Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerPATCH 9/13 mm/vma: update create_init_stack_vma() to use vma_flags_t
In reply to Lorenzo StoakesReplace use of the legacy vm_flags_t flags with vma_flags_t values in create_init_stack_vma().
As part of this change we add VMA_STACK_EARLY and VMA_STACK_INCOMPLETE vma_flags_t defines, and slightly rework create_init_stack_vma() for clarity.
No functional change intended.
--- include/linux/mm.h | 4 ++++ mm/vma_exec.c | 18 +++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-)
include/linux/mm.h +4 -0
@@ -440,8 +440,10 @@ enum {#define VM_STACK INIT_VM_FLAG(STACK)#ifdef CONFIG_STACK_GROWSUP #define VM_STACK_EARLY INIT_VM_FLAG(STACK_EARLY) +#define VMA_STACK_EARLY mk_vma_flags(VMA_STACK_EARLY_BIT)#else #define VM_STACK_EARLY VM_NONE +#define VMA_STACK_EARLY EMPTY_VMA_FLAGS#endif #ifdef CONFIG_ARCH_HAS_PKEYS #define VM_PKEY_SHIFT ((__force int)VMA_HIGH_ARCH_0_BIT)
@@ -532,6 +534,8 @@ enum {/* Bits set in the VMA until the stack is in its final location */ #define VM_STACK_INCOMPLETE_SETUP (VM_RAND_READ | VM_SEQ_READ | VM_STACK_EARLY) +#define VMA_STACK_INCOMPLETE_SETUP append_vma_flags( \ + VMA_STACK_EARLY, VMA_RAND_READ_BIT, VMA_SEQ_READ_BIT)#define TASK_EXEC_BIT ((current->personality & READ_IMPLIES_EXEC) ? \ VMA_EXEC_BIT : VMA_READ_BIT)
mm/vma_exec.c +11 -7
@@ -112,15 +112,17 @@ int relocate_vma_down(struct vm_area_struct *vma, unsigned long shift)int create_init_stack_vma (struct mm_struct *mm,struct vm_area_struct **vmap,unsigned long *top_mem_p) { -unsigned long flags =VM_STACK_FLAGS |VM_STACK_INCOMPLETE_SETUP ; +vma_flags_t flags =VMA_STACK_INCOMPLETE_SETUP ; +struct vm_area_struct *vma;int err; -struct vm_area_struct *vma =vm_area_alloc (mm); +/* VMA_STACK_FLAGS and VMA_STACK_INCOMPLETE_SETUP must not overlap. */ +VM_WARN_ON_ONCE (vma_flags_test_any_mask (&flags,VMA_STACK_FLAGS )); + + vma =vm_area_alloc (mm);if (!vma)return -ENOMEM ; -vma_set_anonymous (vma); -if (mmap_write_lock_killable (mm)) { err = -EINTR ; goto err_free;
@@ -134,18 +136,20 @@ int create_init_stack_vma(struct mm_struct *mm, struct vm_area_struct **vmap,if (err) goto err_ksm; +vma_flags_set_mask (&flags,VMA_STACK_FLAGS ); +vma_set_anonymous (vma); +/* * Place the stack at the largest stack address the architecture * supports. Later, we'll move this to an appropriate place. We don't * use STACK_TOP because that can depend on attributes which aren't * configured yet. */ -VM_WARN_ON_ONCE (VM_STACK_FLAGS &VM_STACK_INCOMPLETE_SETUP ); vma->vm_end =STACK_TOP_MAX ; vma->vm_start = vma->vm_end -PAGE_SIZE ;if (pgtable_supports_soft_dirty ()) - flags |=VM_SOFTDIRTY ; -vm_flags_init (vma, flags); +vma_flags_set (&flags,VMA_SOFTDIRTY_BIT ); + vma->flags = flags; vma->vm_page_prot =vma_get_page_prot (vma); err =insert_vm_struct (mm, vma);
Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerPATCH 10/13 mm/vma: convert miscellaneous uses of VMA flags in core mm
In reply to Lorenzo StoakesUpdate various uses of legacy flags in vma.c and mmap.c to the new vma_flags_t type, updating comments alongside them to be consistent.
Also update __install_special_mapping() to rearrange things slightly to accommodate the changes.
--- mm/mmap.c | 39 +++++++++++++++++++++------------------ mm/vma.c | 16 ++++++++++------ 2 files changed, 31 insertions(+), 24 deletions(-)
mm/mmap.c +21 -18
@@ -557,8 +557,8 @@ unsigned long do_mmap(struct file *file, unsigned long addr, }/* -* Set 'VM_NORESERVE' if we should not account for the -* memory use of this mapping. +* Set VMA_NORESERVE_BIT if we should not account for the memory use +* of this mapping. */ if (flags &MAP_NORESERVE ) {/* We honor MAP_NORESERVE if allowed to overcommit */
@@ -985,7 +985,7 @@ struct vm_area_struct *find_extend_vma_locked(struct mm_struct *mm, unsigned lonreturn NULL ;if (expand_stack_locked (prev, addr))return NULL ; -if (prev->vm_flags &VM_LOCKED ) +if (vma_test (prev,VMA_LOCKED_BIT ))populate_vma_page_range (prev, addr, prev->vm_end,NULL );return prev; }
@@ -1009,7 +1009,7 @@ struct vm_area_struct *find_extend_vma_locked(struct mm_struct *mm, unsigned lon start = vma->vm_start;if (expand_stack_locked (vma, addr))return NULL ; -if (vma->vm_flags &VM_LOCKED ) +if (vma_test (vma,VMA_LOCKED_BIT ))populate_vma_page_range (vma, addr, start,NULL );return vma; }
@@ -1134,18 +1134,18 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,*/ vma =vma_lookup (mm, start); -if (!vma || !(vma->vm_flags &VM_SHARED )) { +if (!vma || !vma_test (vma,VMA_SHARED_BIT )) {mmap_read_unlock (mm);return -EINVAL ; } - prot |= vma->vm_flags &VM_READ ?PROT_READ :0 ; - prot |= vma->vm_flags &VM_WRITE ?PROT_WRITE :0 ; - prot |= vma->vm_flags &VM_EXEC ?PROT_EXEC :0 ; + prot |=vma_test (vma,VMA_READ_BIT ) ?PROT_READ :0 ; + prot |=vma_test (vma,VMA_WRITE_BIT ) ?PROT_WRITE :0 ; + prot |=vma_test (vma,VMA_EXEC_BIT ) ?PROT_EXEC :0 ; flags &=MAP_NONBLOCK ; flags |=MAP_SHARED |MAP_FIXED |MAP_POPULATE ; -if (vma->vm_flags &VM_LOCKED ) +if (vma_test (vma,VMA_LOCKED_BIT )) flags |=MAP_LOCKED ;/* Save vm_flags used to calculate prot and flags, and recheck later. */
@@ -1271,7 +1271,7 @@ unsigned long tear_down_vmas(struct mm_struct *mm, struct vma_iterator *vmi,mmap_assert_write_locked (mm);vma_iter_set (vmi, vma->vm_end); do { - if (vma->vm_flags & VM_ACCOUNT) + if (vma_test(vma, VMA_ACCOUNT_BIT)) nr_accounted += vma_pages(vma); vma_mark_detached(vma); remove_vma(vma);
@@ -1420,7 +1420,7 @@ static int special_mapping_split(struct vm_area_struct *vma, unsigned long addr) {/* * Forbid splitting special mappings - kernel has expectations over -* the number of pages in mapping. Together with VM_DONTEXPAND +* the number of pages in mapping. Together with VMA_DONTEXPAND_BIT * the size of vma should stay the same over the special mapping's * lifetime. */
@@ -1692,7 +1692,7 @@ bool mmap_read_lock_maybe_expand(struct mm_struct *mm,return true; } -if (!(new_vma->vm_flags &VM_GROWSDOWN )) +if (!vma_test (new_vma,VMA_GROWSDOWN_BIT ))return false;mmap_write_lock (mm);
@@ -1742,7 +1742,7 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm) retval =vma_start_write_killable (mpnt);if (retval <0 ) goto loop_out; -if (mpnt->vm_flags &VM_DONTCOPY ) { +if (vma_test (mpnt,VMA_DONTCOPY_BIT )) { retval =vma_iter_clear_gfp (&vmi, mpnt->vm_start, mpnt->vm_end,GFP_KERNEL );if (retval)
@@ -1752,7 +1752,7 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)continue ; } charge =0 ; -if (mpnt->vm_flags &VM_ACCOUNT ) { +if (vma_test (mpnt,VMA_ACCOUNT_BIT )) {unsigned long len =vma_pages (mpnt);if (security_vm_enough_memory_mm (oldmm, len))/* sic */
@@ -1770,16 +1770,19 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm) retval =dup_userfaultfd (tmp, &uf);if (retval) goto fail_nomem_anon_vma_fork; -if (tmp->vm_flags &VM_WIPEONFORK ) { + +if (vma_test (tmp,VMA_WIPEONFORK_BIT )) {/* -* VM_WIPEONFORK gets a clean slate in the child. +* VMA_WIPEONFORK_BIT gets a clean slate in the child. * Don't prepare anon_vma until fault since we don't * copy page for current vma. */ tmp->anon_vma =NULL ; }else if (anon_vma_fork (tmp, mpnt)) goto fail_nomem_anon_vma_fork; -vm_flags_clear (tmp,VM_LOCKED_MASK ); + +vma_start_write (tmp); +vma_clear_flags_mask (tmp,VMA_LOCKED_MASK );/* * Copy/update hugetlb private vma information. */
@@ -1812,7 +1815,7 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)i_mmap_unlock_write (mapping); } -if (!(tmp->vm_flags &VM_WIPEONFORK )) +if (!vma_test (tmp,VMA_WIPEONFORK_BIT )) retval =copy_page_range (tmp, mpnt);if (retval) {
mm/vma.c +10 -6
@@ -3417,23 +3417,27 @@ struct vm_area_struct *__install_special_mapping( vm_flags_t vm_flags, void *priv, const struct vm_operations_struct *ops) { - int ret; + vma_flags_t vma_flags = legacy_to_vma_flags(vm_flags);struct vm_area_struct *vma; +int ret; vma =vm_area_alloc (mm); -if (unlikely (vma ==NULL )) +if (unlikely (!vma))return ERR_PTR (-ENOMEM ); -vma_set_range (vma, addr, addr + len,0 ); - vm_flags |=vma_flags_to_legacy (mm->def_vma_flags) |VM_DONTEXPAND ; +vma_flags_set_mask (&vma_flags, mm->def_vma_flags); +vma_flags_set (&vma_flags,VMA_DONTEXPAND_BIT );if (pgtable_supports_soft_dirty ()) - vm_flags |=VM_SOFTDIRTY ; -vm_flags_init (vma, vm_flags & ~VM_LOCKED_MASK ); +vma_flags_set (&vma_flags,VMA_SOFTDIRTY_BIT ); +vma_flags_clear_mask (&vma_flags,VMA_LOCKED_MASK ); + vma->flags = vma_flags; vma->vm_page_prot =vma_get_page_prot (vma); vma->vm_ops = ops; vma->vm_private_data = priv; +vma_set_range (vma, addr, addr + len,0 ); + ret =insert_vm_struct (mm, vma);if (ret) goto out;
Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerPATCH 11/13 mm/mlock: convert mlock code to use vma_flags_t
In reply to Lorenzo StoakesReplace use of the legacy vm_flags_t flags with vma_flags_t values throughout the mlock logic.
Additionally update comments to reflect the changes to be consistent.
No functional change intended.
--- mm/mlock.c | 86 +++++++++++++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 40 deletions(-)
mm/mlock.c +46 -40
@@ -329,7 +329,7 @@ static inline bool allow_mlock_munlock(struct folio *folio,* be split. And the pages are not in VM_LOCKed VMA * can be reclaimed. */ -if (!(vma->vm_flags &VM_LOCKED )) +if (!vma_test (vma,VMA_LOCKED_BIT ))return true;/* folio_within_range() cannot take KSM, but any small folio is OK */
@@ -368,7 +368,7 @@ static int mlock_pte_range(pmd_t *pmd, unsigned long addr, folio =pmd_folio (*pmd);if (folio_is_zone_device (folio)) goto out; -if (vma->vm_flags &VM_LOCKED ) +if (vma_test (vma,VMA_LOCKED_BIT ))mlock_folio (folio);else munlock_folio (folio);
@@ -393,7 +393,7 @@ static int mlock_pte_range(pmd_t *pmd, unsigned long addr,if (!allow_mlock_munlock (folio, vma, start, end, step)) goto next_entry; -if (vma->vm_flags &VM_LOCKED ) +if (vma_test (vma,VMA_LOCKED_BIT ))mlock_folio (folio);else munlock_folio (folio);
@@ -417,8 +417,8 @@ static int mlock_pte_range(pmd_t *pmd, unsigned long addr,* @end - end of range in @vma * @new_vma_flags - the new set of flags for @vma. * -* Called for mlock(), mlock2() and mlockall(), to set @vma VM_LOCKED; -* called for munlock() and munlockall(), to clear VM_LOCKED from @vma. +* Called for mlock(), mlock2() and mlockall(), to set @vma VMA_LOCKED_BIT; +* called for munlock() and munlockall(), to clear VMA_LOCKED_BIT from @vma. */ static void mlock_vma_pages_range(struct vm_area_struct *vma, unsigned long start, unsigned long end,
@@ -431,14 +431,14 @@ static void mlock_vma_pages_range(struct vm_area_struct *vma,/* * There is a slight chance that concurrent page migration, -* or page reclaim finding a page of this now-VM_LOCKED vma, +* or page reclaim finding a page of this now-VMA_LOCKED_BIT vma, * will call mlock_vma_folio() and raise page's mlock_count: * double counting, leaving the page unevictable indefinitely. -* Communicate this danger to mlock_vma_folio() with VM_IO, -* which is a VM_SPECIAL flag not allowed on VM_LOCKED vmas. +* Communicate this danger to mlock_vma_folio() with VMA_IO_BIT, +* which is a VMA_SPECIAL_FLAGS flag not allowed on VMA_LOCKED_BIT vmas. * mmap_lock is held in write mode here, so this weird * combination should not be visible to other mmap_lock users; -* but WRITE_ONCE so rmap walkers must see VM_IO if VM_LOCKED. +* but WRITE_ONCE so rmap walkers must see VMA_IO_BIT if VMA_LOCKED_BIT. */ if (vma_flags_test (new_vma_flags,VMA_LOCKED_BIT ))vma_flags_set (new_vma_flags,VMA_IO_BIT );
@@ -458,7 +458,7 @@ static void mlock_vma_pages_range(struct vm_area_struct *vma,/* * mlock_fixup - handle mlock[all]/munlock[all] requests. * -* Filters out "special" vmas -- VM_LOCKED never gets set for these, and +* Filters out "special" vmas -- VMA_LOCKED_BIT never gets set for these, and * munlock is a no-op. However, for some special vmas, we go ahead and * populate the ptes. *
@@ -466,24 +466,23 @@ static void mlock_vma_pages_range(struct vm_area_struct *vma,*/ static int mlock_fixup (struct vma_iterator *vmi,struct vm_area_struct *vma,struct vm_area_struct **prev,unsigned long start, -unsigned long end,vm_flags_t newflags) +unsigned long end,vma_flags_t *new_vma_flags) { -vma_flags_t new_vma_flags =legacy_to_vma_flags (newflags);const vma_flags_t old_vma_flags = vma->flags;struct mm_struct *mm = vma->vm_mm;int nr_pages;int ret =0 ; -if (vma_flags_same_pair (&old_vma_flags, &new_vma_flags) || +if (vma_flags_same_pair (&old_vma_flags, new_vma_flags) ||vma_is_secretmem (vma) || !vma_supports_mlock (vma)) {/* -* Don't set VM_LOCKED or VM_LOCKONFAULT and don't count. +* Don't set VMA_LOCKED_BIT or VM_LOCKONFAULT and don't count. * For secretmem, don't allow the memory to be unlocked. */ goto out; } - vma =vma_modify_flags (vmi, *prev, vma, start, end, &new_vma_flags); + vma =vma_modify_flags (vmi, *prev, vma, start, end, new_vma_flags);if (IS_ERR (vma)) { ret =PTR_ERR (vma); goto out;
@@ -493,7 +492,7 @@ static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_struct *vma,* Keep track of amount of locked VM. */ nr_pages = (end - start) >>PAGE_SHIFT ; -if (!vma_flags_test (&new_vma_flags,VMA_LOCKED_BIT )) +if (!vma_flags_test (new_vma_flags,VMA_LOCKED_BIT )) nr_pages = -nr_pages;else if (vma_flags_test (&old_vma_flags,VMA_LOCKED_BIT )) nr_pages =0 ;
@@ -502,15 +501,15 @@ static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_struct *vma,/* * vm_flags is protected by the mmap_lock held in write mode. * It's okay if try_to_unmap_one unmaps a page just after we -* set VM_LOCKED, populate_vma_page_range will bring it back. +* set VMA_LOCKED_BIT, populate_vma_page_range will bring it back. */ -if (vma_flags_test (&new_vma_flags,VMA_LOCKED_BIT ) && +if (vma_flags_test (new_vma_flags,VMA_LOCKED_BIT ) &&vma_flags_test (&old_vma_flags,VMA_LOCKED_BIT )) {/* No work to do, and mlocking twice would be wrong */ vma_start_write (vma); - vma->flags = new_vma_flags; + vma->flags = *new_vma_flags; }else { -mlock_vma_pages_range (vma, start, end, &new_vma_flags); +mlock_vma_pages_range (vma, start, end, new_vma_flags); } out: *prev = vma;
@@ -518,7 +517,7 @@ static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_struct *vma, }static int apply_vma_lock_flags (unsigned long start,size_t len, -vm_flags_t flags) +const vma_flags_t *flags) {unsigned long nstart, end, tmp;struct vm_area_struct *vma, *prev;
@@ -543,18 +542,20 @@ static int apply_vma_lock_flags(unsigned long start, size_t len, tmp = vma->vm_start;for_each_vma_range (vmi, vma, end) {int error; -vm_flags_t newflags; +vma_flags_t newflags;if (vma->vm_start != tmp)return -ENOMEM ; - newflags = vma->vm_flags & ~VM_LOCKED_MASK ; - newflags |= flags; + newflags = vma->flags; +vma_flags_clear_mask (&newflags,VMA_LOCKED_MASK ); +vma_flags_set_mask (&newflags, *flags); +/* Here we know that vma->vm_start <= nstart < vma->vm_end. */ tmp = vma->vm_end;if (tmp > end) tmp = end; - error =mlock_fixup (&vmi, vma, &prev, nstart, tmp, newflags); + error =mlock_fixup (&vmi, vma, &prev, nstart, tmp, &newflags);if (error)return error; tmp =vma_iter_end (&vmi);
@@ -589,7 +590,7 @@ static unsigned long count_mm_mlocked_page_nr(struct mm_struct *mm, end = start + len;for_each_vma_range (vmi, vma, end) { -if (vma->vm_flags &VM_LOCKED ) { +if (vma_test (vma,VMA_LOCKED_BIT )) {if (start > vma->vm_start) count -= (start - vma->vm_start);if (end < vma->vm_end) {
@@ -615,7 +616,8 @@ static int __mlock_posix_error_return(long retval)return retval; } -static __must_check int do_mlock (unsigned long start,size_t len,vm_flags_t flags) +static __must_check int do_mlock (unsigned long start,size_t len, +vma_flags_t *flags) {unsigned long locked;unsigned long lock_limit;
@@ -664,24 +666,27 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla SYSCALL_DEFINE2(mlock, unsigned long, start, size_t, len) { -return do_mlock (start, len,VM_LOCKED ); +vma_flags_t flags =mk_vma_flags (VMA_LOCKED_BIT ); + +return do_mlock (start, len, &flags); } SYSCALL_DEFINE3(mlock2, unsigned long, start, size_t, len, int, flags) { -vm_flags_t vm_flags =VM_LOCKED ; +vma_flags_t vma_flags =mk_vma_flags (VMA_LOCKED_BIT );if (flags & ~MLOCK_ONFAULT )return -EINVAL ;if (flags &MLOCK_ONFAULT ) - vm_flags |=VM_LOCKONFAULT ; +vma_flags_set (&vma_flags,VMA_LOCKONFAULT_BIT ); -return do_mlock (start, len, vm_flags); +return do_mlock (start, len, &vma_flags); } SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len) { +vma_flags_t flags =EMPTY_VMA_FLAGS ;int ret; start =untagged_addr (start);
@@ -691,7 +696,7 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)if (mmap_write_lock_killable (current->mm))return -EINTR ; - ret =apply_vma_lock_flags (start, len,0 ); + ret =apply_vma_lock_flags (start, len, &flags);mmap_write_unlock (current->mm);return ret;
@@ -705,14 +710,15 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)* There are a couple of subtleties with this. If mlockall() is called multiple * times with different flags, the values do not necessarily stack. If mlockall * is called once including the MCL_FUTURE flag and then a second time without -* it, VM_LOCKED and VM_LOCKONFAULT will be cleared from mm->def_vma_flags. +* it, VMA_LOCKED_BIT and VMA_LOCKONFAULT_BIT will be cleared from +* mm->def_vma_flags. */ static int apply_mlockall_flags (int flags) {VMA_ITERATOR (vmi, current->mm,0 );struct mm_struct *mm = current->mm;struct vm_area_struct *vma, *prev =NULL ; -vm_flags_t to_add =0 ; +vma_flags_t to_add =EMPTY_VMA_FLAGS ;vma_flags_clear_mask (&mm->def_vma_flags,VMA_LOCKED_MASK );if (flags &MCL_FUTURE ) {
@@ -726,20 +732,20 @@ static int apply_mlockall_flags(int flags) }if (flags &MCL_CURRENT ) { - to_add |=VM_LOCKED ; +vma_flags_set (&to_add,VMA_LOCKED_BIT );if (flags &MCL_ONFAULT ) - to_add |=VM_LOCKONFAULT ; +vma_flags_set (&to_add,VMA_LOCKONFAULT_BIT ); }for_each_vma (vmi, vma) {int error; -vm_flags_t newflags; +vma_flags_t newflags = vma->flags; - newflags = vma->vm_flags & ~VM_LOCKED_MASK ; - newflags |= to_add; +vma_flags_clear_mask (&newflags,VMA_LOCKED_MASK ); +vma_flags_set_mask (&newflags, to_add); error =mlock_fixup (&vmi, vma, &prev, vma->vm_start, vma->vm_end, - newflags); + &newflags);/* Ignore errors, but prev needs fixing up. */ if (error) prev = vma;
Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerPATCH 12/13 mm/mprotect: convert mprotect code to use vma_flags_t
In reply to Lorenzo StoakesReplace use of the legacy vm_flags_t flags with vma_flags_t values throughout the mprotect logic.
Note that we retain the legacy vm_flags_t bit shifting code in do_mprotect_key(), deferring a vma_flags_t approach to this for the time being.
Additionally update comments to reflect the changes to be consistent.
No functional change intended.
--- mm/mprotect.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
mm/mprotect.c +8 -8
@@ -40,7 +40,7 @@static bool maybe_change_pte_writable (struct vm_area_struct *vma,pte_t pte) { -if (WARN_ON_ONCE (!(vma->vm_flags &VM_WRITE ))) +if (WARN_ON_ONCE (!vma_test (vma,VMA_WRITE_BIT )))return false;/* Don't touch entries that are not even readable. */
@@ -97,7 +97,7 @@ static bool can_change_shared_pte_writable(struct vm_area_struct *vma,bool can_change_pte_writable (struct vm_area_struct *vma,unsigned long addr,pte_t pte) { -if (!(vma->vm_flags &VM_SHARED )) +if (!vma_test (vma,VMA_SHARED_BIT ))return can_change_private_pte_writable (vma, addr, pte);return can_change_shared_pte_writable (vma, pte);
@@ -194,7 +194,7 @@ static __always_inline void set_write_prot_commit_flush_ptes(struct vm_area_stru {bool set_write; -if (vma->vm_flags &VM_SHARED ) { +if (vma_test (vma,VMA_SHARED_BIT )) { set_write =can_change_shared_pte_writable (vma, ptent);prot_commit_flush_ptes (vma, addr, ptep, oldpte, ptent, nr_ptes,/* idx = */ 0 , set_write, tlb);
@@ -811,8 +811,8 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,vm_unacct_memory (nrpages);/* -* Private VM_LOCKED VMA becoming writable: trigger COW to avoid major -* fault on access. +* Private VMA_LOCKED_BIT VMA becoming writable: trigger COW to avoid +* major fault on access. */ if (vma_flags_test(&new_vma_flags, VMA_WRITE_BIT) && vma_flags_test(&old_vma_flags, VMA_LOCKED_BIT) &&
@@ -886,7 +886,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, goto out; start = vma->vm_start; error = -EINVAL ; -if (!(vma->vm_flags &VM_GROWSDOWN )) +if (!vma_test (vma,VMA_GROWSDOWN_BIT )) goto out; }else {if (vma->vm_start > start)
@@ -894,7 +894,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,if (unlikely (grows &PROT_GROWSUP )) { end = vma->vm_end; error = -EINVAL ; -if (!(vma->vm_flags &VM_GROWSUP )) +if (!vma_test (vma,VMA_GROWSUP_BIT )) goto out; } }
@@ -918,7 +918,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, }/* Does the application expect PROT_READ to imply PROT_EXEC */ -if (rier && (vma->vm_flags &VM_MAYEXEC )) +if (rier &&vma_test (vma,VMA_MAYEXEC_BIT )) prot |=PROT_EXEC ;/*
Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerPATCH 13/13 mm/mremap: convert mremap code to use vma_flags_t
In reply to Lorenzo StoakesReplace use of the legacy vm_flags_t flags with vma_flags_t values throughout the mremap logic.
Additionally update comments to reflect the changes to be consistent.
No functional change intended.
--- mm/mremap.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-)
mm/mremap.c +20 -18
@@ -68,7 +68,7 @@ struct vma_remap_struct {bool populate_expand;/* mlock()'d expanded, must populate. */ enum mremap_type remap_type;/* expand, shrink, etc. */ bool mmap_locked;/* Is mm currently write-locked? */ -unsigned long charged;/* If VM_ACCOUNT, # pages to account. */ +unsigned long charged;/* If VMA_ACCOUNT_BIT, # pgs to account */ bool vmi_needs_invalidate;/* Is the VMA iterator invalidated? */ };
@@ -954,7 +954,7 @@ static unsigned long vrm_set_new_addr(struct vma_remap_struct *vrm)if (vrm->flags &MREMAP_FIXED ) map_flags |=MAP_FIXED ; -if (vma->vm_flags &VM_MAYSHARE ) +if (vma_test (vma,VMA_MAYSHARE_BIT )) map_flags |=MAP_SHARED ; res = get_unmapped_area(vma->vm_file, new_addr, vrm->new_len, pgoff,
@@ -976,7 +976,7 @@ static bool vrm_calc_charge(struct vma_remap_struct *vrm) {unsigned long charged; -if (!(vrm->vma->vm_flags &VM_ACCOUNT )) +if (!vma_test (vrm->vma,VMA_ACCOUNT_BIT ))return true;/*
@@ -1003,7 +1003,7 @@ static bool vrm_calc_charge(struct vma_remap_struct *vrm)*/ static void vrm_uncharge (struct vma_remap_struct *vrm) { -if (!(vrm->vma->vm_flags &VM_ACCOUNT )) +if (!vma_test (vrm->vma,VMA_ACCOUNT_BIT ))return ;vm_unacct_memory (vrm->charged);
@@ -1023,7 +1023,7 @@ static void vrm_stat_account(struct vma_remap_struct *vrm,struct vm_area_struct *vma = vrm->vma;vm_stat_account (mm, vma->vm_flags, pages); -if (vma->vm_flags &VM_LOCKED ) +if (vma_test (vma,VMA_LOCKED_BIT )) mm->locked_vm += pages; }
@@ -1167,7 +1167,7 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)* arose, in which case we _do_ wish to unmap the _new_ VMA, which means * we actually _do_ want it be unaccounted. */ -bool accountable_move = (vma->vm_flags &VM_ACCOUNT ) && +bool accountable_move =vma_test (vma,VMA_ACCOUNT_BIT ) && !(vrm->flags &MREMAP_DONTUNMAP );/*
@@ -1186,7 +1186,7 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)* portions of the original VMA that remain. */ if (accountable_move) { -vm_flags_clear (vma,VM_ACCOUNT ); +vma_clear_flags (vma,VMA_ACCOUNT_BIT );/* We are about to split vma, so store the start/end. */ vm_start = vma->vm_start; vm_end = vma->vm_end;
@@ -1211,8 +1211,8 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)* | | * |-------------| * -* Having cleared VM_ACCOUNT from the whole VMA, after we unmap above -* we'll end up with: +* Having cleared VMA_ACCOUNT_BIT from the whole VMA, after we unmap +* above we'll end up with: * * addr end * | |
@@ -1232,13 +1232,15 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)if (vm_start < addr) {struct vm_area_struct *prev =vma_prev (&vmi); -vm_flags_set (prev,VM_ACCOUNT );/* Acquires VMA lock. */ +vma_start_write (prev); +vma_set_flags (prev,VMA_ACCOUNT_BIT ); }if (vm_end > end) {struct vm_area_struct *next =vma_next (&vmi); -vm_flags_set (next,VM_ACCOUNT );/* Acquires VMA lock. */ +vma_start_write (next); +vma_set_flags (next,VMA_ACCOUNT_BIT ); } } }
@@ -1321,8 +1323,8 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,unsigned long old_start = vrm->vma->vm_start;unsigned long old_end = vrm->vma->vm_end; -/* We always clear VM_LOCKED[ONFAULT] on the old VMA. */ -vm_flags_clear (vrm->vma,VM_LOCKED_MASK ); +/* We always clear VMA_LOCKED[ONFAULT]_BIT on the old VMA. */ +vma_clear_flags_mask (vrm->vma,VMA_LOCKED_MASK );/* * anon_vma links of the old vma is no longer needed after its page
@@ -1758,14 +1760,14 @@ static int check_prep_vma(struct vma_remap_struct *vrm)* based on the original. There are no known use cases for this * behavior. As a result, fail such attempts. */ -if (!old_len && !(vma->vm_flags & (VM_SHARED |VM_MAYSHARE ))) { +if (!old_len && !vma_test_any (vma,VMA_SHARED_BIT ,VMA_MAYSHARE_BIT )) {pr_warn_once ("%s (%d): attempted to duplicate a private mapping with mremap. This is not supported.\n" , current->comm, current->pid);return -EINVAL ; }if ((vrm->flags &MREMAP_DONTUNMAP ) && - (vma->vm_flags & (VM_DONTEXPAND |VM_PFNMAP ))) +vma_test_any (vma,VMA_DONTEXPAND_BIT ,VMA_PFNMAP_BIT ))return -EINVAL ;/*
@@ -1795,7 +1797,7 @@ static int check_prep_vma(struct vma_remap_struct *vrm)return 0 ;/* We are expanding and the VMA is mlock()'d so we need to populate. */ -if (vma->vm_flags &VM_LOCKED ) +if (vma_test (vma,VMA_LOCKED_BIT )) vrm->populate_expand = true;/* Need to be careful about a growing mapping */
@@ -1803,10 +1805,10 @@ static int check_prep_vma(struct vma_remap_struct *vrm)if (pgoff + (new_len >>PAGE_SHIFT ) < pgoff)return -EINVAL ; -if (vma->vm_flags & (VM_DONTEXPAND |VM_PFNMAP )) +if (vma_test_any (vma,VMA_DONTEXPAND_BIT ,VMA_PFNMAP_BIT ))return -EFAULT ; -if (!mlock_future_ok (mm, vma->vm_flags &VM_LOCKED , vrm->delta)) +if (!mlock_future_ok (mm,vma_test (vma,VMA_LOCKED_BIT ), vrm->delta))return -EAGAIN ;if (!may_expand_vm (mm, &vma->flags, vrm->delta >>PAGE_SHIFT ))
Signature · 1 line
2.54.0
Zi Yan
reviewerRe: [PATCH 01/13] mm: introduce vma_flags_can_grow() and vma_can_grow()
In reply to Lorenzo StoakesQuoted 48 lines · Lorenzo Stoakes
These test whether the VMA has stack sematics, i.e. is able to grow upwards or downwards depending on the architecture.
In order to account for arches which do not support upward-growing stacks, introduce
VMA_GROWSUPwhose definition depends on the architecture supporting it, and usevma_flags_test_single_mask()invma_flags_can_grow()to account for this.Update the VMA userland tests to reflect the changes
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- include/linux/mm.h | 21 ++++++++++++++++++--- tools/testing/vma/include/dup.h | 4 ++++ 2 files changed, 22 insertions(+), 3 deletions(-)include/linux/mm.h +5
-2@@ -472,6 +472,7 @@ enum {#define VM_SAO INIT_VM_FLAG(SAO)#elif defined(CONFIG_PARISC )#define VM_GROWSUP INIT_VM_FLAG(GROWSUP) +#define VMA_GROWSUP mk_vma_flags(VMA_GROWSUP_BIT)#elif defined(CONFIG_SPARC64 )#define VM_SPARC_ADI INIT_VM_FLAG(SPARC_ADI)#define VM_ARCH_CLEAR INIT_VM_FLAG(ARCH_CLEAR)@@ -483,6 +484,7 @@ enum {#endif #ifndef VM_GROWSUP #define VM_GROWSUP VM_NONE +#define VMA_GROWSUP EMPTY_VMA_FLAGS#endif #ifdef CONFIG_ARM64_MTE #define VM_MTE INIT_VM_FLAG(MTE)@@ -1563,11 +1565,24 @@ static inline bool vma_is_initial_stack(const struct vm_area_struct *vma) vma->vm_end >= vma->vm_mm->start_stack; } -static inline bool vma_is_temporary_stack (const struct vm_area_struct *vma) +static inline bool vma_flags_can_grow (const vma_flags_t *flags) { -int maybe_stack = vma->vm_flags & (VM_GROWSDOWN |VM_GROWSUP ); +if (vma_flags_test_single_mask (flags,VMA_GROWSUP )) +return true;
include/linux/mm.h static inline bool vma_is_initial_stack(const struct vm_area_struct *vma) · line 1580
Quoted 10 lines
include/linux/mm.h +8
-1+if (vma_flags_test (flags,VMA_GROWSDOWN_BIT )) +return true; + +return false; +} -if (!maybe_stack) +static inline bool vma_can_grow (const struct vm_area_struct *vma) +{ +return vma_flags_can_grow (&vma->flags);
Would it save vma_flags_can_grow() if we do below?
return vma_test(vma, VMA_GROWSDOWN_BIT) || vma_test_single_mask(vma, VMA_GROWSUP);
I find these two functions when I am reading mm.h.
Quoted 24 lines
include/linux/mm.h +5
-0+} + +static inline bool vma_is_temporary_stack (const struct vm_area_struct *vma) +{ +if (!vma_can_grow (vma))return false; if ((vma->vm_flags & VM_STACK_INCOMPLETE_SETUP) ==tools/testing/vma/include/dup.h +2
-0@@ -245,8 +245,10 @@ enum {#define VM_STACK INIT_VM_FLAG(STACK)#ifdef CONFIG_STACK_GROWS_UP #define VM_STACK_EARLY INIT_VM_FLAG(STACK_EARLY) +#define VMA_STACK_EARLY mk_vma_flags(VMA_STACK_EARLY_BIT)#else #define VM_STACK_EARLY VM_NONE +#define VMA_STACK_EARLY EMPTY_VMA_FLAGS#endif #ifdef CONFIG_ARCH_HAS_PKEYS #define VM_PKEY_SHIFT ((__force int)VMA_HIGH_ARCH_0_BIT)
tools/testing/vma/include/dup.h enum { · line 324
Quoted 8 lines
tools/testing/vma/include/dup.h +2
-0@@ -315,6 +317,8 @@ enum {/* Bits set in the VMA until the stack is in its final location */ #define VM_STACK_INCOMPLETE_SETUP (VM_RAND_READ | VM_SEQ_READ | VM_STACK_EARLY) +#define VMA_STACK_INCOMPLETE_SETUP append_vma_flags( \ + VMA_STACK_EARLY, VMA_RAND_READ_BIT, VMA_SEQ_READ_BIT)#define TASK_EXEC_BIT ((current->personality & READ_IMPLIES_EXEC) ? \ VM_EXEC_BIT : VM_READ_BIT)
Why are VMA_STACK_EARLY and VMA_STACK_INCOMPLETE_SETUP added here but not in mm.h?
Signature · 2 lines
Best Regards, Yan, Zi
Lorenzo Stoakes
maintainerRe: [PATCH 01/13] mm: introduce vma_flags_can_grow() and vma_can_grow()
Quoted 62 lines · Zi Yan
Quoted 48 lines · Lorenzo Stoakes
These test whether the VMA has stack sematics, i.e. is able to grow upwards or downwards depending on the architecture.
In order to account for arches which do not support upward-growing stacks, introduce
VMA_GROWSUPwhose definition depends on the architecture supporting it, and usevma_flags_test_single_mask()invma_flags_can_grow()to account for this.Update the VMA userland tests to reflect the changes
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- include/linux/mm.h | 21 ++++++++++++++++++--- tools/testing/vma/include/dup.h | 4 ++++ 2 files changed, 22 insertions(+), 3 deletions(-)include/linux/mm.h +5
-2@@ -472,6 +472,7 @@ enum {#define VM_SAO INIT_VM_FLAG(SAO)#elif defined(CONFIG_PARISC )#define VM_GROWSUP INIT_VM_FLAG(GROWSUP) +#define VMA_GROWSUP mk_vma_flags(VMA_GROWSUP_BIT)#elif defined(CONFIG_SPARC64 )#define VM_SPARC_ADI INIT_VM_FLAG(SPARC_ADI)#define VM_ARCH_CLEAR INIT_VM_FLAG(ARCH_CLEAR)@@ -483,6 +484,7 @@ enum {#endif #ifndef VM_GROWSUP #define VM_GROWSUP VM_NONE +#define VMA_GROWSUP EMPTY_VMA_FLAGS#endif #ifdef CONFIG_ARM64_MTE #define VM_MTE INIT_VM_FLAG(MTE)@@ -1563,11 +1565,24 @@ static inline bool vma_is_initial_stack(const struct vm_area_struct *vma) vma->vm_end >= vma->vm_mm->start_stack; } -static inline bool vma_is_temporary_stack (const struct vm_area_struct *vma) +static inline bool vma_flags_can_grow (const vma_flags_t *flags) { -int maybe_stack = vma->vm_flags & (VM_GROWSDOWN |VM_GROWSUP ); +if (vma_flags_test_single_mask (flags,VMA_GROWSUP )) +return true;include/linux/mm.h static inline bool vma_is_initial_stack(const struct vm_area_struct *vma) · line 1580
Quoted 10 lines
include/linux/mm.h +8
-1+if (vma_flags_test (flags,VMA_GROWSDOWN_BIT )) +return true; + +return false; +} -if (!maybe_stack) +static inline bool vma_can_grow (const struct vm_area_struct *vma) +{ +return vma_flags_can_grow (&vma->flags);Would it save
vma_flags_can_grow()if we do below?
Quoted 3 lines
return
vma_test(vma, VMA_GROWSDOWN_BIT)||vma_test_single_mask(vma, VMA_GROWSUP);I find these two functions when I am reading
mm.h.
Yeah but we require vma_flags_can_grow() for code in mmap.c, the majority of checks of this have only vma_flags_t to work with not a VMA :)
Quoted 31 lines
Quoted 31 lines
include/linux/mm.h +5
-0+} + +static inline bool vma_is_temporary_stack (const struct vm_area_struct *vma) +{ +if (!vma_can_grow (vma))return false; if ((vma->vm_flags & VM_STACK_INCOMPLETE_SETUP) ==tools/testing/vma/include/dup.h +4
-0@@ -245,8 +245,10 @@ enum {#define VM_STACK INIT_VM_FLAG(STACK)#ifdef CONFIG_STACK_GROWS_UP #define VM_STACK_EARLY INIT_VM_FLAG(STACK_EARLY) +#define VMA_STACK_EARLY mk_vma_flags(VMA_STACK_EARLY_BIT)#else #define VM_STACK_EARLY VM_NONE +#define VMA_STACK_EARLY EMPTY_VMA_FLAGS#endif #ifdef CONFIG_ARCH_HAS_PKEYS #define VM_PKEY_SHIFT ((__force int)VMA_HIGH_ARCH_0_BIT)@@ -315,6 +317,8 @@ enum {/* Bits set in the VMA until the stack is in its final location */ #define VM_STACK_INCOMPLETE_SETUP (VM_RAND_READ | VM_SEQ_READ | VM_STACK_EARLY) +#define VMA_STACK_INCOMPLETE_SETUP append_vma_flags( \ + VMA_STACK_EARLY, VMA_RAND_READ_BIT, VMA_SEQ_READ_BIT)
Quoted 5 lines
tools/testing/vma/include/dup.h enum { · line 324
Quoted 2 lines
tools/testing/vma/include/dup.h +0
-0#define TASK_EXEC_BIT ((current->personality & READ_IMPLIES_EXEC) ? \ VM_EXEC_BIT : VM_READ_BIT)Why are
VMA_STACK_EARLYandVMA_STACK_INCOMPLETE_SETUPadded here but not inmm.h?
Yeah urgh oops my bad. It doesn't really break anything but I'll fix it if a respin is needed...
Quoted 6 lines
--
Best Regards,
Yan, Zi
Thanks, Lorenzo
Re: [PATCH 08/13] mm: introduce vma_get_page_prot() and use it
In reply to Lorenzo StoakesAm 29.06.26 um 21:25 schrieb Lorenzo Stoakes:
Quoted 10 lines
There's a large number of
vm_get_page_prot(vma->vm_flags)invocations. Make life easier by introducingvma_get_page_prot()parameterised by the VMA.This also makes converting
vm_get_page_prot()tovma_flags_teasier.Also update the userland VMA tests to reflect the change.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>
For the DRM changes:
Quoted 402 lines
--- drivers/gpu/drm/drm_gem.c | 2 +- drivers/gpu/drm/drm_gem_dma_helper.c | 2 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_gem.c | 6 +++--- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 12 ++++++------ drivers/gpu/drm/msm/msm_gem.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +- drivers/gpu/drm/omapdrm/omap_gem.c | 6 +++--- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 2 +- drivers/gpu/drm/tegra/gem.c | 2 +- drivers/gpu/drm/virtio/virtgpu_vram.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 2 +- drivers/gpu/drm/xe/xe_device.c | 2 +- drivers/gpu/drm/xe/xe_mmio_gem.c | 2 +- drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +- drivers/video/fbdev/core/fb_io_fops.c | 2 +- include/linux/mm.h | 10 +++++++++- mm/vma.c | 2 +- mm/vma_exec.c | 2 +- sound/core/memalloc.c | 2 +- tools/testing/vma/include/dup.h | 4 ++++ 23 files changed, 43 insertions(+), 31 deletions(-)drivers/gpu/drm/drm_gem.c +1
-1@@ -1252,7 +1252,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, }vm_flags_set (vma,VM_IO |VM_PFNMAP |VM_DONTEXPAND |VM_DONTDUMP ); - vma->vm_page_prot =pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_writecombine (vma_get_page_prot (vma)); vma->vm_page_prot =pgprot_decrypted (vma->vm_page_prot); }drivers/gpu/drm/drm_gem_dma_helper.c +1
-1@@ -540,7 +540,7 @@ int drm_gem_dma_mmap(struct drm_gem_dma_object *dma_obj, struct vm_area_struct *vm_flags_mod (vma,VM_DONTDUMP |VM_DONTEXPAND ,VM_PFNMAP );if (dma_obj->map_noncoherent) { - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); ret = dma_mmap_pages(drm_dev_dma_dev(dma_obj->base.dev), vma, vma->vm_end - vma->vm_start,drivers/gpu/drm/drm_gem_shmem_helper.c +1
-1@@ -764,7 +764,7 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_structreturn ret;vm_flags_set (vma,VM_PFNMAP |VM_DONTEXPAND |VM_DONTDUMP ); - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma);if (shmem->map_wc) vma->vm_page_prot =pgprot_writecombine (vma->vm_page_prot);drivers/gpu/drm/etnaviv/etnaviv_gem.c +1
-1@@ -133,7 +133,7 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,vm_flags_set (vma,VM_PFNMAP |VM_DONTEXPAND |VM_DONTDUMP ); - vm_page_prot =vm_get_page_prot (vma->vm_flags); + vm_page_prot =vma_get_page_prot (vma);if (etnaviv_obj->flags &ETNA_BO_WC ) { vma->vm_page_prot =pgprot_writecombine (vm_page_prot);drivers/gpu/drm/exynos/exynos_drm_gem.c +3
-3@@ -377,13 +377,13 @@ static int exynos_drm_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct/* non-cachable as default. */ if (exynos_gem->flags &EXYNOS_BO_CACHABLE ) - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma);else if (exynos_gem->flags &EXYNOS_BO_WC ) vma->vm_page_prot = -pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); +pgprot_writecombine (vma_get_page_prot (vma));else vma->vm_page_prot = -pgprot_noncached (vm_get_page_prot (vma->vm_flags)); +pgprot_noncached (vma_get_page_prot (vma)); ret =exynos_drm_gem_mmap_buffer (exynos_gem, vma);if (ret)drivers/gpu/drm/i915/gem/i915_gem_mman.c +6
-6@@ -112,7 +112,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data, vma =find_vma (mm, addr);if (vma &&__vma_matches (vma, obj->base.filp, addr, args->size)) vma->vm_page_prot = -pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); +pgprot_writecombine (vma_get_page_prot (vma));else addr = -ENOMEM ;mmap_write_unlock (mm);@@ -1024,7 +1024,7 @@ i915_gem_object_mmap(struct drm_i915_gem_object *obj,fput (anon);if (obj->ops->mmap_ops) { - vma->vm_page_prot =pgprot_decrypted (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_decrypted (vma_get_page_prot (vma)); vma->vm_ops = obj->ops->mmap_ops; vma->vm_private_data = obj->base.vma_node.driver_private;return 0 ;@@ -1035,7 +1035,7 @@ i915_gem_object_mmap(struct drm_i915_gem_object *obj,switch (mmo->mmap_type) {case I915_MMAP_TYPE_WC : vma->vm_page_prot = -pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); +pgprot_writecombine (vma_get_page_prot (vma)); vma->vm_ops = &vm_ops_cpu;break ;@@ -1043,19 +1043,19 @@ i915_gem_object_mmap(struct drm_i915_gem_object *obj,GEM_WARN_ON (1 ); fallthrough;case I915_MMAP_TYPE_WB : - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); vma->vm_ops = &vm_ops_cpu;break ;case I915_MMAP_TYPE_UC : vma->vm_page_prot = -pgprot_noncached (vm_get_page_prot (vma->vm_flags)); +pgprot_noncached (vma_get_page_prot (vma)); vma->vm_ops = &vm_ops_cpu;break ;case I915_MMAP_TYPE_GTT : vma->vm_page_prot = -pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); +pgprot_writecombine (vma_get_page_prot (vma)); vma->vm_ops = &vm_ops_gtt;break ; }drivers/gpu/drm/msm/msm_gem.c +1
-1@@ -1125,7 +1125,7 @@ static int msm_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_structstruct msm_gem_object *msm_obj =to_msm_bo (obj);vm_flags_set (vma,VM_PFNMAP |VM_DONTEXPAND |VM_DONTDUMP ); - vma->vm_page_prot =msm_gem_pgprot (msm_obj,vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =msm_gem_pgprot (msm_obj,vma_get_page_prot (vma));return 0 ; }drivers/gpu/drm/nouveau/nouveau_gem.c +1
-1@@ -55,7 +55,7 @@ static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf) goto error_unlock;nouveau_bo_del_io_reserve_lru (bo); - prot =vm_get_page_prot (vma->vm_flags); + prot =vma_get_page_prot (vma); ret =ttm_bo_vm_fault_reserved (vmf, prot,TTM_BO_VM_NUM_PREFAULT );nouveau_bo_add_io_reserve_lru (bo);if (ret ==VM_FAULT_RETRY && !(vmf->flags &FAULT_FLAG_RETRY_NOWAIT ))drivers/gpu/drm/omapdrm/omap_fbdev.c +1
-1@@ -84,7 +84,7 @@ static int omap_fbdev_pan_display(struct fb_var_screeninfo *var, struct fb_infostatic int omap_fbdev_fb_mmap (struct fb_info *info,struct vm_area_struct *vma) { - vma->vm_page_prot =pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_writecombine (vma_get_page_prot (vma));return fb_deferred_io_mmap (info, vma); }drivers/gpu/drm/omapdrm/omap_gem.c +3
-3@@ -538,9 +538,9 @@ static int omap_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_strucvm_flags_set (vma,VM_DONTEXPAND |VM_DONTDUMP |VM_IO |VM_MIXEDMAP );if (omap_obj->flags &OMAP_BO_WC ) { - vma->vm_page_prot =pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_writecombine (vma_get_page_prot (vma)); }else if (omap_obj->flags &OMAP_BO_UNCACHED ) { - vma->vm_page_prot =pgprot_noncached (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_noncached (vma_get_page_prot (vma)); }else {/* * We do have some private objects, at least for scanout buffers @@ -558,7 +558,7 @@ static int omap_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struc vma->vm_pgoff -=drm_vma_node_start (&obj->vma_node);vma_set_file (vma, obj->filp); - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); } vma->vm_page_prot =pgprot_decrypted (vma->vm_page_prot);drivers/gpu/drm/rockchip/rockchip_drm_gem.c +1
-1@@ -255,7 +255,7 @@ static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,*/ vm_flags_mod (vma,VM_IO |VM_DONTEXPAND |VM_DONTDUMP ,VM_PFNMAP ); - vma->vm_page_prot =pgprot_writecombine (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_writecombine (vma_get_page_prot (vma)); vma->vm_page_prot =pgprot_decrypted (vma->vm_page_prot);if (rk_obj->pages)drivers/gpu/drm/tegra/gem.c +1
-1@@ -602,7 +602,7 @@ int __tegra_gem_mmap(struct drm_gem_object *gem, struct vm_area_struct *vma) vma->vm_pgoff = vm_pgoff; }else { -pgprot_t prot =vm_get_page_prot (vma->vm_flags); +pgprot_t prot =vma_get_page_prot (vma);vm_flags_mod (vma,VM_MIXEDMAP ,VM_PFNMAP );drivers/gpu/drm/virtio/virtgpu_vram.c +1
-1@@ -55,7 +55,7 @@ static int virtio_gpu_vram_mmap(struct drm_gem_object *obj, vma->vm_pgoff -=drm_vma_node_start (&obj->vma_node);vm_flags_set (vma,VM_MIXEDMAP |VM_DONTEXPAND ); - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); vma->vm_page_prot =pgprot_decrypted (vma->vm_page_prot); vma->vm_ops = &virtio_gpu_vram_vm_ops;drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c +1
-1@@ -481,7 +481,7 @@ vm_fault_t vmw_bo_vm_fault(struct vm_fault *vmf)if (vbo->dirty && vbo->dirty->method ==VMW_BO_DIRTY_MKWRITE ) prot =vm_get_page_prot (vma->vm_flags & ~VM_SHARED );else - prot =vm_get_page_prot (vma->vm_flags); + prot =vma_get_page_prot (vma); ret =ttm_bo_vm_fault_reserved (vmf, prot, num_prefault);if (ret ==VM_FAULT_RETRY && !(vmf->flags &FAULT_FLAG_RETRY_NOWAIT ))drivers/gpu/drm/xe/xe_device.c +1
-1@@ -281,7 +281,7 @@ static vm_fault_t barrier_fault(struct vm_fault *vmf)pgprot_t prot;int idx; - prot =vm_get_page_prot (vma->vm_flags); + prot =vma_get_page_prot (vma);if (drm_dev_enter (dev, &idx)) {unsigned long pfn;drivers/gpu/drm/xe/xe_mmio_gem.c +1
-1@@ -149,7 +149,7 @@ static int xe_mmio_gem_mmap(struct drm_gem_object *base, struct vm_area_struct */* Set vm_pgoff (used as a fake buffer offset by DRM) to 0 */ vma->vm_pgoff =0 ; - vma->vm_page_prot =pgprot_noncached (vm_get_page_prot (vma->vm_flags)); + vma->vm_page_prot =pgprot_noncached (vma_get_page_prot (vma));vm_flags_set (vma,VM_IO |VM_PFNMAP |VM_DONTEXPAND |VM_DONTDUMP |VM_DONTCOPY |VM_NORESERVE );drivers/gpu/drm/xen/xen_drm_front_gem.c +1
-1@@ -80,7 +80,7 @@ static int xen_drm_front_gem_object_mmap(struct drm_gem_object *gem_obj,* which is mapped as Normal Inner Write-Back Outer Write-Back * Inner-Shareable. */ - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma);/* * vm_operations_struct.fault handler will be called if CPU access drivers/video/fbdev/core/fb_io_fops.c +1
-1@@ -161,7 +161,7 @@ int fb_io_mmap(struct fb_info *info, struct vm_area_struct *vma) len = info->fix.mmio_len; } - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); vma->vm_page_prot =pgprot_framebuffer (vma->vm_page_prot, vma->vm_start, vma->vm_end, start);include/linux/mm.h +9
-1@@ -4575,6 +4575,10 @@ static inline pgprot_t vma_flags_to_page_prot(vma_flags_t vma_flags) return vm_get_page_prot(vm_flags); } +static inline pgprot_t vma_get_page_prot (const struct vm_area_struct *vma) +{ +return vma_flags_to_page_prot (vma->flags); +}void vma_set_page_prot (struct vm_area_struct *vma);#else static inline pgprot_t vm_get_page_prot (vm_flags_t vm_flags)@@ -4585,9 +4589,13 @@ static inline pgprot_t vma_flags_to_page_prot(vma_flags_t vma_flags) {return __pgprot (0 ); } +static inline pgprot_t vma_get_page_prot (const struct vm_area_struct *vma) +{ +return __pgprot (0 ); +}static inline void vma_set_page_prot (struct vm_area_struct *vma) { - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); }#endif mm/vma.c +1
-1@@ -3429,7 +3429,7 @@ struct vm_area_struct *__install_special_mapping(if (pgtable_supports_soft_dirty ()) vm_flags |=VM_SOFTDIRTY ;vm_flags_init (vma, vm_flags & ~VM_LOCKED_MASK ); - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); vma->vm_ops = ops; vma->vm_private_data = priv;mm/vma_exec.c +1
-1@@ -146,7 +146,7 @@ int create_init_stack_vma(struct mm_struct *mm, struct vm_area_struct **vmap,if (pgtable_supports_soft_dirty ()) flags |=VM_SOFTDIRTY ;vm_flags_init (vma, flags); - vma->vm_page_prot =vm_get_page_prot (vma->vm_flags); + vma->vm_page_prot =vma_get_page_prot (vma); err =insert_vm_struct (mm, vma);if (err)sound/core/memalloc.c +1
-1@@ -851,7 +851,7 @@ static void snd_dma_noncoherent_free(struct snd_dma_buffer *dmab)static int snd_dma_noncoherent_mmap (struct snd_dma_buffer *dmab,struct vm_area_struct *area) { - area->vm_page_prot =vm_get_page_prot (area->vm_flags); + area->vm_page_prot =vma_get_page_prot (area);return dma_mmap_pages (dmab->dev.dev, area, area->vm_end - area->vm_start,virt_to_page (dmab->area));tools/testing/vma/include/dup.h +4
-0@@ -1577,3 +1577,7 @@ static inline pgoff_t linear_page_index(const struct vm_area_struct *vma, pgoff +=vma_start_pgoff (vma);return pgoff; } +static inline pgprot_t vma_get_page_prot (const struct vm_area_struct *vma) +{ +return vma_flags_to_page_prot (vma->flags); +}
Signature · 6 lines
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
Re: [PATCH 08/13] mm: introduce vma_get_page_prot() and use it
In reply to Lorenzo StoakesQuoted 1 line · Lorenzo Stoakes
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 12 ++++++------
For i915,
Signature · 1 line
Jani Nikula, Intel
Zi Yan
reviewerRe: [PATCH 01/13] mm: introduce vma_flags_can_grow() and vma_can_grow()
In reply to Lorenzo StoakesQuoted 63 lines · Lorenzo Stoakes
Quoted 62 lines · Zi Yan
Quoted 48 lines · Lorenzo Stoakes
These test whether the VMA has stack sematics, i.e. is able to grow upwards or downwards depending on the architecture.
In order to account for arches which do not support upward-growing stacks, introduce
VMA_GROWSUPwhose definition depends on the architecture supporting it, and usevma_flags_test_single_mask()invma_flags_can_grow()to account for this.Update the VMA userland tests to reflect the changes
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- include/linux/mm.h | 21 ++++++++++++++++++--- tools/testing/vma/include/dup.h | 4 ++++ 2 files changed, 22 insertions(+), 3 deletions(-)include/linux/mm.h +5
-2@@ -472,6 +472,7 @@ enum {#define VM_SAO INIT_VM_FLAG(SAO)#elif defined(CONFIG_PARISC )#define VM_GROWSUP INIT_VM_FLAG(GROWSUP) +#define VMA_GROWSUP mk_vma_flags(VMA_GROWSUP_BIT)#elif defined(CONFIG_SPARC64 )#define VM_SPARC_ADI INIT_VM_FLAG(SPARC_ADI)#define VM_ARCH_CLEAR INIT_VM_FLAG(ARCH_CLEAR)@@ -483,6 +484,7 @@ enum {#endif #ifndef VM_GROWSUP #define VM_GROWSUP VM_NONE +#define VMA_GROWSUP EMPTY_VMA_FLAGS#endif #ifdef CONFIG_ARM64_MTE #define VM_MTE INIT_VM_FLAG(MTE)@@ -1563,11 +1565,24 @@ static inline bool vma_is_initial_stack(const struct vm_area_struct *vma) vma->vm_end >= vma->vm_mm->start_stack; } -static inline bool vma_is_temporary_stack (const struct vm_area_struct *vma) +static inline bool vma_flags_can_grow (const vma_flags_t *flags) { -int maybe_stack = vma->vm_flags & (VM_GROWSDOWN |VM_GROWSUP ); +if (vma_flags_test_single_mask (flags,VMA_GROWSUP )) +return true;include/linux/mm.h static inline bool vma_is_initial_stack(const struct vm_area_struct *vma) · line 1580
Quoted 10 lines
include/linux/mm.h +8
-1+if (vma_flags_test (flags,VMA_GROWSDOWN_BIT )) +return true; + +return false; +} -if (!maybe_stack) +static inline bool vma_can_grow (const struct vm_area_struct *vma) +{ +return vma_flags_can_grow (&vma->flags);Would it save
vma_flags_can_grow()if we do below?
Quoted 7 lines
Quoted 3 lines
return
vma_test(vma, VMA_GROWSDOWN_BIT)||vma_test_single_mask(vma, VMA_GROWSUP);I find these two functions when I am reading
mm.h.Yeah but we require
vma_flags_can_grow()for code inmmap.c, the majority of checks of this have onlyvma_flags_tto work with not a VMA :)
Got it. I think I need to finish this series. :)
Quoted 31 lines
Quoted 31 lines
Quoted 31 lines
include/linux/mm.h +5
-0+} + +static inline bool vma_is_temporary_stack (const struct vm_area_struct *vma) +{ +if (!vma_can_grow (vma))return false; if ((vma->vm_flags & VM_STACK_INCOMPLETE_SETUP) ==tools/testing/vma/include/dup.h +4
-0@@ -245,8 +245,10 @@ enum {#define VM_STACK INIT_VM_FLAG(STACK)#ifdef CONFIG_STACK_GROWS_UP #define VM_STACK_EARLY INIT_VM_FLAG(STACK_EARLY) +#define VMA_STACK_EARLY mk_vma_flags(VMA_STACK_EARLY_BIT)#else #define VM_STACK_EARLY VM_NONE +#define VMA_STACK_EARLY EMPTY_VMA_FLAGS#endif #ifdef CONFIG_ARCH_HAS_PKEYS #define VM_PKEY_SHIFT ((__force int)VMA_HIGH_ARCH_0_BIT)@@ -315,6 +317,8 @@ enum {/* Bits set in the VMA until the stack is in its final location */ #define VM_STACK_INCOMPLETE_SETUP (VM_RAND_READ | VM_SEQ_READ | VM_STACK_EARLY) +#define VMA_STACK_INCOMPLETE_SETUP append_vma_flags( \ + VMA_STACK_EARLY, VMA_RAND_READ_BIT, VMA_SEQ_READ_BIT)
Quoted 8 lines
Quoted 5 lines
tools/testing/vma/include/dup.h enum { · line 324
Quoted 2 lines
tools/testing/vma/include/dup.h +0
-0#define TASK_EXEC_BIT ((current->personality & READ_IMPLIES_EXEC) ? \ VM_EXEC_BIT : VM_READ_BIT)Why are
VMA_STACK_EARLYandVMA_STACK_INCOMPLETE_SETUPadded here but not inmm.h?Yeah urgh oops my bad. It doesn't really break anything but I'll fix it if a respin is needed...
Sure.
Signature · 2 lines
Best Regards, Yan, Zi
Lance Yang
reviewerRe: [PATCH 12/13] mm/mprotect: convert mprotect code to use vma_flags_t
In reply to Lorenzo StoakesQuoted 68 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mprotect logic.Note that we retain the legacy
vm_flags_tbit shifting code indo_mprotect_key(), deferring avma_flags_tapproach to this for the time being.Additionally update comments to reflect the changes to be consistent.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
mm/mprotect.c| 16 ++++++++--------1 file changed, 8 insertions(+), 8 deletions(-)mm/mprotect.c +6
-6@@ -40,7 +40,7 @@static bool maybe_change_pte_writable (struct vm_area_struct *vma,pte_t pte) { -if (WARN_ON_ONCE (!(vma->vm_flags &VM_WRITE ))) +if (WARN_ON_ONCE (!vma_test (vma,VMA_WRITE_BIT )))return false;/* Don't touch entries that are not even readable. */ @@ -97,7 +97,7 @@ static bool can_change_shared_pte_writable(struct vm_area_struct *vma,bool can_change_pte_writable (struct vm_area_struct *vma,unsigned long addr,pte_t pte) { -if (!(vma->vm_flags &VM_SHARED )) +if (!vma_test (vma,VMA_SHARED_BIT ))return can_change_private_pte_writable (vma, addr, pte);return can_change_shared_pte_writable (vma, pte);@@ -194,7 +194,7 @@ static __always_inline void set_write_prot_commit_flush_ptes(struct vm_area_stru {bool set_write; -if (vma->vm_flags &VM_SHARED ) { +if (vma_test (vma,VMA_SHARED_BIT )) { set_write =can_change_shared_pte_writable (vma, ptent);prot_commit_flush_ptes (vma, addr, ptep, oldpte, ptent, nr_ptes,/* idx = */ 0 , set_write, tlb);@@ -811,8 +811,8 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,vm_unacct_memory (nrpages);/* -* Private VM_LOCKED VMA becoming writable: trigger COW to avoid major -* fault on access. +* Private VMA_LOCKED_BIT VMA becoming writable: trigger COW to avoid +* major fault on access. */ if (vma_flags_test(&new_vma_flags, VMA_WRITE_BIT) && vma_flags_test(&old_vma_flags, VMA_LOCKED_BIT) &&@@ -886,7 +886,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, goto out; start = vma->vm_start; error = -EINVAL ; -if (!(vma->vm_flags &VM_GROWSDOWN )) +if (!vma_test (vma,VMA_GROWSDOWN_BIT )) goto out; }else {if (vma->vm_start > start)
mm/mprotect.c static int do_mprotect_pkey(unsigned long start, size_t len, · line 897
Quoted 5 lines
mm/mprotect.c +1
-1@@ -894,7 +894,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,if (unlikely (grows &PROT_GROWSUP )) { end = vma->vm_end; error = -EINVAL ; -if (!(vma->vm_flags &VM_GROWSUP )) +if (!vma_test (vma,VMA_GROWSUP_BIT ))
IIUC, should this be
if (!vma_test_single_mask(vma, VMA_GROWSUP))
instead?
#elif defined(CONFIG_PARISC)
#define VM_GROWSUP INIT_VM_FLAG(GROWSUP)
...
#ifndef VM_GROWSUP
#define VM_GROWSUP VM_NONE
...
VM_GROWSUP is only defined as GROWSUP on parisc and becomes VM_NONE elsewhere. But VMA_GROWSUP_BIT is the raw ARCH_1 bit, which is also used for other arch-specific VMA flags:
DECLARE_VMA_BIT_ALIAS(SAO, ARCH_1), /* Strong Access Ordering (powerpc) */ DECLARE_VMA_BIT_ALIAS(GROWSUP, ARCH_1), /* parisc */ DECLARE_VMA_BIT_ALIAS(SPARC_ADI, ARCH_1), /* sparc64 */ DECLARE_VMA_BIT_ALIAS(ARM64_BTI, ARCH_1), /* arm64 */ DECLARE_VMA_BIT_ALIAS(ARCH_CLEAR, ARCH_1), /* sparc64, arm64 */ DECLARE_VMA_BIT_ALIAS(MAPPED_COPY, ARCH_1), /* !CONFIG_MMU */
Other vma_test() changes look fine to me: just fixed INIT_VM_FLAG() masks matching their VMA_*_BIT :)
Cheers, Lance
Quoted 16 lines
mm/mprotect.c +1
-1goto out; } }@@ -918,7 +918,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, }/* Does the application expect PROT_READ to imply PROT_EXEC */ -if (rier && (vma->vm_flags &VM_MAYEXEC )) +if (rier &&vma_test (vma,VMA_MAYEXEC_BIT )) prot |=PROT_EXEC ;/* Signature · 1 line
2.54.0
Patchwork
CI✗ Fi.CI.BUILD: failure for convert more vm_flags_t users to vma_flags_t
In reply to Lorenzo Stoakes== Series Details ==
== Series Details ==
Series: convert more vm_flags_t users to vma_flags_t
URL : https://patchwork.freedesktop.org/series/169610/
State : failure
== Summary ==
Error: patch https://patchwork.freedesktop.org/api/1.0/series/169610/revisions/1/mbox/ not applied
Applying: mm: introduce vma_flags_can_grow() and vma_can_grow()
Applying: mm/vma: update do_mmap() to use vma_flags_t
Applying: mm: convert __get_unmapped_area() to use vma_flags_t
Applying: mm: update generic_get_unmapped_area[_topdown]() to use vma_flags_t
Applying: mm: prefer mm->def_vma_flags in mm logic
error: sha1 information is lacking or useless (mm/debug.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0005 mm: prefer mm->def_vma_flags in mm logic
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Build failed, no error log produced
Re: [PATCH 01/13] mm: introduce vma_flags_can_grow() and vma_can_grow()
In reply to Lorenzo StoakesQuoted 8 lines · Lorenzo Stoakes
These test whether the VMA has stack sematics, i.e. is able to grow upwards or downwards depending on the architecture.
In order to account for arches which do not support upward-growing stacks, introduce
VMA_GROWSUPwhose definition depends on the architecture supporting it, and usevma_flags_test_single_mask()invma_flags_can_grow()to account for this.
Quoted 6 lines
Update the VMA userland tests to reflect the changes
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
Nice cleanup! Feel free to add:
Re: [PATCH 02/13] mm/vma: update do_mmap() to use vma_flags_t
In reply to Lorenzo StoakesQuoted 19 lines · Lorenzo Stoakes
The core
do_mmap()function accepts avm_flags_tparameter which it then manipulates before passing tommap_region()to do the heavy lifting of the memory mapping.Update
do_mmap()to instead accept avma_flags_tparameter, and adjust all the logic withindo_mmap()to manipulate this instead.This is as part of the ongoing effort to convert VMA flags from a system word size to a bitmap type which allows us to unrestrict the number of VMA flags, as well as gain control over how VMA flag manipulation occurs.
We do not cascade these changes to all functions which accept
vm_flags_t, but rather usevma_flags_to_legacy()where necessary, specifically deferring convertingcalc_vm_prot_bits(),calc_vm_flag_bits()and__get_unmapped_area()tovma_flags_t.Also utilise the new
vma_flags_can_grow()predicate which correctly handles the case of architectures without upward growing stacks.
Quoted 7 lines
As part of this change, introduce
VMA_SHADOW_STACKso we can correctly handle the case of the shadow stack not being defined.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
Not exactly a small one :) I stared at this patch for a while, hopefully don't miss anythig ...
Just one tiny nit below. Overall, LGTM, feel free to add:
[...]
Quoted 4 lines
mm/mmap.c +0
-0
[...]
Quoted 34 lines
mm/mmap.c +9
-5@@ -488,23 +496,27 @@ unsigned long do_mmap(struct file *file, unsigned long addr,* Check to see if we are violating any seals and update VMA * flags if necessary to avoid future seal violations. */ - err =memfd_check_seals_mmap (file, &vm_flags); + err =memfd_check_seals_mmap (file, &vma_flags);if (err)return (unsigned long )err; }else {switch (flags &MAP_TYPE ) {case MAP_SHARED : -if (vm_flags & (VM_GROWSDOWN |VM_GROWSUP )) +if (vma_flags_can_grow (&vma_flags))return -EINVAL ;/* * Ignore pgoff. */ pgoff =0 ; - vm_flags |=VM_SHARED |VM_MAYSHARE ; +vma_flags_set (&vma_flags,VMA_SHARED_BIT ,VMA_MAYSHARE_BIT );break ; -case MAP_DROPPABLE : -if (VM_DROPPABLE ==VM_NONE ) +case MAP_DROPPABLE : { +vma_flags_t droppable =VMA_DROPPABLE ; + +if (vma_flags_empty (&droppable))return -EOPNOTSUPP ; +vma_flags_set_mask (&vma_flags, droppable); +/* * A locked or stack area makes no sense to be droppable. *
mm/mmap.c unsigned long do_mmap(struct file *file, unsigned long addr, · line 521
Quoted 8 lines
mm/mmap.c +1
-2@@ -515,23 +527,24 @@ unsigned long do_mmap(struct file *file, unsigned long addr,*/ if (flags & (MAP_LOCKED |MAP_HUGETLB ))return -EINVAL ; -if (vm_flags & (VM_GROWSDOWN |VM_GROWSUP )) +if (vma_flags_can_grow (&vma_flags))return -EINVAL ; - vm_flags |=VM_DROPPABLE ;
Old code checked VM_GROWSDOWN|VM_GROWSUP before seting VM_DROPPABLE. New code flips that around. Hmm, shouldn't master, just made me look twice ;)
Maybe keep old order?
Cheers, Lance
Quoted 21 lines
mm/mmap.c +5
-3-/* * If the pages can be dropped, then it doesn't make * sense to reserve them. */ - vm_flags |=VM_NORESERVE ; +vma_flags_set (&vma_flags,VMA_NORESERVE_BIT );/* * Likewise, they're volatile enough that they * shouldn't survive forks or coredumps. */ - vm_flags |=VM_WIPEONFORK |VM_DONTDUMP ; +vma_flags_set (&vma_flags,VMA_WIPEONFORK_BIT , +VMA_DONTDUMP_BIT ); + fallthrough; + }case MAP_PRIVATE :/* * Set pgoff according to addr for anon_vma.
[...]
Re: [PATCH 03/13] mm: convert __get_unmapped_area() to use vma_flags_t
In reply to Lorenzo StoakesQuoted 4 lines · Lorenzo Stoakes
Update
__get_unmapped_area()to be parameterised byvma_flags_trather thanvm_flags_tas part of the effort to move VMA flags from a system word to a bitmap.
Quoted 8 lines
We cascade the changes up to
arch_get_unmapped_area_topdown()andarch_get_unmapped_area(), where, for now, we usevma_flags_to_legacy()in order to propagate the VMA flags.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
Nothing suspicious that I could spot :) Feel free to add:
Re: [PATCH 04/13] mm: update generic_get_unmapped_area[_topdown]() to use vma_flags_t
In reply to Lorenzo StoakesQuoted 5 lines · Lorenzo Stoakes
As part of the changes converting VMA flags from a system word size to a bitmap, extend this change to
generic_get_unmapped_area()andgeneric_get_unmapped_area_topdown(), which also allows us to convertstack_guard_placement()as well.
Quoted 7 lines
We retain
arch_get_unmapped_area()andarch_get_unmapped_area_topdown()as-is for now, usinglegacy_to_vma_flags()as necessary to do so.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
Nothing jumped out at me ;) Feel free to add:
Re: [PATCH 05/13] mm: prefer mm->def_vma_flags in mm logic
In reply to Lorenzo StoakesQuoted 22 lines · Lorenzo Stoakes
Currently
mm->def_flags(of typevm_flags_t) is union'd withmm->def_vma_flags(of typevma_flags_t).As part of the effort to convert
vm_flags_tusage tovma_flags_t(in order to no longer be arbitrarily limited to a system word size for VMA flags), prefermm->def_vma_flagstomm->def_flagsthroughout the mm logic.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
mm/debug.c | 2 +- mm/mlock.c | 13 +++++++------ mm/mmap.c | 11 ++++++----- mm/vma.c | 4 ++-- 4 files changed, 16 insertions(+), 14 deletions(-)mm/debug.c +0
-0
mm/debug.c void dump_mm(const struct mm_struct *mm) · line 230
Quoted 6 lines
mm/debug.c +1
-1@@ -226,7 +226,7 @@ void dump_mm(const struct mm_struct *mm) mm->numa_next_scan, mm->numa_scan_offset, mm->numa_scan_seq, #endifatomic_read (&mm->tlb_flush_pending), - mm->def_flags, &mm->def_flags +vma_flags_to_legacy (mm->def_vma_flags), &mm->def_vma_flags );
While at it, one thing for later: dump_mm() still assumes one-world VMA flags. That works today since vma_flags_t is one word. Maybe worth a BUILD_BUG_ON() here, before that stops being true?
Not a big deal though. Feel free to add:
Quoted 2 lines
mm/debug.c +0
-0}EXPORT_SYMBOL (dump_mm);
[...]
Re: [PATCH 06/13] mm/vma: convert vm_pgprot_modify() to use vma_flags_t and rename
In reply to Lorenzo StoakesQuoted 16 lines · Lorenzo Stoakes
Update
vm_pgprot_modify()to use the new VMA flags typevma_flags_t, and rename tovma_pgprot_modify()accordingly.This is part of the ongoing work to convert
vm_flags_ttovma_flags_t, in order to eliminate the arbitrary limit of the number of bits in a system word on available VMA flags.Update VMA userland tests accordingly, updating
vma_set_page_prot()to no longer inlinevma_pgprot_modify(), rather we can simply definevma_pgprot_modify()as a static inline function and the tests will pick it up fromvma.h.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
[...]
Quoted 5 lines
mm/vma.h +0
-0
mm/vma.h static inline bool vma_wants_manual_pte_write_upgrade(struct vm_area_struct *vma · line 529
Quoted 10 lines
mm/vma.h +4
-2@@ -522,9 +522,11 @@ static inline bool vma_wants_manual_pte_write_upgrade(struct vm_area_struct *vma }#ifdef CONFIG_MMU -static inline pgprot_t vm_pgprot_modify (pgprot_t oldprot,vm_flags_t vm_flags) +static inline pgprot_t vma_pgprot_modify (pgprot_t oldprot,vma_flags_t vma_flags) { -return pgprot_modify (oldprot,vm_get_page_prot (vm_flags)); +const pgprot_t prot =vma_get_page_prot (vma_flags); + +return pgprot_modify (oldprot, prot);
Nit: could this just stay as a single return? something like:
return pgprot_modify(oldprot, vma_get_page_prot(vma_flags));
Otherwise, LGTM, feel free to add:
[...]
Re: [PATCH 07/13] mm/vma: rename vma_get_page_prot to vma_flags_to_page_prot
In reply to Lorenzo StoakesQuoted 10 lines · Lorenzo Stoakes
Having
vma_get_page_prot()refer to VMA flags andvma_set_page_prot()refer to a VMA is confusing.Rename
vma_get_page_prot()tovma_flags_to_page_prot()to resolve this confusion.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
straight rename, no stale callers that I can spot ;) Feel free to add:
Lance Yang
reviewerRe: [PATCH 08/13] mm: introduce vma_get_page_prot() and use it
In reply to Lorenzo StoakesQuoted 21 lines · Lorenzo Stoakes
There's a large number of
vm_get_page_prot(vma->vm_flags)invocations. Make life easier by introducingvma_get_page_prot()parameterised by the VMA.This also makes converting
vm_get_page_prot()tovma_flags_teasier.Also update the userland VMA tests to reflect the change.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
drivers/gpu/drm/drm_gem.c | 2 +- drivers/gpu/drm/drm_gem_dma_helper.c | 2 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_gem.c | 6 +++--- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 12 ++++++------ drivers/gpu/drm/msm/msm_gem.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +- drivers/gpu/drm/omapdrm/omap_gem.c | 6 +++---
Quoted 8 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 2 +- drivers/gpu/drm/tegra/gem.c | 2 +- drivers/gpu/drm/virtio/virtgpu_vram.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 2 +- drivers/gpu/drm/xe/xe_device.c | 2 +- drivers/gpu/drm/xe/xe_mmio_gem.c | 2 +- drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +- drivers/video/fbdev/core/fb_io_fops.c | 2 +-
One missed?
drivers/gpu/drm/panthor/panthor_gem.c still has:
vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
Can use vma_get_page_prot(vma) too.
[...]
Re: [PATCH 09/13] mm/vma: update create_init_stack_vma() to use vma_flags_t
In reply to Lorenzo StoakesQuoted 11 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues increate_init_stack_vma().As part of this change we add
VMA_STACK_EARLYandVMA_STACK_INCOMPLETEvma_flags_tdefines, and slightly reworkcreate_init_stack_vma()for clarity.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
Nothing looks off to me ;) Feel free to add:
Lance Yang
reviewerRe: [PATCH 10/13] mm/vma: convert miscellaneous uses of VMA flags in core mm
In reply to Lorenzo StoakesQuoted 8 lines · Lorenzo Stoakes
Update various uses of legacy flags in
vma.candmmap.cto the newvma_flags_ttype, updating comments alongside them to be consistent.Also update
__install_special_mapping()to rearrange things slightly to accommodate the changes.Signed-off-by Lorenzo Stoakes <[email protected]>---
[...]
Quoted 18 lines
mm/vma.c +3
-2@@ -3417,23 +3417,27 @@ struct vm_area_struct *__install_special_mapping( vm_flags_t vm_flags, void *priv, const struct vm_operations_struct *ops) { - int ret; + vma_flags_t vma_flags = legacy_to_vma_flags(vm_flags);struct vm_area_struct *vma; +int ret; vma =vm_area_alloc (mm); -if (unlikely (vma ==NULL )) +if (unlikely (!vma))return ERR_PTR (-ENOMEM );
mm/vma.c struct vm_area_struct *__install_special_mapping( · line 3433
Quoted 10 lines
mm/vma.c +5
-4-vma_set_range (vma, addr, addr + len,0 ); - vm_flags |=vma_flags_to_legacy (mm->def_vma_flags) |VM_DONTEXPAND ; +vma_flags_set_mask (&vma_flags, mm->def_vma_flags); +vma_flags_set (&vma_flags,VMA_DONTEXPAND_BIT );if (pgtable_supports_soft_dirty ()) - vm_flags |=VM_SOFTDIRTY ; -vm_flags_init (vma, vm_flags & ~VM_LOCKED_MASK ); +vma_flags_set (&vma_flags,VMA_SOFTDIRTY_BIT ); +vma_flags_clear_mask (&vma_flags,VMA_LOCKED_MASK ); + vma->flags = vma_flags;
Maybe worth a vma_flags_init() helper here to mirror vm_flags_init()? With this open-coded, we lose the soft-dirty WARN_ON_ONCE sanity check.
Might be nicer to keep that check in one place ;)
[...]
Re: [PATCH 11/13] mm/mlock: convert mlock code to use vma_flags_t
In reply to Lorenzo StoakesQuoted 9 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mlock logic.Additionally update comments to reflect the changes to be consistent.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
Nothing scary jumped out at me. Just one tiny nit below ;)
[...]
Quoted 9 lines
mm/mlock.c +1
-2@@ -466,24 +466,23 @@ static void mlock_vma_pages_range(struct vm_area_struct *vma,*/ static int mlock_fixup (struct vma_iterator *vmi,struct vm_area_struct *vma,struct vm_area_struct **prev,unsigned long start, -unsigned long end,vm_flags_t newflags) +unsigned long end,vma_flags_t *new_vma_flags) { -vma_flags_t new_vma_flags =legacy_to_vma_flags (newflags);const vma_flags_t old_vma_flags = vma->flags;
mm/mlock.c static void mlock_vma_pages_range(struct vm_area_struct *vma, · line 479
Quoted 10 lines
mm/mlock.c +2
-2struct mm_struct *mm = vma->vm_mm;int nr_pages;int ret =0 ; -if (vma_flags_same_pair (&old_vma_flags, &new_vma_flags) || +if (vma_flags_same_pair (&old_vma_flags, new_vma_flags) ||vma_is_secretmem (vma) || !vma_supports_mlock (vma)) {/* -* Don't set VM_LOCKED or VM_LOCKONFAULT and don't count. +* Don't set VMA_LOCKED_BIT or VM_LOCKONFAULT and don't count.
s/VM_LOCKONFAULT/VMA_LOCKONFAULT_BIT/
Otherwise LGTM. Feel free to add:
Quoted 10 lines
mm/mlock.c +1
-1* For secretmem, don't allow the memory to be unlocked. */ goto out; } - vma =vma_modify_flags (vmi, *prev, vma, start, end, &new_vma_flags); + vma =vma_modify_flags (vmi, *prev, vma, start, end, new_vma_flags);if (IS_ERR (vma)) { ret =PTR_ERR (vma); goto out;
[...]
Lance Yang
reviewerRe: [PATCH 13/13] mm/mremap: convert mremap code to use vma_flags_t
In reply to Lorenzo StoakesQuoted 9 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mremap logic.Additionally update comments to reflect the changes to be consistent.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
The vm_flags_set() cases below spell out vma_start_write(), but the vm_flags_clear() cases don't?
Thanks, Lance
Quoted 153 lines
mm/mremap.c| 38 ++++++++++++++++++++------------------1 file changed, 20 insertions(+), 18 deletions(-)mm/mremap.c +20
-18@@ -68,7 +68,7 @@ struct vma_remap_struct {bool populate_expand;/* mlock()'d expanded, must populate. */ enum mremap_type remap_type;/* expand, shrink, etc. */ bool mmap_locked;/* Is mm currently write-locked? */ -unsigned long charged;/* If VM_ACCOUNT, # pages to account. */ +unsigned long charged;/* If VMA_ACCOUNT_BIT, # pgs to account */ bool vmi_needs_invalidate;/* Is the VMA iterator invalidated? */ };@@ -954,7 +954,7 @@ static unsigned long vrm_set_new_addr(struct vma_remap_struct *vrm)if (vrm->flags &MREMAP_FIXED ) map_flags |=MAP_FIXED ; -if (vma->vm_flags &VM_MAYSHARE ) +if (vma_test (vma,VMA_MAYSHARE_BIT )) map_flags |=MAP_SHARED ; res = get_unmapped_area(vma->vm_file, new_addr, vrm->new_len, pgoff,@@ -976,7 +976,7 @@ static bool vrm_calc_charge(struct vma_remap_struct *vrm) {unsigned long charged; -if (!(vrm->vma->vm_flags &VM_ACCOUNT )) +if (!vma_test (vrm->vma,VMA_ACCOUNT_BIT ))return true;/* @@ -1003,7 +1003,7 @@ static bool vrm_calc_charge(struct vma_remap_struct *vrm)*/ static void vrm_uncharge (struct vma_remap_struct *vrm) { -if (!(vrm->vma->vm_flags &VM_ACCOUNT )) +if (!vma_test (vrm->vma,VMA_ACCOUNT_BIT ))return ;vm_unacct_memory (vrm->charged);@@ -1023,7 +1023,7 @@ static void vrm_stat_account(struct vma_remap_struct *vrm,struct vm_area_struct *vma = vrm->vma;vm_stat_account (mm, vma->vm_flags, pages); -if (vma->vm_flags &VM_LOCKED ) +if (vma_test (vma,VMA_LOCKED_BIT )) mm->locked_vm += pages; }@@ -1167,7 +1167,7 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)* arose, in which case we _do_ wish to unmap the _new_ VMA, which means * we actually _do_ want it be unaccounted. */ -bool accountable_move = (vma->vm_flags &VM_ACCOUNT ) && +bool accountable_move =vma_test (vma,VMA_ACCOUNT_BIT ) && !(vrm->flags &MREMAP_DONTUNMAP );/* @@ -1186,7 +1186,7 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)* portions of the original VMA that remain. */ if (accountable_move) { -vm_flags_clear (vma,VM_ACCOUNT ); +vma_clear_flags (vma,VMA_ACCOUNT_BIT );/* We are about to split vma, so store the start/end. */ vm_start = vma->vm_start; vm_end = vma->vm_end;@@ -1211,8 +1211,8 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)* | | * |-------------| * -* Having cleared VM_ACCOUNT from the whole VMA, after we unmap above -* we'll end up with: +* Having cleared VMA_ACCOUNT_BIT from the whole VMA, after we unmap +* above we'll end up with: * * addr end * | | @@ -1232,13 +1232,15 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)if (vm_start < addr) {struct vm_area_struct *prev =vma_prev (&vmi); -vm_flags_set (prev,VM_ACCOUNT );/* Acquires VMA lock. */ +vma_start_write (prev); +vma_set_flags (prev,VMA_ACCOUNT_BIT ); }if (vm_end > end) {struct vm_area_struct *next =vma_next (&vmi); -vm_flags_set (next,VM_ACCOUNT );/* Acquires VMA lock. */ +vma_start_write (next); +vma_set_flags (next,VMA_ACCOUNT_BIT ); } } }@@ -1321,8 +1323,8 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,unsigned long old_start = vrm->vma->vm_start;unsigned long old_end = vrm->vma->vm_end; -/* We always clear VM_LOCKED[ONFAULT] on the old VMA. */ -vm_flags_clear (vrm->vma,VM_LOCKED_MASK ); +/* We always clear VMA_LOCKED[ONFAULT]_BIT on the old VMA. */ +vma_clear_flags_mask (vrm->vma,VMA_LOCKED_MASK );/* * anon_vma links of the old vma is no longer needed after its page @@ -1758,14 +1760,14 @@ static int check_prep_vma(struct vma_remap_struct *vrm)* based on the original. There are no known use cases for this * behavior. As a result, fail such attempts. */ -if (!old_len && !(vma->vm_flags & (VM_SHARED |VM_MAYSHARE ))) { +if (!old_len && !vma_test_any (vma,VMA_SHARED_BIT ,VMA_MAYSHARE_BIT )) {pr_warn_once ("%s (%d): attempted to duplicate a private mapping with mremap. This is not supported.\n" , current->comm, current->pid);return -EINVAL ; }if ((vrm->flags &MREMAP_DONTUNMAP ) && - (vma->vm_flags & (VM_DONTEXPAND |VM_PFNMAP ))) +vma_test_any (vma,VMA_DONTEXPAND_BIT ,VMA_PFNMAP_BIT ))return -EINVAL ;/* @@ -1795,7 +1797,7 @@ static int check_prep_vma(struct vma_remap_struct *vrm)return 0 ;/* We are expanding and the VMA is mlock()'d so we need to populate. */ -if (vma->vm_flags &VM_LOCKED ) +if (vma_test (vma,VMA_LOCKED_BIT )) vrm->populate_expand = true;/* Need to be careful about a growing mapping */ @@ -1803,10 +1805,10 @@ static int check_prep_vma(struct vma_remap_struct *vrm)if (pgoff + (new_len >>PAGE_SHIFT ) < pgoff)return -EINVAL ; -if (vma->vm_flags & (VM_DONTEXPAND |VM_PFNMAP )) +if (vma_test_any (vma,VMA_DONTEXPAND_BIT ,VMA_PFNMAP_BIT ))return -EFAULT ; -if (!mlock_future_ok (mm, vma->vm_flags &VM_LOCKED , vrm->delta)) +if (!mlock_future_ok (mm,vma_test (vma,VMA_LOCKED_BIT ), vrm->delta))return -EAGAIN ;if (!may_expand_vm (mm, &vma->flags, vrm->delta >>PAGE_SHIFT ))Signature · 1 line
2.54.0
Lorenzo Stoakes
maintainerRe: [PATCH 02/13] mm/vma: update do_mmap() to use vma_flags_t
In reply to Lance YangQuoted 24 lines · Lance Yang
Quoted 22 lines · Lorenzo Stoakes
The core
do_mmap()function accepts avm_flags_tparameter which it then manipulates before passing tommap_region()to do the heavy lifting of the memory mapping.Update
do_mmap()to instead accept avma_flags_tparameter, and adjust all the logic withindo_mmap()to manipulate this instead.This is as part of the ongoing effort to convert VMA flags from a system word size to a bitmap type which allows us to unrestrict the number of VMA flags, as well as gain control over how VMA flag manipulation occurs.
We do not cascade these changes to all functions which accept
vm_flags_t, but rather usevma_flags_to_legacy()where necessary, specifically deferring convertingcalc_vm_prot_bits(),calc_vm_flag_bits()and__get_unmapped_area()tovma_flags_t.Also utilise the new
vma_flags_can_grow()predicate which correctly handles the case of architectures without upward growing stacks.As part of this change, introduce
VMA_SHADOW_STACKso we can correctly handle the case of the shadow stack not being defined.
Quoted 7 lines
Quoted 4 lines
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
Not exactly a small one :) I stared at this patch for a while, hopefully don't miss anythig ...
Yeah sorry maybe I could have broken this down more!
Quoted 4 lines
Just one tiny nit below. Overall, LGTM, feel free to add:
Reviewed-by Lance Yang <[email protected]>
Thanks!
Quoted 48 lines
[...]
Quoted 4 lines
mm/mmap.c +0
-0[...]
Quoted 41 lines
mm/mmap.c +10
-6@@ -488,23 +496,27 @@ unsigned long do_mmap(struct file *file, unsigned long addr,* Check to see if we are violating any seals and update VMA * flags if necessary to avoid future seal violations. */ - err =memfd_check_seals_mmap (file, &vm_flags); + err =memfd_check_seals_mmap (file, &vma_flags);if (err)return (unsigned long )err; }else {switch (flags &MAP_TYPE ) {case MAP_SHARED : -if (vm_flags & (VM_GROWSDOWN |VM_GROWSUP )) +if (vma_flags_can_grow (&vma_flags))return -EINVAL ;/* * Ignore pgoff. */ pgoff =0 ; - vm_flags |=VM_SHARED |VM_MAYSHARE ; +vma_flags_set (&vma_flags,VMA_SHARED_BIT ,VMA_MAYSHARE_BIT );break ; -case MAP_DROPPABLE : -if (VM_DROPPABLE ==VM_NONE ) +case MAP_DROPPABLE : { +vma_flags_t droppable =VMA_DROPPABLE ; + +if (vma_flags_empty (&droppable))return -EOPNOTSUPP ; +vma_flags_set_mask (&vma_flags, droppable); +/* * A locked or stack area makes no sense to be droppable. * @@ -515,23 +527,24 @@ unsigned long do_mmap(struct file *file, unsigned long addr,*/ if (flags & (MAP_LOCKED |MAP_HUGETLB ))return -EINVAL ; -if (vm_flags & (VM_GROWSDOWN |VM_GROWSUP )) +if (vma_flags_can_grow (&vma_flags))return -EINVAL ;
Quoted 6 lines
mm/mmap.c unsigned long do_mmap(struct file *file, unsigned long addr, · line 521
Quoted 1 line
mm/mmap.c +0
-1- vm_flags |=VM_DROPPABLE ;Old code checked VM_GROWSDOWN|VM_GROWSUP before seting
VM_DROPPABLE. New code flips that around. Hmm, shouldn't master, just made me look twice ;)Maybe keep old order?
I guess I feared that defining droppable above then referencing it below would be less clear?
Can move if you feel strongly about it, and sorry for making the move at the same time as the general vm_flags_t -> vma_flags_t refactor as it does make that less clear...
Quoted 11 lines
Cheers, Lance
Quoted 8 lines
mm/mmap.c +1
-2-/* * If the pages can be dropped, then it doesn't make * sense to reserve them. */ - vm_flags |=VM_NORESERVE ; +vma_flags_set (&vma_flags,VMA_NORESERVE_BIT );
Quoted 14 lines
Quoted 13 lines
mm/mmap.c +4
-1/* * Likewise, they're volatile enough that they * shouldn't survive forks or coredumps. */ - vm_flags |=VM_WIPEONFORK |VM_DONTDUMP ; +vma_flags_set (&vma_flags,VMA_WIPEONFORK_BIT , +VMA_DONTDUMP_BIT ); + fallthrough; + }case MAP_PRIVATE :/* * Set pgoff according to addr for anon_vma. [...]
Thanks, Lorenzo
Lance Yang
reviewerRe: [PATCH 02/13] mm/vma: update do_mmap() to use vma_flags_t
Quoted 1 line · Lorenzo Stoakes
On Thu, Jul 02, 2026 at 07:15:31PM +0800, Lance Yang wrote:
[...]
Quoted 53 lines
Quoted 53 lines
[...]
Quoted 4 lines
mm/mmap.c +0
-0[...]
Quoted 34 lines
mm/mmap.c +9
-5@@ -488,23 +496,27 @@ unsigned long do_mmap(struct file *file, unsigned long addr,* Check to see if we are violating any seals and update VMA * flags if necessary to avoid future seal violations. */ - err =memfd_check_seals_mmap (file, &vm_flags); + err =memfd_check_seals_mmap (file, &vma_flags);if (err)return (unsigned long )err; }else {switch (flags &MAP_TYPE ) {case MAP_SHARED : -if (vm_flags & (VM_GROWSDOWN |VM_GROWSUP )) +if (vma_flags_can_grow (&vma_flags))return -EINVAL ;/* * Ignore pgoff. */ pgoff =0 ; - vm_flags |=VM_SHARED |VM_MAYSHARE ; +vma_flags_set (&vma_flags,VMA_SHARED_BIT ,VMA_MAYSHARE_BIT );break ; -case MAP_DROPPABLE : -if (VM_DROPPABLE ==VM_NONE ) +case MAP_DROPPABLE : { +vma_flags_t droppable =VMA_DROPPABLE ; + +if (vma_flags_empty (&droppable))return -EOPNOTSUPP ; +vma_flags_set_mask (&vma_flags, droppable); +/* * A locked or stack area makes no sense to be droppable. * mm/mmap.c unsigned long do_mmap(struct file *file, unsigned long addr, · line 521
Quoted 8 lines
mm/mmap.c +1
-2@@ -515,23 +527,24 @@ unsigned long do_mmap(struct file *file, unsigned long addr,*/ if (flags & (MAP_LOCKED |MAP_HUGETLB ))return -EINVAL ; -if (vm_flags & (VM_GROWSDOWN |VM_GROWSUP )) +if (vma_flags_can_grow (&vma_flags))return -EINVAL ; - vm_flags |=VM_DROPPABLE ;Old code checked VM_GROWSDOWN|VM_GROWSUP before seting
VM_DROPPABLE. New code flips that around. Hmm, shouldn't master, just made me look twice ;)
Quoted 8 lines
Quoted 1 line
Maybe keep old order?
I guess I feared that defining droppable above then referencing it below would be less clear?
Can move if you feel strongly about it, and sorry for making the move at the same time as the general
vm_flags_t->vma_flags_trefactor as it does make that less clear...
No need to churn just for me. Thanks :)
Lorenzo Stoakes
maintainerRe: [PATCH 05/13] mm: prefer mm->def_vma_flags in mm logic
In reply to Lance YangQuoted 19 lines · Lance Yang
Quoted 17 lines · Lorenzo Stoakes
Currently
mm->def_flags(of typevm_flags_t) is union'd withmm->def_vma_flags(of typevma_flags_t).As part of the effort to convert
vm_flags_tusage tovma_flags_t(in order to no longer be arbitrarily limited to a system word size for VMA flags), prefermm->def_vma_flagstomm->def_flagsthroughout the mm logic.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
mm/debug.c | 2 +- mm/mlock.c | 13 +++++++------ mm/mmap.c | 11 ++++++----- mm/vma.c | 4 ++-- 4 files changed, 16 insertions(+), 14 deletions(-)
Quoted 15 lines
mm/debug.c void dump_mm(const struct mm_struct *mm) · line 230
Quoted 11 lines
mm/debug.c +1
-1@@ -226,7 +226,7 @@ void dump_mm(const struct mm_struct *mm) mm->numa_next_scan, mm->numa_scan_offset, mm->numa_scan_seq, #endifatomic_read (&mm->tlb_flush_pending), - mm->def_flags, &mm->def_flags +vma_flags_to_legacy (mm->def_vma_flags), &mm->def_vma_flags );While at it, one thing for later:
dump_mm()still assumes one-world VMA flags. That works today sincevma_flags_tis one word. Maybe worth aBUILD_BUG_ON()here, before that stops being true?
Ah yeah that could actually be pretty straightforward, if you see
https://www.kernel.org/doc/Documentation/printk-formats.txt there is a bitmap format that could be used.
Can do that on a respin!
Quoted 8 lines
Not a big deal though. Feel free to add:
Reviewed-by Lance Yang <[email protected]>Quoted 2 lines
mm/debug.c +0
-0}EXPORT_SYMBOL (dump_mm);[...]
Cheers, Lorenzo
Lorenzo Stoakes
maintainerRe: [PATCH 06/13] mm/vma: convert vm_pgprot_modify() to use vma_flags_t and rename
In reply to Lance YangQuoted 37 lines · Lance Yang
Quoted 16 lines · Lorenzo Stoakes
Update
vm_pgprot_modify()to use the new VMA flags typevma_flags_t, and rename tovma_pgprot_modify()accordingly.This is part of the ongoing work to convert
vm_flags_ttovma_flags_t, in order to eliminate the arbitrary limit of the number of bits in a system word on available VMA flags.Update VMA userland tests accordingly, updating
vma_set_page_prot()to no longer inlinevma_pgprot_modify(), rather we can simply definevma_pgprot_modify()as a static inline function and the tests will pick it up fromvma.h.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
[...]
Quoted 5 lines
mm/vma.h +0
-0mm/vma.h static inline bool vma_wants_manual_pte_write_upgrade(struct vm_area_struct *vma · line 529
Quoted 10 lines
mm/vma.h +4
-2@@ -522,9 +522,11 @@ static inline bool vma_wants_manual_pte_write_upgrade(struct vm_area_struct *vma }#ifdef CONFIG_MMU -static inline pgprot_t vm_pgprot_modify (pgprot_t oldprot,vm_flags_t vm_flags) +static inline pgprot_t vma_pgprot_modify (pgprot_t oldprot,vma_flags_t vma_flags) { -return pgprot_modify (oldprot,vm_get_page_prot (vm_flags)); +const pgprot_t prot =vma_get_page_prot (vma_flags); + +return pgprot_modify (oldprot, prot);Nit: could this just stay as a single return? something like:
Quoted 1 line
return
pgprot_modify(oldprot, vma_get_page_prot(vma_flags));
I feel it's a bit clearer this way, separating out the two, I know it's a super tiny difference but anyway :P
Quoted 4 lines
Otherwise, LGTM, feel free to add:
Reviewed-by Lance Yang <[email protected]>
Thanks!
Quoted 2 lines
[...]
Cheers, Lorenzo
Lorenzo Stoakes
maintainerRe: [PATCH 08/13] mm: introduce vma_get_page_prot() and use it
In reply to Lance YangQuoted 32 lines · Lance Yang
Quoted 21 lines · Lorenzo Stoakes
There's a large number of
vm_get_page_prot(vma->vm_flags)invocations. Make life easier by introducingvma_get_page_prot()parameterised by the VMA.This also makes converting
vm_get_page_prot()tovma_flags_teasier.Also update the userland VMA tests to reflect the change.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
drivers/gpu/drm/drm_gem.c | 2 +- drivers/gpu/drm/drm_gem_dma_helper.c | 2 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_gem.c | 6 +++--- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 12 ++++++------ drivers/gpu/drm/msm/msm_gem.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +- drivers/gpu/drm/omapdrm/omap_gem.c | 6 +++---Quoted 8 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 2 +- drivers/gpu/drm/tegra/gem.c | 2 +- drivers/gpu/drm/virtio/virtgpu_vram.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 2 +- drivers/gpu/drm/xe/xe_device.c | 2 +- drivers/gpu/drm/xe/xe_mmio_gem.c | 2 +- drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +- drivers/video/fbdev/core/fb_io_fops.c | 2 +-
Quoted 7 lines
One missed?
drivers/gpu/drm/panthor/panthor_gem.cstill has:vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);Can use
vma_get_page_prot(vma)too.
Oops! Will respin and update. Good spot!
Quoted 2 lines
[...]
Thanks, Lorenzo
Lorenzo Stoakes
maintainerRe: [PATCH 10/13] mm/vma: convert miscellaneous uses of VMA flags in core mm
In reply to Lance YangQuoted 43 lines · Lance Yang
Quoted 8 lines · Lorenzo Stoakes
Update various uses of legacy flags in
vma.candmmap.cto the newvma_flags_ttype, updating comments alongside them to be consistent.Also update
__install_special_mapping()to rearrange things slightly to accommodate the changes.Signed-off-by Lorenzo Stoakes <[email protected]>---
[...]
Quoted 18 lines
mm/vma.c +3
-2@@ -3417,23 +3417,27 @@ struct vm_area_struct *__install_special_mapping( vm_flags_t vm_flags, void *priv, const struct vm_operations_struct *ops) { - int ret; + vma_flags_t vma_flags = legacy_to_vma_flags(vm_flags);struct vm_area_struct *vma; +int ret; vma =vm_area_alloc (mm); -if (unlikely (vma ==NULL )) +if (unlikely (!vma))return ERR_PTR (-ENOMEM );mm/vma.c struct vm_area_struct *__install_special_mapping( · line 3433
Quoted 10 lines
mm/vma.c +5
-4-vma_set_range (vma, addr, addr + len,0 ); - vm_flags |=vma_flags_to_legacy (mm->def_vma_flags) |VM_DONTEXPAND ; +vma_flags_set_mask (&vma_flags, mm->def_vma_flags); +vma_flags_set (&vma_flags,VMA_DONTEXPAND_BIT );if (pgtable_supports_soft_dirty ()) - vm_flags |=VM_SOFTDIRTY ; -vm_flags_init (vma, vm_flags & ~VM_LOCKED_MASK ); +vma_flags_set (&vma_flags,VMA_SOFTDIRTY_BIT ); +vma_flags_clear_mask (&vma_flags,VMA_LOCKED_MASK ); + vma->flags = vma_flags;Maybe worth a
vma_flags_init()helper here to mirrorvm_flags_init()? With this open-coded, we lose the soft-dirtyWARN_ON_ONCEsanity check.
Quoted 1 line
Might be nicer to keep that check in one place ;)
I really hate all the VMA flag accessors, they conflate things horribly - we should be explicitly taking VMA write locks when we need to (and often killable ones actually) not assuming that a VMA flags accessor does (they should at most assert).
This case is even more terribly egregious - you are setting flags at an arbitrary time, why are we asserting something about softdirty?
You may update them as part of initialisation, maybe not. It's far from a guarantee and feels like a lazy place to put it.
BUT obviously it's an oversight not to open code that here, so I'll update the patch to do that!
I want VMA flags to be a clean stateless thing, other than the flags themselves. Implicit, unrelated, asserts or lock acquisitions in general should be done separately IMO.
Quoted 2 lines
[...]
Thanks, Lorenzo
Lorenzo Stoakes
maintainerRe: [PATCH 11/13] mm/mlock: convert mlock code to use vma_flags_t
In reply to Lance YangQuoted 28 lines · Lance Yang
Quoted 9 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mlock logic.Additionally update comments to reflect the changes to be consistent.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
Nothing scary jumped out at me. Just one tiny nit below ;)
[...]
Quoted 13 lines
mm/mlock.c +1
-2@@ -466,24 +466,23 @@ static void mlock_vma_pages_range(struct vm_area_struct *vma,*/ static int mlock_fixup (struct vma_iterator *vmi,struct vm_area_struct *vma,struct vm_area_struct **prev,unsigned long start, -unsigned long end,vm_flags_t newflags) +unsigned long end,vma_flags_t *new_vma_flags) { -vma_flags_t new_vma_flags =legacy_to_vma_flags (newflags);const vma_flags_t old_vma_flags = vma->flags;struct mm_struct *mm = vma->vm_mm;int nr_pages;int ret =0 ;
Quoted 8 lines
mm/mlock.c static void mlock_vma_pages_range(struct vm_area_struct *vma, · line 479
Quoted 6 lines
mm/mlock.c +2
-2-if (vma_flags_same_pair (&old_vma_flags, &new_vma_flags) || +if (vma_flags_same_pair (&old_vma_flags, new_vma_flags) ||vma_is_secretmem (vma) || !vma_supports_mlock (vma)) {/* -* Don't set VM_LOCKED or VM_LOCKONFAULT and don't count. +* Don't set VMA_LOCKED_BIT or VM_LOCKONFAULT and don't count. s/VM_LOCKONFAULT/VMA_LOCKONFAULT_BIT/
Ah yeah oops, will fix and respin! Good spot [and claude missed it ugh] :)
Quoted 10 lines
Otherwise LGTM. Feel free to add:
Reviewed-by Lance Yang <[email protected]>Quoted 5 lines
mm/mlock.c +0
-0* For secretmem, don't allow the memory to be unlocked. */ goto out; }
Quoted 6 lines
Quoted 5 lines
mm/mlock.c +1
-1- vma =vma_modify_flags (vmi, *prev, vma, start, end, &new_vma_flags); + vma =vma_modify_flags (vmi, *prev, vma, start, end, new_vma_flags);if (IS_ERR (vma)) { ret =PTR_ERR (vma); goto out;[...]
Thanks, Lorenzo
Lorenzo Stoakes
maintainerRe: [PATCH 12/13] mm/mprotect: convert mprotect code to use vma_flags_t
In reply to Lance YangQuoted 100 lines · Lance Yang
Quoted 68 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mprotect logic.Note that we retain the legacy
vm_flags_tbit shifting code indo_mprotect_key(), deferring avma_flags_tapproach to this for the time being.Additionally update comments to reflect the changes to be consistent.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
mm/mprotect.c| 16 ++++++++--------1 file changed, 8 insertions(+), 8 deletions(-)mm/mprotect.c +6
-6@@ -40,7 +40,7 @@static bool maybe_change_pte_writable (struct vm_area_struct *vma,pte_t pte) { -if (WARN_ON_ONCE (!(vma->vm_flags &VM_WRITE ))) +if (WARN_ON_ONCE (!vma_test (vma,VMA_WRITE_BIT )))return false;/* Don't touch entries that are not even readable. */ @@ -97,7 +97,7 @@ static bool can_change_shared_pte_writable(struct vm_area_struct *vma,bool can_change_pte_writable (struct vm_area_struct *vma,unsigned long addr,pte_t pte) { -if (!(vma->vm_flags &VM_SHARED )) +if (!vma_test (vma,VMA_SHARED_BIT ))return can_change_private_pte_writable (vma, addr, pte);return can_change_shared_pte_writable (vma, pte);@@ -194,7 +194,7 @@ static __always_inline void set_write_prot_commit_flush_ptes(struct vm_area_stru {bool set_write; -if (vma->vm_flags &VM_SHARED ) { +if (vma_test (vma,VMA_SHARED_BIT )) { set_write =can_change_shared_pte_writable (vma, ptent);prot_commit_flush_ptes (vma, addr, ptep, oldpte, ptent, nr_ptes,/* idx = */ 0 , set_write, tlb);@@ -811,8 +811,8 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,vm_unacct_memory (nrpages);/* -* Private VM_LOCKED VMA becoming writable: trigger COW to avoid major -* fault on access. +* Private VMA_LOCKED_BIT VMA becoming writable: trigger COW to avoid +* major fault on access. */ if (vma_flags_test(&new_vma_flags, VMA_WRITE_BIT) && vma_flags_test(&old_vma_flags, VMA_LOCKED_BIT) &&@@ -886,7 +886,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, goto out; start = vma->vm_start; error = -EINVAL ; -if (!(vma->vm_flags &VM_GROWSDOWN )) +if (!vma_test (vma,VMA_GROWSDOWN_BIT )) goto out; }else {if (vma->vm_start > start)mm/mprotect.c static int do_mprotect_pkey(unsigned long start, size_t len, · line 897
Quoted 5 lines
mm/mprotect.c +1
-1@@ -894,7 +894,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,if (unlikely (grows &PROT_GROWSUP )) { end = vma->vm_end; error = -EINVAL ; -if (!(vma->vm_flags &VM_GROWSUP )) +if (!vma_test (vma,VMA_GROWSUP_BIT ))IIUC, should this be
if (!vma_test_single_mask(vma,
VMA_GROWSUP))instead?
#elif
defined(CONFIG_PARISC)
#defineVM_GROWSUPINIT_VM_FLAG(GROWSUP)
...
#ifndefVM_GROWSUP
#defineVM_GROWSUPVM_NONE
...
VM_GROWSUPis only defined as GROWSUP on parisc and becomesVM_NONEelsewhere. ButVMA_GROWSUP_BITis the rawARCH_1bit, which is also used for other arch-specific VMA flags:DECLARE_VMA_BIT_ALIAS(SAO, ARCH_1), /* Strong Access Ordering (powerpc) */ DECLARE_VMA_BIT_ALIAS(GROWSUP, ARCH_1), /* parisc */ DECLARE_VMA_BIT_ALIAS(SPARC_ADI, ARCH_1), /* sparc64 */ DECLARE_VMA_BIT_ALIAS(ARM64_BTI, ARCH_1), /* arm64 */ DECLARE_VMA_BIT_ALIAS(ARCH_CLEAR, ARCH_1), /* sparc64, arm64 */ DECLARE_VMA_BIT_ALIAS(MAPPED_COPY, ARCH_1), /* !CONFIG_MMU */
Quoted 2 lines
Other
vma_test()changes look fine to me: just fixedINIT_VM_FLAG()masks matching their VMA_*_BIT :)
Thanks you're right, will fix!
Again I swear I ran claude on all of this so it's failing me here :)
Quoted 19 lines
Cheers, Lance
Quoted 16 lines
mm/mprotect.c +1
-2goto out; } }@@ -918,7 +918,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, }/* Does the application expect PROT_READ to imply PROT_EXEC */ -if (rier && (vma->vm_flags &VM_MAYEXEC )) +if (rier &&vma_test (vma,VMA_MAYEXEC_BIT )) prot |=PROT_EXEC ;/* -- 2.54.0
Lorenzo Stoakes
maintainerRe: [PATCH 13/13] mm/mremap: convert mremap code to use vma_flags_t
In reply to Lance YangQuoted 7 lines · Lance Yang
Quoted 5 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mremap logic.Additionally update comments to reflect the changes to be consistent.
Quoted 7 lines
Quoted 4 lines
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
The
vm_flags_set()cases below spell outvma_start_write(), but thevm_flags_clear()cases don't?
Yep as I said elsewhere, implicitly taking the lock is terrible and me doing this is completely on purpose to get rid of that :)
But I haven't been clear enough clearly, so I should put the argument as to why that's ok in the commit message.
Will do so on respin.
Quoted 63 lines
Thanks, Lance
Quoted 60 lines
mm/mremap.c| 38 ++++++++++++++++++++------------------1 file changed, 20 insertions(+), 18 deletions(-)mm/mremap.c +6
-6@@ -68,7 +68,7 @@ struct vma_remap_struct {bool populate_expand;/* mlock()'d expanded, must populate. */ enum mremap_type remap_type;/* expand, shrink, etc. */ bool mmap_locked;/* Is mm currently write-locked? */ -unsigned long charged;/* If VM_ACCOUNT, # pages to account. */ +unsigned long charged;/* If VMA_ACCOUNT_BIT, # pgs to account */ bool vmi_needs_invalidate;/* Is the VMA iterator invalidated? */ };@@ -954,7 +954,7 @@ static unsigned long vrm_set_new_addr(struct vma_remap_struct *vrm)if (vrm->flags &MREMAP_FIXED ) map_flags |=MAP_FIXED ; -if (vma->vm_flags &VM_MAYSHARE ) +if (vma_test (vma,VMA_MAYSHARE_BIT )) map_flags |=MAP_SHARED ; res = get_unmapped_area(vma->vm_file, new_addr, vrm->new_len, pgoff,@@ -976,7 +976,7 @@ static bool vrm_calc_charge(struct vma_remap_struct *vrm) {unsigned long charged; -if (!(vrm->vma->vm_flags &VM_ACCOUNT )) +if (!vma_test (vrm->vma,VMA_ACCOUNT_BIT ))return true;/* @@ -1003,7 +1003,7 @@ static bool vrm_calc_charge(struct vma_remap_struct *vrm)*/ static void vrm_uncharge (struct vma_remap_struct *vrm) { -if (!(vrm->vma->vm_flags &VM_ACCOUNT )) +if (!vma_test (vrm->vma,VMA_ACCOUNT_BIT ))return ;vm_unacct_memory (vrm->charged);@@ -1023,7 +1023,7 @@ static void vrm_stat_account(struct vma_remap_struct *vrm,struct vm_area_struct *vma = vrm->vma;vm_stat_account (mm, vma->vm_flags, pages); -if (vma->vm_flags &VM_LOCKED ) +if (vma_test (vma,VMA_LOCKED_BIT )) mm->locked_vm += pages; }@@ -1167,7 +1167,7 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)* arose, in which case we _do_ wish to unmap the _new_ VMA, which means * we actually _do_ want it be unaccounted. */ -bool accountable_move = (vma->vm_flags &VM_ACCOUNT ) && +bool accountable_move =vma_test (vma,VMA_ACCOUNT_BIT ) && !(vrm->flags &MREMAP_DONTUNMAP );
Quoted 7 lines
mm/mremap.c static void unmap_source_vma(struct vma_remap_struct *vrm) · line 1189
Quoted 7 lines
mm/mremap.c +1
-1/* @@ -1186,7 +1186,7 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)* portions of the original VMA that remain. */ if (accountable_move) { -vm_flags_clear (vma,VM_ACCOUNT ); +vma_clear_flags (vma,VMA_ACCOUNT_BIT );
This is called from move_vma() which holds the VMA write lock on vma.
Quoted 23 lines
Quoted 23 lines
mm/mremap.c +4
-3/* We are about to split vma, so store the start/end. */ vm_start = vma->vm_start; vm_end = vma->vm_end;@@ -1211,8 +1211,8 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)* | | * |-------------| * -* Having cleared VM_ACCOUNT from the whole VMA, after we unmap above -* we'll end up with: +* Having cleared VMA_ACCOUNT_BIT from the whole VMA, after we unmap +* above we'll end up with: * * addr end * | | @@ -1232,13 +1232,15 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)if (vm_start < addr) {struct vm_area_struct *prev =vma_prev (&vmi); -vm_flags_set (prev,VM_ACCOUNT );/* Acquires VMA lock. */ +vma_start_write (prev); +vma_set_flags (prev,VMA_ACCOUNT_BIT ); }
Quoted 6 lines
mm/mremap.c static void unmap_source_vma(struct vma_remap_struct *vrm) · line 1243
Quoted 6 lines
mm/mremap.c +2
-1if (vm_end > end) {struct vm_area_struct *next =vma_next (&vmi); -vm_flags_set (next,VM_ACCOUNT );/* Acquires VMA lock. */ +vma_start_write (next); +vma_set_flags (next,VMA_ACCOUNT_BIT );
These need vma_start_write() as referencing other, unlocked VMAs.
Quoted 11 lines
mm/mremap.c static void dontunmap_complete(struct vma_remap_struct *vrm, · line 1327
Quoted 11 lines
mm/mremap.c +2
-2} } }@@ -1321,8 +1323,8 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,unsigned long old_start = vrm->vma->vm_start;unsigned long old_end = vrm->vma->vm_end; -/* We always clear VM_LOCKED[ONFAULT] on the old VMA. */ -vm_flags_clear (vrm->vma,VM_LOCKED_MASK ); +/* We always clear VMA_LOCKED[ONFAULT]_BIT on the old VMA. */ +vma_clear_flags_mask (vrm->vma,VMA_LOCKED_MASK );
-
Same as above, called from move_vma() with VMA write lock held.
Quoted 41 lines
Quoted 41 lines
mm/mremap.c +5
-5/* * anon_vma links of the old vma is no longer needed after its page @@ -1758,14 +1760,14 @@ static int check_prep_vma(struct vma_remap_struct *vrm)* based on the original. There are no known use cases for this * behavior. As a result, fail such attempts. */ -if (!old_len && !(vma->vm_flags & (VM_SHARED |VM_MAYSHARE ))) { +if (!old_len && !vma_test_any (vma,VMA_SHARED_BIT ,VMA_MAYSHARE_BIT )) {pr_warn_once ("%s (%d): attempted to duplicate a private mapping with mremap. This is not supported.\n" , current->comm, current->pid);return -EINVAL ; }if ((vrm->flags &MREMAP_DONTUNMAP ) && - (vma->vm_flags & (VM_DONTEXPAND |VM_PFNMAP ))) +vma_test_any (vma,VMA_DONTEXPAND_BIT ,VMA_PFNMAP_BIT ))return -EINVAL ;/* @@ -1795,7 +1797,7 @@ static int check_prep_vma(struct vma_remap_struct *vrm)return 0 ;/* We are expanding and the VMA is mlock()'d so we need to populate. */ -if (vma->vm_flags &VM_LOCKED ) +if (vma_test (vma,VMA_LOCKED_BIT )) vrm->populate_expand = true;/* Need to be careful about a growing mapping */ @@ -1803,10 +1805,10 @@ static int check_prep_vma(struct vma_remap_struct *vrm)if (pgoff + (new_len >>PAGE_SHIFT ) < pgoff)return -EINVAL ; -if (vma->vm_flags & (VM_DONTEXPAND |VM_PFNMAP )) +if (vma_test_any (vma,VMA_DONTEXPAND_BIT ,VMA_PFNMAP_BIT ))return -EFAULT ; -if (!mlock_future_ok (mm, vma->vm_flags &VM_LOCKED , vrm->delta)) +if (!mlock_future_ok (mm,vma_test (vma,VMA_LOCKED_BIT ), vrm->delta))return -EAGAIN ;
Quoted 5 lines
Quoted 5 lines
mm/mremap.c +0
-1if (!may_expand_vm (mm, &vma->flags, vrm->delta >>PAGE_SHIFT )) --2.54.0
Cheers, Lorenzo
Lorenzo Stoakes
maintainerRe: ✗ Fi.CI.BUILD: failure for convert more vm_flags_t users to vma_flags_t
In reply to PatchworkQuoted 8 lines · Patchwork
== Series Details ==
Series: convert more
vm_flags_tusers tovma_flags_t
URL : https://patchwork.freedesktop.org/series/169610/
State : failure== Summary ==
Quoted 16 lines
Error: patch https://patchwork.freedesktop.org/api/1.0/series/169610/revisions/1/mbox/ not applied
Applying: mm: introducevma_flags_can_grow()andvma_can_grow()
Applying: mm/vma: updatedo_mmap()to usevma_flags_t
Applying: mm: convert__get_unmapped_area()to usevma_flags_t
Applying: mm: update generic_get_unmapped_area[_topdown]() to usevma_flags_t
Applying: mm: prefermm->def_vma_flagsin mm logic
error: sha1 information is lacking or useless (mm/debug.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0005 mm: prefermm->def_vma_flagsin mm logic
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Build failed, no error log produced
This will likely be because this series has a dependency on the 30 patch monster series I sent which hasn't been merged anyway :)
Lance Yang
reviewerRe: [PATCH 13/13] mm/mremap: convert mremap code to use vma_flags_t
In reply to Lorenzo StoakesQuoted 16 lines · Lorenzo Stoakes
Quoted 7 lines · Lance Yang
Quoted 5 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mremap logic.Additionally update comments to reflect the changes to be consistent.
Quoted 7 lines
Quoted 4 lines
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
The
vm_flags_set()cases below spell outvma_start_write(), but thevm_flags_clear()cases don't?
Quoted 7 lines
Yep as I said elsewhere, implicitly taking the lock is terrible and me doing this is completely on purpose to get rid of that :)
But I haven't been clear enough clearly, so I should put the argument as to why that's ok in the commit message.
Will do so on respin.
Makes sense, thanks for spelling it out! A short changelog note should clear it up for me :D
[...]
Lorenzo Stoakes
maintainerRe: [PATCH 13/13] mm/mremap: convert mremap code to use vma_flags_t
Quoted 22 lines · Lance Yang
Quoted 19 lines · Lorenzo Stoakes
Quoted 7 lines · Lance Yang
Quoted 5 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mremap logic.Additionally update comments to reflect the changes to be consistent.
Quoted 7 lines
Quoted 4 lines
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
The
vm_flags_set()cases below spell outvma_start_write(), but thevm_flags_clear()cases don't?Yep as I said elsewhere, implicitly taking the lock is terrible and me doing this is completely on purpose to get rid of that :)
Quoted 7 lines
Quoted 4 lines
But I haven't been clear enough clearly, so I should put the argument as to why that's ok in the commit message.
Will do so on respin.
Makes sense, thanks for spelling it out! A short changelog note should clear it up for me :D
Yeah absolutely, that's necessary and was an oversight on my part, will correct!
Thanks for your great review here, much appreciated! :)
Quoted 2 lines
[...]
Cheers, Lorenzo
Re: [PATCH 01/13] mm: introduce vma_flags_can_grow() and vma_can_grow()
In reply to Lorenzo StoakesQuoted 10 lines · Lorenzo Stoakes
These test whether the VMA has stack sematics, i.e. is able to grow upwards or downwards depending on the architecture.
In order to account for arches which do not support upward-growing stacks, introduce
VMA_GROWSUPwhose definition depends on the architecture supporting it, and usevma_flags_test_single_mask()invma_flags_can_grow()to account for this.Update the VMA userland tests to reflect the changes
Quoted 8 lines
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- include/linux/mm.h | 21 ++++++++++++++++++--- tools/testing/vma/include/dup.h | 4 ++++ 2 files changed, 22 insertions(+), 3 deletions(-)
Signature · 2 lines
Best Regards, Yan, Zi
Re: [PATCH 02/13] mm/vma: update do_mmap() to use vma_flags_t
In reply to Lorenzo StoakesQuoted 39 lines · Lorenzo Stoakes
The core
do_mmap()function accepts avm_flags_tparameter which it then manipulates before passing tommap_region()to do the heavy lifting of the memory mapping.Update
do_mmap()to instead accept avma_flags_tparameter, and adjust all the logic withindo_mmap()to manipulate this instead.This is as part of the ongoing effort to convert VMA flags from a system word size to a bitmap type which allows us to unrestrict the number of VMA flags, as well as gain control over how VMA flag manipulation occurs.
We do not cascade these changes to all functions which accept
vm_flags_t, but rather usevma_flags_to_legacy()where necessary, specifically deferring convertingcalc_vm_prot_bits(),calc_vm_flag_bits()and__get_unmapped_area()tovma_flags_t.Also utilise the new
vma_flags_can_grow()predicate which correctly handles the case of architectures without upward growing stacks.As part of this change, introduce
VMA_SHADOW_STACKso we can correctly handle the case of the shadow stack not being defined.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- arch/mips/kernel/vdso.c | 4 +-- fs/aio.c | 2 +- include/linux/memfd.h | 6 ++-- include/linux/mm.h | 6 ++-- ipc/shm.c | 3 +- mm/memfd.c | 15 ++++----- mm/mmap.c | 67 ++++++++++++++++++++++++----------------- mm/nommu.c | 3 +- mm/util.c | 10 +++--- mm/vma.c | 7 ++--- mm/vma.h | 2 +- 11 files changed, 69 insertions(+), 56 deletions(-)
<snip>
Quoted 19 lines
mm/memfd.c +3
-6-static int check_write_seal (vm_flags_t *vm_flags_ptr) +static int check_write_seal (vma_flags_t *vma_flags_ptr) { -vm_flags_t vm_flags = *vm_flags_ptr; -vm_flags_t mask = vm_flags & (VM_SHARED |VM_WRITE ); -/* If a private mapping then writability is irrelevant. */ -if (!(mask &VM_SHARED )) +if (!vma_flags_test (vma_flags_ptr,VMA_SHARED_BIT ))return 0 ;/* * New PROT_WRITE and MAP_SHARED mmaps are not allowed when * write seals are active. */ -if (mask &VM_WRITE ) +if (vma_flags_test (vma_flags_ptr,VMA_WRITE_BIT ))return -EPERM ;
mm/memfd.c static inline bool is_write_sealed(unsigned int seals) · line 392
Quoted 10 lines
mm/memfd.c +1
-1/* * This is a read-only mapping, disallow mprotect() from making a * write-sealed mapping writable in future. */ - *vm_flags_ptr &= ~VM_MAYWRITE ; +vma_flags_clear (vma_flags_ptr,VMA_MAYWRITE_BIT );return 0 ; }
This function alone changed its original behavior, since vm_flags is a snapshot of *vm_flags_ptr, but after the change this snapshot is gone. But its only caller memfd_check_seals_mmap() gets vm_flags_ptr from the input parameter of do_mmap(), so the overall behavior does not change.
<snip>
Quoted 11 lines
mm/mmap.c +6
-0+case MAP_DROPPABLE : { +vma_flags_t droppable =VMA_DROPPABLE ; + +if (vma_flags_empty (&droppable))return -EOPNOTSUPP ; +vma_flags_set_mask (&vma_flags, droppable); +/* * A locked or stack area makes no sense to be droppable. *
mm/mmap.c unsigned long do_mmap(struct file *file, unsigned long addr, · line 522
Quoted 9 lines
mm/mmap.c +1
-3@@ -515,23 +527,24 @@ unsigned long do_mmap(struct file *file, unsigned long addr,*/ if (flags & (MAP_LOCKED |MAP_HUGETLB ))return -EINVAL ; -if (vm_flags & (VM_GROWSDOWN |VM_GROWSUP )) +if (vma_flags_can_grow (&vma_flags))return -EINVAL ; - vm_flags |=VM_DROPPABLE ; -
Lance pointed out the reordering of setting VMA_DROPPABLE and checking of can_grow, but these flags are not overlapped and there is no parallel writer to vma_flags. So it is still no functional change, just not mechanical changes. :)
Otherwise, LGTM.
Signature · 2 lines
Best Regards, Yan, Zi
Re: [PATCH 03/13] mm: convert __get_unmapped_area() to use vma_flags_t
In reply to Lorenzo StoakesQuoted 20 lines · Lorenzo Stoakes
Update
__get_unmapped_area()to be parameterised byvma_flags_trather thanvm_flags_tas part of the effort to move VMA flags from a system word to a bitmap.We cascade the changes up to
arch_get_unmapped_area_topdown()andarch_get_unmapped_area(), where, for now, we usevma_flags_to_legacy()in order to propagate the VMA flags.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- fs/hugetlbfs/inode.c | 3 ++- include/linux/huge_mm.h | 10 +++++----- include/linux/mm.h | 6 ++++-- include/linux/sched/mm.h | 12 ++++++------ mm/huge_memory.c | 21 ++++++++++++--------- mm/mmap.c | 27 ++++++++++++++------------- 6 files changed, 43 insertions(+), 36 deletions(-)
<snip>
Quoted 10 lines
include/linux/sched/mm.h +0
-2@@ -193,12 +193,12 @@ unsigned long mm_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags); -unsigned long mm_get_unmapped_area_vmflags(struct file *filp, - unsigned long addr,
include/linux/sched/mm.h unsigned long mm_get_unmapped_area(struct file *filp, unsigned long addr, · line 201
Quoted 10 lines
include/linux/sched/mm.h +6
-4-unsigned long len, - unsigned long pgoff, - unsigned long flags, - vm_flags_t vm_flags); +unsigned long mm_get_unmapped_area_vmaflags (struct file *filp, +unsigned long addr, +unsigned long len, +unsigned long pgoff, +unsigned long flags, +vma_flags_t vma_flags);
Want to use two-tab indentation while at it?
<snip>
include/linux/sched/mm.h arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr, · line 816
Quoted 10 lines
include/linux/sched/mm.h +3
-3@@ -812,19 +811,20 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr, }#endif -unsigned long mm_get_unmapped_area_vmflags (struct file *filp,unsigned long addr, -unsigned long len,unsigned long pgoff, -unsigned long flags,vm_flags_t vm_flags) +unsigned long mm_get_unmapped_area_vmaflags (struct file *filp,unsigned long addr, +unsigned long len,unsigned long pgoff, +unsigned long flags,vma_flags_t vma_flags)
Ditto.
LGTM.
Signature · 2 lines
Best Regards, Yan, Zi
Re: [PATCH 04/13] mm: update generic_get_unmapped_area[_topdown]() to use vma_flags_t
In reply to Lorenzo StoakesQuoted 8 lines · Lorenzo Stoakes
As part of the changes converting VMA flags from a system word size to a bitmap, extend this change to
generic_get_unmapped_area()andgeneric_get_unmapped_area_topdown(), which also allows us to convertstack_guard_placement()as well.We retain
arch_get_unmapped_area()andarch_get_unmapped_area_topdown()as-is for now, usinglegacy_to_vma_flags()as necessary to do so.
Quoted 9 lines
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- arch/powerpc/mm/book3s64/slice.c | 6 ++++-- include/linux/sched/mm.h | 4 ++-- mm/mmap.c | 16 ++++++++-------- 3 files changed, 14 insertions(+), 12 deletions(-)
LGTM.
Signature · 2 lines
Best Regards, Yan, Zi
Lorenzo Stoakes
maintainerRe: [PATCH 02/13] mm/vma: update do_mmap() to use vma_flags_t
In reply to Zi YanQuoted 70 lines · Zi Yan
Quoted 39 lines · Lorenzo Stoakes
The core
do_mmap()function accepts avm_flags_tparameter which it then manipulates before passing tommap_region()to do the heavy lifting of the memory mapping.Update
do_mmap()to instead accept avma_flags_tparameter, and adjust all the logic withindo_mmap()to manipulate this instead.This is as part of the ongoing effort to convert VMA flags from a system word size to a bitmap type which allows us to unrestrict the number of VMA flags, as well as gain control over how VMA flag manipulation occurs.
We do not cascade these changes to all functions which accept
vm_flags_t, but rather usevma_flags_to_legacy()where necessary, specifically deferring convertingcalc_vm_prot_bits(),calc_vm_flag_bits()and__get_unmapped_area()tovma_flags_t.Also utilise the new
vma_flags_can_grow()predicate which correctly handles the case of architectures without upward growing stacks.As part of this change, introduce
VMA_SHADOW_STACKso we can correctly handle the case of the shadow stack not being defined.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- arch/mips/kernel/vdso.c | 4 +-- fs/aio.c | 2 +- include/linux/memfd.h | 6 ++-- include/linux/mm.h | 6 ++-- ipc/shm.c | 3 +- mm/memfd.c | 15 ++++----- mm/mmap.c | 67 ++++++++++++++++++++++++----------------- mm/nommu.c | 3 +- mm/util.c | 10 +++--- mm/vma.c | 7 ++--- mm/vma.h | 2 +- 11 files changed, 69 insertions(+), 56 deletions(-)<snip>
Quoted 27 lines
mm/memfd.c +4
-7-static int check_write_seal (vm_flags_t *vm_flags_ptr) +static int check_write_seal (vma_flags_t *vma_flags_ptr) { -vm_flags_t vm_flags = *vm_flags_ptr; -vm_flags_t mask = vm_flags & (VM_SHARED |VM_WRITE ); -/* If a private mapping then writability is irrelevant. */ -if (!(mask &VM_SHARED )) +if (!vma_flags_test (vma_flags_ptr,VMA_SHARED_BIT ))return 0 ;/* * New PROT_WRITE and MAP_SHARED mmaps are not allowed when * write seals are active. */ -if (mask &VM_WRITE ) +if (vma_flags_test (vma_flags_ptr,VMA_WRITE_BIT ))return -EPERM ;/* * This is a read-only mapping, disallow mprotect() from making a * write-sealed mapping writable in future. */ - *vm_flags_ptr &= ~VM_MAYWRITE ; +vma_flags_clear (vma_flags_ptr,VMA_MAYWRITE_BIT );
Quoted 7 lines
Quoted 2 lines
return 0;
}This function alone changed its original behavior, since
vm_flagsis a snapshot of*vm_flags_ptr, but after the change this snapshot is gone. But its only callermemfd_check_seals_mmap()getsvm_flags_ptrfrom the input parameter ofdo_mmap(), so the overall behavior does not change.
Right yeah, the snapshot was always just a convenience thing :)
Quoted 21 lines
<snip>
Quoted 18 lines
mm/mmap.c +7
-1+case MAP_DROPPABLE : { +vma_flags_t droppable =VMA_DROPPABLE ; + +if (vma_flags_empty (&droppable))return -EOPNOTSUPP ; +vma_flags_set_mask (&vma_flags, droppable); +/* * A locked or stack area makes no sense to be droppable. * @@ -515,23 +527,24 @@ unsigned long do_mmap(struct file *file, unsigned long addr,*/ if (flags & (MAP_LOCKED |MAP_HUGETLB ))return -EINVAL ; -if (vm_flags & (VM_GROWSDOWN |VM_GROWSUP )) +if (vma_flags_can_grow (&vma_flags))return -EINVAL ;
Quoted 7 lines
mm/mmap.c unsigned long do_mmap(struct file *file, unsigned long addr, · line 522
Quoted 2 lines
mm/mmap.c +0
-2- vm_flags |=VM_DROPPABLE ; -Lance pointed out the reordering of setting
VMA_DROPPABLEand checking ofcan_grow, but these flags are not overlapped and there is no parallel writer tovma_flags. So it is still no functional change, just not mechanical changes. :)
Right yes exactly :)
Quoted 4 lines
Otherwise, LGTM.
Reviewed-by Zi Yan <[email protected]>
Thanks!
Quoted 5 lines
--
Best Regards,
Yan, Zi
Cheers, Lorenzo
Lorenzo Stoakes
maintainerRe: [PATCH 03/13] mm: convert __get_unmapped_area() to use vma_flags_t
In reply to Zi YanQuoted 31 lines · Zi Yan
Quoted 20 lines · Lorenzo Stoakes
Update
__get_unmapped_area()to be parameterised byvma_flags_trather thanvm_flags_tas part of the effort to move VMA flags from a system word to a bitmap.We cascade the changes up to
arch_get_unmapped_area_topdown()andarch_get_unmapped_area(), where, for now, we usevma_flags_to_legacy()in order to propagate the VMA flags.No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- fs/hugetlbfs/inode.c | 3 ++- include/linux/huge_mm.h | 10 +++++----- include/linux/mm.h | 6 ++++-- include/linux/sched/mm.h | 12 ++++++------ mm/huge_memory.c | 21 ++++++++++++--------- mm/mmap.c | 27 ++++++++++++++------------- 6 files changed, 43 insertions(+), 36 deletions(-)<snip>
Quoted 8 lines
include/linux/sched/mm.h +0
-0@@ -193,12 +193,12 @@ unsigned long mm_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags);
Quoted 14 lines
include/linux/sched/mm.h unsigned long mm_get_unmapped_area(struct file *filp, unsigned long addr, · line 201
Quoted 12 lines
include/linux/sched/mm.h +6
-6-unsigned long mm_get_unmapped_area_vmflags (struct file *filp, -unsigned long addr, -unsigned long len, -unsigned long pgoff, -unsigned long flags, -vm_flags_t vm_flags); +unsigned long mm_get_unmapped_area_vmaflags (struct file *filp, +unsigned long addr, +unsigned long len, +unsigned long pgoff, +unsigned long flags, +vma_flags_t vma_flags);Want to use two-tab indentation while at it?
Yeah sure will fix on respin!
Quoted 7 lines
<snip>
Quoted 4 lines
include/linux/sched/mm.h +0
-0@@ -812,19 +811,20 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr, }#endif
Quoted 8 lines
include/linux/sched/mm.h arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr, · line 816
Quoted 6 lines
include/linux/sched/mm.h +3
-3-unsigned long mm_get_unmapped_area_vmflags (struct file *filp,unsigned long addr, -unsigned long len,unsigned long pgoff, -unsigned long flags,vm_flags_t vm_flags) +unsigned long mm_get_unmapped_area_vmaflags (struct file *filp,unsigned long addr, +unsigned long len,unsigned long pgoff, +unsigned long flags,vma_flags_t vma_flags)Ditto.
Ack will fix!
Quoted 4 lines
LGTM.
Reviewed-by Zi Yan <[email protected]>
Thanks!
Quoted 6 lines
--
Best Regards,
Yan, Zi
Cheers, Lorenzo
Re: [PATCH 05/13] mm: prefer mm->def_vma_flags in mm logic
In reply to Lorenzo StoakesQuoted 9 lines · Lorenzo Stoakes
Currently
mm->def_flags(of typevm_flags_t) is union'd withmm->def_vma_flags(of typevma_flags_t).As part of the effort to convert
vm_flags_tusage tovma_flags_t(in order to no longer be arbitrarily limited to a system word size for VMA flags), prefermm->def_vma_flagstomm->def_flagsthroughout the mm logic.No functional change intended.
Quoted 8 lines
Signed-off-by Lorenzo Stoakes <[email protected]>--- mm/debug.c | 2 +- mm/mlock.c | 13 +++++++------ mm/mmap.c | 11 ++++++----- mm/vma.c | 4 ++-- 4 files changed, 16 insertions(+), 14 deletions(-)
LGTM.
Signature · 2 lines
Best Regards, Yan, Zi
Re: [PATCH 06/13] mm/vma: convert vm_pgprot_modify() to use vma_flags_t and rename
In reply to Lorenzo StoakesQuoted 14 lines · Lorenzo Stoakes
Update
vm_pgprot_modify()to use the new VMA flags typevma_flags_t, and rename tovma_pgprot_modify()accordingly.This is part of the ongoing work to convert
vm_flags_ttovma_flags_t, in order to eliminate the arbitrary limit of the number of bits in a system word on available VMA flags.Update VMA userland tests accordingly, updating
vma_set_page_prot()to no longer inlinevma_pgprot_modify(), rather we can simply definevma_pgprot_modify()as a static inline function and the tests will pick it up fromvma.h.No functional change intended.
Quoted 8 lines
Signed-off-by Lorenzo Stoakes <[email protected]>--- mm/mmap.c | 8 ++++---- mm/vma.c | 2 +- mm/vma.h | 6 ++++-- tools/testing/vma/include/dup.h | 12 +++++------- 4 files changed, 14 insertions(+), 14 deletions(-)
Signature · 2 lines
Best Regards, Yan, Zi
Re: [PATCH 07/13] mm/vma: rename vma_get_page_prot to vma_flags_to_page_prot
In reply to Lorenzo StoakesQuoted 8 lines · Lorenzo Stoakes
Having
vma_get_page_prot()refer to VMA flags andvma_set_page_prot()refer to a VMA is confusing.Rename
vma_get_page_prot()tovma_flags_to_page_prot()to resolve this confusion.No functional change intended.
Quoted 8 lines
Signed-off-by Lorenzo Stoakes <[email protected]>--- include/linux/mm.h | 4 ++-- mm/vma.c | 2 +- mm/vma.h | 2 +- tools/testing/vma/include/dup.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-)
Makes sense.
Signature · 2 lines
Best Regards, Yan, Zi
Re: [PATCH 08/13] mm: introduce vma_get_page_prot() and use it
In reply to Lorenzo StoakesQuoted 27 lines · Lorenzo Stoakes
There's a large number of
vm_get_page_prot(vma->vm_flags)invocations. Make life easier by introducingvma_get_page_prot()parameterised by the VMA.This also makes converting
vm_get_page_prot()tovma_flags_teasier.Also update the userland VMA tests to reflect the change.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- drivers/gpu/drm/drm_gem.c | 2 +- drivers/gpu/drm/drm_gem_dma_helper.c | 2 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_gem.c | 6 +++--- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 12 ++++++------ drivers/gpu/drm/msm/msm_gem.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +- drivers/gpu/drm/omapdrm/omap_gem.c | 6 +++--- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 2 +- drivers/gpu/drm/tegra/gem.c | 2 +- drivers/gpu/drm/virtio/virtgpu_vram.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 2 +- drivers/gpu/drm/xe/xe_device.c | 2 +- drivers/gpu/drm/xe/xe_mmio_gem.c | 2 +-
Quoted 9 lines
drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +- drivers/video/fbdev/core/fb_io_fops.c | 2 +- include/linux/mm.h | 10 +++++++++- mm/vma.c | 2 +- mm/vma_exec.c | 2 +- sound/core/memalloc.c | 2 +- tools/testing/vma/include/dup.h | 4 ++++ 23 files changed, 43 insertions(+), 31 deletions(-)
Signature · 2 lines
Best Regards, Yan, Zi
Re: [PATCH 09/13] mm/vma: update create_init_stack_vma() to use vma_flags_t
In reply to Lorenzo StoakesQuoted 7 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues increate_init_stack_vma().As part of this change we add
VMA_STACK_EARLYandVMA_STACK_INCOMPLETEvma_flags_tdefines, and slightly reworkcreate_init_stack_vma()for clarity.
Quoted 8 lines
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- include/linux/mm.h | 4 ++++ mm/vma_exec.c | 18 +++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-)
LGTM.
Signature · 2 lines
Best Regards, Yan, Zi
Re: [PATCH 10/13] mm/vma: convert miscellaneous uses of VMA flags in core mm
In reply to Lorenzo StoakesQuoted 51 lines · Lorenzo Stoakes
Quoted 43 lines · Lance Yang
Quoted 8 lines · Lorenzo Stoakes
Update various uses of legacy flags in
vma.candmmap.cto the newvma_flags_ttype, updating comments alongside them to be consistent.Also update
__install_special_mapping()to rearrange things slightly to accommodate the changes.Signed-off-by Lorenzo Stoakes <[email protected]>---
[...]
Quoted 18 lines
mm/vma.c +3
-2@@ -3417,23 +3417,27 @@ struct vm_area_struct *__install_special_mapping( vm_flags_t vm_flags, void *priv, const struct vm_operations_struct *ops) { - int ret; + vma_flags_t vma_flags = legacy_to_vma_flags(vm_flags);struct vm_area_struct *vma; +int ret; vma =vm_area_alloc (mm); -if (unlikely (vma ==NULL )) +if (unlikely (!vma))return ERR_PTR (-ENOMEM );mm/vma.c struct vm_area_struct *__install_special_mapping( · line 3433
Quoted 10 lines
mm/vma.c +5
-4-vma_set_range (vma, addr, addr + len,0 ); - vm_flags |=vma_flags_to_legacy (mm->def_vma_flags) |VM_DONTEXPAND ; +vma_flags_set_mask (&vma_flags, mm->def_vma_flags); +vma_flags_set (&vma_flags,VMA_DONTEXPAND_BIT );if (pgtable_supports_soft_dirty ()) - vm_flags |=VM_SOFTDIRTY ; -vm_flags_init (vma, vm_flags & ~VM_LOCKED_MASK ); +vma_flags_set (&vma_flags,VMA_SOFTDIRTY_BIT ); +vma_flags_clear_mask (&vma_flags,VMA_LOCKED_MASK ); + vma->flags = vma_flags;Maybe worth a
vma_flags_init()helper here to mirrorvm_flags_init()? With this open-coded, we lose the soft-dirtyWARN_ON_ONCEsanity check.Quoted 1 line
Might be nicer to keep that check in one place ;)
I really hate all the VMA flag accessors, they conflate things horribly - we should be explicitly taking VMA write locks when we need to (and often killable ones actually) not assuming that a VMA flags accessor does (they should at most assert).
Quoted 8 lines
This case is even more terribly egregious - you are setting flags at an arbitrary time, why are we asserting something about softdirty?
You may update them as part of initialisation, maybe not. It's far from a guarantee and feels like a lazy place to put it.
BUT obviously it's an oversight not to open code that here, so I'll update the patch to do that!
What do you want to open code here? softdirty WARN_ON_ONCE()?
vma_flags gets VMA_SOFTDIRTY_BIT just above vma->flags, why do we need a check after that?
BTW, if you think the check is needed, patch 9 will need to be updated, since the same pattern appears in create_init_stack_vma().
Quoted 5 lines
I want VMA flags to be a clean stateless thing, other than the flags themselves. Implicit, unrelated, asserts or lock acquisitions in general should be done separately IMO.
Anyway,
Signature · 2 lines
Best Regards, Yan, Zi
Re: [PATCH 11/13] mm/mlock: convert mlock code to use vma_flags_t
In reply to Lorenzo StoakesQuoted 12 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mlock logic.Additionally update comments to reflect the changes to be consistent.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- mm/mlock.c | 86 +++++++++++++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 40 deletions(-)
LGTM. What confused me when I was reading the code is VMA_LOCKED_MASK and VMA_LOCKED_BIT, since both share the same prefix VMA_LOCKED. Before looking at the definition of VMA_LOCKED_MASK, I was wondering when to use _BIT or _MASK. But it is just a naming thing.
Anyway,
Signature · 2 lines
Best Regards, Yan, Zi
Zi Yan
reviewerRe: [PATCH 12/13] mm/mprotect: convert mprotect code to use vma_flags_t
In reply to Lorenzo StoakesQuoted 101 lines · Lorenzo Stoakes
Quoted 100 lines · Lance Yang
Quoted 68 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mprotect logic.Note that we retain the legacy
vm_flags_tbit shifting code indo_mprotect_key(), deferring avma_flags_tapproach to this for the time being.Additionally update comments to reflect the changes to be consistent.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
mm/mprotect.c| 16 ++++++++--------1 file changed, 8 insertions(+), 8 deletions(-)mm/mprotect.c +6
-6@@ -40,7 +40,7 @@static bool maybe_change_pte_writable (struct vm_area_struct *vma,pte_t pte) { -if (WARN_ON_ONCE (!(vma->vm_flags &VM_WRITE ))) +if (WARN_ON_ONCE (!vma_test (vma,VMA_WRITE_BIT )))return false;/* Don't touch entries that are not even readable. */ @@ -97,7 +97,7 @@ static bool can_change_shared_pte_writable(struct vm_area_struct *vma,bool can_change_pte_writable (struct vm_area_struct *vma,unsigned long addr,pte_t pte) { -if (!(vma->vm_flags &VM_SHARED )) +if (!vma_test (vma,VMA_SHARED_BIT ))return can_change_private_pte_writable (vma, addr, pte);return can_change_shared_pte_writable (vma, pte);@@ -194,7 +194,7 @@ static __always_inline void set_write_prot_commit_flush_ptes(struct vm_area_stru {bool set_write; -if (vma->vm_flags &VM_SHARED ) { +if (vma_test (vma,VMA_SHARED_BIT )) { set_write =can_change_shared_pte_writable (vma, ptent);prot_commit_flush_ptes (vma, addr, ptep, oldpte, ptent, nr_ptes,/* idx = */ 0 , set_write, tlb);@@ -811,8 +811,8 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,vm_unacct_memory (nrpages);/* -* Private VM_LOCKED VMA becoming writable: trigger COW to avoid major -* fault on access. +* Private VMA_LOCKED_BIT VMA becoming writable: trigger COW to avoid +* major fault on access. */ if (vma_flags_test(&new_vma_flags, VMA_WRITE_BIT) && vma_flags_test(&old_vma_flags, VMA_LOCKED_BIT) &&@@ -886,7 +886,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, goto out; start = vma->vm_start; error = -EINVAL ; -if (!(vma->vm_flags &VM_GROWSDOWN )) +if (!vma_test (vma,VMA_GROWSDOWN_BIT )) goto out; }else {if (vma->vm_start > start)mm/mprotect.c static int do_mprotect_pkey(unsigned long start, size_t len, · line 897
Quoted 5 lines
mm/mprotect.c +1
-1@@ -894,7 +894,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,if (unlikely (grows &PROT_GROWSUP )) { end = vma->vm_end; error = -EINVAL ; -if (!(vma->vm_flags &VM_GROWSUP )) +if (!vma_test (vma,VMA_GROWSUP_BIT ))IIUC, should this be
if (!vma_test_single_mask(vma,
VMA_GROWSUP))instead?
#elif
defined(CONFIG_PARISC)
#defineVM_GROWSUPINIT_VM_FLAG(GROWSUP)
...
#ifndefVM_GROWSUP
#defineVM_GROWSUPVM_NONE
...
VM_GROWSUPis only defined as GROWSUP on parisc and becomesVM_NONEelsewhere. ButVMA_GROWSUP_BITis the rawARCH_1bit, which is also used for other arch-specific VMA flags:DECLARE_VMA_BIT_ALIAS(SAO, ARCH_1), /* Strong Access Ordering (powerpc) */ DECLARE_VMA_BIT_ALIAS(GROWSUP, ARCH_1), /* parisc */ DECLARE_VMA_BIT_ALIAS(SPARC_ADI, ARCH_1), /* sparc64 */ DECLARE_VMA_BIT_ALIAS(ARM64_BTI, ARCH_1), /* arm64 */ DECLARE_VMA_BIT_ALIAS(ARCH_CLEAR, ARCH_1), /* sparc64, arm64 */ DECLARE_VMA_BIT_ALIAS(MAPPED_COPY, ARCH_1), /* !CONFIG_MMU */
Quoted 7 lines
Quoted 2 lines
Other
vma_test()changes look fine to me: just fixedINIT_VM_FLAG()masks matching their VMA_*_BIT :)Thanks you're right, will fix!
Again I swear I ran claude on all of this so it's failing me here :)
Is it better to add something like below to avoid misuse these mutually exclusive bit aliases?
An example for VMA_GROWSUP_BIT:
#if defined(CONFIG_PARISC)DECLARE_VMA_BIT_ALIAS(GROWSUP, ARCH_1), /* parisc */
#else
/* make VMA_GROWSUP_BIT a build bug on */
#endif
Hmm, these VMA_*_BIT are enum items, so the above might not be possible. An alternative is to only define them for the corresponding config and you will get build errors when trying to use them directly and the config is not enabled. Otherwise, misuses like "vma_test(vma, VMA_GROWSUP_BIT)" is harder to uncover.
Signature · 2 lines
Best Regards, Yan, Zi
Re: [PATCH 13/13] mm/mremap: convert mremap code to use vma_flags_t
In reply to Lorenzo StoakesQuoted 16 lines · Lorenzo Stoakes
Quoted 7 lines · Lance Yang
Quoted 5 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mremap logic.Additionally update comments to reflect the changes to be consistent.
Quoted 7 lines
Quoted 4 lines
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
The
vm_flags_set()cases below spell outvma_start_write(), but thevm_flags_clear()cases don't?
Quoted 7 lines
Yep as I said elsewhere, implicitly taking the lock is terrible and me doing this is completely on purpose to get rid of that :)
But I haven't been clear enough clearly, so I should put the argument as to why that's ok in the commit message.
Will do so on respin.
How about also add a comment to vma_clear*() telling us a lock is not needed and why like you explained a lock is needed for vma_set*()? This asymmetry could confuse people.
This patch looks good to me.
Signature · 2 lines
Best Regards, Yan, Zi
Lorenzo Stoakes
maintainerRe: [PATCH 10/13] mm/vma: convert miscellaneous uses of VMA flags in core mm
In reply to Zi YanQuoted 55 lines · Zi Yan
Quoted 54 lines · Lorenzo Stoakes
Quoted 43 lines · Lance Yang
Quoted 8 lines · Lorenzo Stoakes
Update various uses of legacy flags in
vma.candmmap.cto the newvma_flags_ttype, updating comments alongside them to be consistent.Also update
__install_special_mapping()to rearrange things slightly to accommodate the changes.Signed-off-by Lorenzo Stoakes <[email protected]>---
[...]
Quoted 18 lines
mm/vma.c +3
-2@@ -3417,23 +3417,27 @@ struct vm_area_struct *__install_special_mapping( vm_flags_t vm_flags, void *priv, const struct vm_operations_struct *ops) { - int ret; + vma_flags_t vma_flags = legacy_to_vma_flags(vm_flags);struct vm_area_struct *vma; +int ret; vma =vm_area_alloc (mm); -if (unlikely (vma ==NULL )) +if (unlikely (!vma))return ERR_PTR (-ENOMEM );mm/vma.c struct vm_area_struct *__install_special_mapping( · line 3433
Quoted 10 lines
mm/vma.c +5
-4-vma_set_range (vma, addr, addr + len,0 ); - vm_flags |=vma_flags_to_legacy (mm->def_vma_flags) |VM_DONTEXPAND ; +vma_flags_set_mask (&vma_flags, mm->def_vma_flags); +vma_flags_set (&vma_flags,VMA_DONTEXPAND_BIT );if (pgtable_supports_soft_dirty ()) - vm_flags |=VM_SOFTDIRTY ; -vm_flags_init (vma, vm_flags & ~VM_LOCKED_MASK ); +vma_flags_set (&vma_flags,VMA_SOFTDIRTY_BIT ); +vma_flags_clear_mask (&vma_flags,VMA_LOCKED_MASK ); + vma->flags = vma_flags;Maybe worth a
vma_flags_init()helper here to mirrorvm_flags_init()? With this open-coded, we lose the soft-dirtyWARN_ON_ONCEsanity check.Quoted 1 line
Might be nicer to keep that check in one place ;)
I really hate all the VMA flag accessors, they conflate things horribly - we should be explicitly taking VMA write locks when we need to (and often killable ones actually) not assuming that a VMA flags accessor does (they should at most assert).
This case is even more terribly egregious - you are setting flags at an arbitrary time, why are we asserting something about softdirty?
Quoted 7 lines
Quoted 5 lines
You may update them as part of initialisation, maybe not. It's far from a guarantee and feels like a lazy place to put it.
BUT obviously it's an oversight not to open code that here, so I'll update the patch to do that!
What do you want to open code here? softdirty
WARN_ON_ONCE()?
As you can tell I said this reflexively without checking the code :)
Quoted 3 lines
vma_flagsgetsVMA_SOFTDIRTY_BITjust abovevma->flags, why do we need a check after that?
And yeah it's completely unnecessary, indeed.
Quoted 3 lines
BTW, if you think the check is needed, patch 9 will need to be updated, since the same pattern appears in
create_init_stack_vma().
I'll check to see if it's valid there.
For me it just feels like the most silly place to put that check, a VMA flags update should update VMA flags not start randomly asserting silly things :)
Quoted 10 lines
Quoted 6 lines
I want VMA flags to be a clean stateless thing, other than the flags themselves. Implicit, unrelated, asserts or lock acquisitions in general should be done separately IMO.
Anyway,
Reviewed-by Zi Yan <[email protected]>
Thanks!
Quoted 5 lines
--
Best Regards,
Yan, Zi
Cheers, Lorenzo
Lorenzo Stoakes
maintainerRe: [PATCH 10/13] mm/vma: convert miscellaneous uses of VMA flags in core mm
Quoted 70 lines · Lorenzo Stoakes
Quoted 55 lines · Zi Yan
Quoted 54 lines · Lorenzo Stoakes
Quoted 43 lines · Lance Yang
Quoted 8 lines · Lorenzo Stoakes
Update various uses of legacy flags in
vma.candmmap.cto the newvma_flags_ttype, updating comments alongside them to be consistent.Also update
__install_special_mapping()to rearrange things slightly to accommodate the changes.Signed-off-by Lorenzo Stoakes <[email protected]>---
[...]
Quoted 18 lines
mm/vma.c +3
-2@@ -3417,23 +3417,27 @@ struct vm_area_struct *__install_special_mapping( vm_flags_t vm_flags, void *priv, const struct vm_operations_struct *ops) { - int ret; + vma_flags_t vma_flags = legacy_to_vma_flags(vm_flags);struct vm_area_struct *vma; +int ret; vma =vm_area_alloc (mm); -if (unlikely (vma ==NULL )) +if (unlikely (!vma))return ERR_PTR (-ENOMEM );mm/vma.c struct vm_area_struct *__install_special_mapping( · line 3433
Quoted 10 lines
mm/vma.c +5
-4-vma_set_range (vma, addr, addr + len,0 ); - vm_flags |=vma_flags_to_legacy (mm->def_vma_flags) |VM_DONTEXPAND ; +vma_flags_set_mask (&vma_flags, mm->def_vma_flags); +vma_flags_set (&vma_flags,VMA_DONTEXPAND_BIT );if (pgtable_supports_soft_dirty ()) - vm_flags |=VM_SOFTDIRTY ; -vm_flags_init (vma, vm_flags & ~VM_LOCKED_MASK ); +vma_flags_set (&vma_flags,VMA_SOFTDIRTY_BIT ); +vma_flags_clear_mask (&vma_flags,VMA_LOCKED_MASK ); + vma->flags = vma_flags;Maybe worth a
vma_flags_init()helper here to mirrorvm_flags_init()? With this open-coded, we lose the soft-dirtyWARN_ON_ONCEsanity check.Quoted 1 line
Might be nicer to keep that check in one place ;)
I really hate all the VMA flag accessors, they conflate things horribly - we should be explicitly taking VMA write locks when we need to (and often killable ones actually) not assuming that a VMA flags accessor does (they should at most assert).
This case is even more terribly egregious - you are setting flags at an arbitrary time, why are we asserting something about softdirty?
Quoted 7 lines
Quoted 5 lines
You may update them as part of initialisation, maybe not. It's far from a guarantee and feels like a lazy place to put it.
BUT obviously it's an oversight not to open code that here, so I'll update the patch to do that!
What do you want to open code here? softdirty
WARN_ON_ONCE()?As you can tell I said this reflexively without checking the code :)
Quoted 3 lines
vma_flagsgetsVMA_SOFTDIRTY_BITjust abovevma->flags, why do we need a check after that?
Quoted 7 lines
And yeah it's completely unnecessary, indeed.
Quoted 3 lines
BTW, if you think the check is needed, patch 9 will need to be updated, since the same pattern appears in
create_init_stack_vma().I'll check to see if it's valid there.
Exactly the same case as here, unnecessary :)
Cheers, LOrenzo
Lorenzo Stoakes
maintainerRe: [PATCH 11/13] mm/mlock: convert mlock code to use vma_flags_t
In reply to Zi YanQuoted 14 lines · Zi Yan
Quoted 12 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mlock logic.Additionally update comments to reflect the changes to be consistent.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>--- mm/mlock.c | 86 +++++++++++++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 40 deletions(-)
Quoted 4 lines
LGTM. What confused me when I was reading the code is
VMA_LOCKED_MASKandVMA_LOCKED_BIT, since both share the same prefixVMA_LOCKED. Before looking at the definition ofVMA_LOCKED_MASK, I was wondering when to use_BITor_MASK. But it is just a naming thing.
Yeah, indeed, not great naming there. Was a pre-existing though, and previously was VM_LOCKED_MASK (also not great :)
The whole VMA_LOCKED_BIT and VMA_LOCK_ON_FAULT_BIT thing is a bit weird anyway, since you set both and then just use the latter to know not to fault in in GUP... anyway, can revisit that one later...! :)
Quoted 4 lines
Anyway,
Reviewed-by Zi Yan <[email protected]>
Thanks!
Quoted 6 lines
--
Best Regards,
Yan, Zi
Cheers, Lorenzo
Lorenzo Stoakes
maintainerRe: [PATCH 12/13] mm/mprotect: convert mprotect code to use vma_flags_t
In reply to Zi YanQuoted 113 lines · Zi Yan
Quoted 101 lines · Lorenzo Stoakes
Quoted 100 lines · Lance Yang
Quoted 68 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mprotect logic.Note that we retain the legacy
vm_flags_tbit shifting code indo_mprotect_key(), deferring avma_flags_tapproach to this for the time being.Additionally update comments to reflect the changes to be consistent.
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
mm/mprotect.c| 16 ++++++++--------1 file changed, 8 insertions(+), 8 deletions(-)mm/mprotect.c +6
-6@@ -40,7 +40,7 @@static bool maybe_change_pte_writable (struct vm_area_struct *vma,pte_t pte) { -if (WARN_ON_ONCE (!(vma->vm_flags &VM_WRITE ))) +if (WARN_ON_ONCE (!vma_test (vma,VMA_WRITE_BIT )))return false;/* Don't touch entries that are not even readable. */ @@ -97,7 +97,7 @@ static bool can_change_shared_pte_writable(struct vm_area_struct *vma,bool can_change_pte_writable (struct vm_area_struct *vma,unsigned long addr,pte_t pte) { -if (!(vma->vm_flags &VM_SHARED )) +if (!vma_test (vma,VMA_SHARED_BIT ))return can_change_private_pte_writable (vma, addr, pte);return can_change_shared_pte_writable (vma, pte);@@ -194,7 +194,7 @@ static __always_inline void set_write_prot_commit_flush_ptes(struct vm_area_stru {bool set_write; -if (vma->vm_flags &VM_SHARED ) { +if (vma_test (vma,VMA_SHARED_BIT )) { set_write =can_change_shared_pte_writable (vma, ptent);prot_commit_flush_ptes (vma, addr, ptep, oldpte, ptent, nr_ptes,/* idx = */ 0 , set_write, tlb);@@ -811,8 +811,8 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,vm_unacct_memory (nrpages);/* -* Private VM_LOCKED VMA becoming writable: trigger COW to avoid major -* fault on access. +* Private VMA_LOCKED_BIT VMA becoming writable: trigger COW to avoid +* major fault on access. */ if (vma_flags_test(&new_vma_flags, VMA_WRITE_BIT) && vma_flags_test(&old_vma_flags, VMA_LOCKED_BIT) &&@@ -886,7 +886,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, goto out; start = vma->vm_start; error = -EINVAL ; -if (!(vma->vm_flags &VM_GROWSDOWN )) +if (!vma_test (vma,VMA_GROWSDOWN_BIT )) goto out; }else {if (vma->vm_start > start)mm/mprotect.c static int do_mprotect_pkey(unsigned long start, size_t len, · line 897
Quoted 5 lines
mm/mprotect.c +1
-1@@ -894,7 +894,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,if (unlikely (grows &PROT_GROWSUP )) { end = vma->vm_end; error = -EINVAL ; -if (!(vma->vm_flags &VM_GROWSUP )) +if (!vma_test (vma,VMA_GROWSUP_BIT ))IIUC, should this be
if (!vma_test_single_mask(vma,
VMA_GROWSUP))instead?
#elif
defined(CONFIG_PARISC)
#defineVM_GROWSUPINIT_VM_FLAG(GROWSUP)
...
#ifndefVM_GROWSUP
#defineVM_GROWSUPVM_NONE
...
VM_GROWSUPis only defined as GROWSUP on parisc and becomesVM_NONEelsewhere. ButVMA_GROWSUP_BITis the rawARCH_1bit, which is also used for other arch-specific VMA flags:DECLARE_VMA_BIT_ALIAS(SAO, ARCH_1), /* Strong Access Ordering (powerpc) */ DECLARE_VMA_BIT_ALIAS(GROWSUP, ARCH_1), /* parisc */ DECLARE_VMA_BIT_ALIAS(SPARC_ADI, ARCH_1), /* sparc64 */ DECLARE_VMA_BIT_ALIAS(ARM64_BTI, ARCH_1), /* arm64 */ DECLARE_VMA_BIT_ALIAS(ARCH_CLEAR, ARCH_1), /* sparc64, arm64 */ DECLARE_VMA_BIT_ALIAS(MAPPED_COPY, ARCH_1), /* !CONFIG_MMU */Quoted 7 lines
Quoted 2 lines
Other
vma_test()changes look fine to me: just fixedINIT_VM_FLAG()masks matching their VMA_*_BIT :)Thanks you're right, will fix!
Again I swear I ran claude on all of this so it's failing me here :)
Is it better to add something like below to avoid misuse these mutually exclusive bit aliases?
Quoted 7 lines
An example for
VMA_GROWSUP_BIT:#if
defined(CONFIG_PARISC)DECLARE_VMA_BIT_ALIAS(GROWSUP, ARCH_1), /* parisc */
#else
/* makeVMA_GROWSUP_BITa build bug on */
#endif
Nah we can't...
Quoted 2 lines
Hmm, these VMA_*_BIT are enum items, so the above might not be possible.
...because of this but also this is the canonical definition and we want to use it for the VMA_xxx form.
Quoted 4 lines
An alternative is to only define them for the corresponding config and you will get build errors when trying to use them directly and the config is not enabled. Otherwise, misuses like "
vma_test(vma, VMA_GROWSUP_BIT)" is harder to uncover.
Yeah good idea, I may add a new DECLARE_INTERNAL_VMA_BIT() that gives you __VMA_xxx_BIT, so nobody makes the obvious mistake and we force uses of safe VMA_xxx form. Hmm :)
Quoted 6 lines
--
Best Regards,
Yan, Zi
Thanks, Lorenzo
Lorenzo Stoakes
maintainerRe: [PATCH 13/13] mm/mremap: convert mremap code to use vma_flags_t
In reply to Zi YanQuoted 20 lines · Zi Yan
Quoted 19 lines · Lorenzo Stoakes
Quoted 7 lines · Lance Yang
Quoted 5 lines · Lorenzo Stoakes
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues throughout the mremap logic.Additionally update comments to reflect the changes to be consistent.
Quoted 7 lines
Quoted 4 lines
No functional change intended.
Signed-off-by Lorenzo Stoakes <[email protected]>---
The
vm_flags_set()cases below spell outvma_start_write(), but thevm_flags_clear()cases don't?Yep as I said elsewhere, implicitly taking the lock is terrible and me doing this is completely on purpose to get rid of that :)
Quoted 8 lines
Quoted 4 lines
But I haven't been clear enough clearly, so I should put the argument as to why that's ok in the commit message.
Will do so on respin.
How about also add a comment to vma_clear*() telling us a lock is not needed and why like you explained a lock is needed for vma_set*()? This asymmetry could confuse people.
Well you do absolutely have to hold the lock once the VMA is initialised when using any VMA flags API that manipulates flags (apart from the explicitly atomic case).
The argument here is that the VMA was _already locked_ at the point of clearing :)
Quoted 4 lines
This patch looks good to me.
Reviewed-by Zi Yan <[email protected]>
Thanks!
Quoted 5 lines
--
Best Regards,
Yan, Zi
Cheers, Lorenzo