Thread (58 messages) 58 messages, 5 authors, 1d ago

Re: [PATCH 10/13] mm/vma: convert miscellaneous uses of VMA flags in core mm

From: "Zi Yan" <ziy@nvidia.com>
Date: 2026-07-09 01:52:41
Also in: dri-devel, intel-gfx, intel-xe, linux-arm-msm, linux-fbdev, linux-fsdevel, linux-mips, linux-mm, linux-rockchip, linux-samsung-soc, linux-sound, linux-tegra, lkml, nouveau, virtualization, xen-devel

On Thu Jul 2, 2026 at 11:46 AM EDT, Lorenzo Stoakes wrote:
On Thu, Jul 02, 2026 at 09:12:33PM +0800, Lance Yang wrote:
quoted
On Mon, Jun 29, 2026 at 08:25:33PM +0100, Lorenzo Stoakes wrote:
quoted
Update 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.

Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
---
[...]
quoted
diff --git a/mm/vma.c b/mm/vma.c
index b81c05e67a61..ab2ef0f04420 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -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;
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 ;)
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!
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().
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 <ziy@nvidia.com>

-- 
Best Regards,
Yan, Zi

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help