On Thu, Jul 02, 2026 at 09:49:47PM +0800, Lance Yang wrote:
>
> On Mon, Jun 29, 2026 at 08:25:36PM +0100, Lorenzo Stoakes wrote:
> >Replace 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.
> >
> >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?

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.

>
> Thanks, Lance
>
> > mm/mremap.c | 38 ++++++++++++++++++++------------------
> > 1 file changed, 20 insertions(+), 18 deletions(-)
> >
> >diff --git a/mm/mremap.c b/mm/mremap.c
> >index 079a0ba0c4a7..0ea43302b7ed 100644
> >--- a/mm/mremap.c
> >+++ b/mm/mremap.c
> >@@ -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);

This is called from move_vma() which holds the VMA write lock on vma.

> >             /* 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);

These need vma_start_write() as referencing other, unlocked VMAs.

> >             }
> >     }
> > }
> >@@ -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.

> >
> >     /*
> >      * 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))
> >--
> >2.54.0
> >
> >

Cheers, Lorenzo

Reply via email to