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

[Lance Yang <lance.yang@xxxxxxxxx>]

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 <ljs@xxxxxxxxxx>
>---

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 <lance.yang@xxxxxxxxx>

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