Re: [PATCH 02/13] mm/vma: update do_mmap() to use vma_flags_t

On Mon, Jun 29, 2026 at 08:25:25PM +0100, Lorenzo Stoakes wrote:
>The 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.
>
>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:

Reviewed-by: Lance Yang <[email protected]>

[...]
>diff --git a/mm/mmap.c b/mm/mmap.c
>index 46174e706bbe..547352183214 100644
>--- a/mm/mmap.c
>+++ b/mm/mmap.c
[...]
>@@ -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;
> 
>-			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

>-
> 			/*
> 			 * 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.
[...]