[PATCH 0/3] mm: only assert mmap and VMA locks if CONFIG_DEBUG_VM is set

[PATCH 0/3] mm: only assert mmap and VMA locks if CONFIG_DEBUG_VM is set

[Lorenzo Stoakes (ARM)]

Currently these asserts are all performed at runtime regardless of whether
or not the kernel is intended for debugging mm code (i.e. whether or not
CONFIG_DEBUG_VM is set).

These are explicitly programmatic checks - that is, checking basic
correctness in mm code which interacts with these locks.

The problem with this is that checks like this are generally asserted in mm
code using VM_WARN_ON_ONCE() and similar, and thus only checked if
CONFIG_DEBUG_VM is set.

So having some assertions which are unconditional and some not without it
being clear, creates something of an unintended two-tier system.

Since what is being asserted in these cases is exactly the same kind of
condition as those asserted in VM_WARN_ON_ONCE() assertions, simply
predicate them on CONFIG_DEBUG_VM.

Retain assertions for mmap lock asserts if CONFIG_LOCKDEP is specified in
order that expected lockdep assertions continue to function.

The VMA lock implementation already performs lockdep tracking regardless of
whether CONFIG_DEBUG_VM is set or not, so no changes are required there.

---
Note that this series assumes [0] has landed and builds on it. That patch
is the uncontroversial bit, this series contains the bits that are slightly
more controversial (but not really :)

[0]: https://lore.kernel.org/linux-mm/20261110-vma-fixup-debug-asserts-v1-1-0b2e0680cd7e@kernel.org/

To: Andrew Morton <akpm@linux-foundation.org>
To: Suren Baghdasaryan <surenb@google.com>
To: Liam R. Howlett <liam@infradead.org>
To: Vlastimil Babka <vbabka@kernel.org>
To: Shakeel Butt <shakeel.butt@linux.dev>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

---
Lorenzo Stoakes (ARM) (3):
      mm: only assert mmap locks if CONFIG_DEBUG_VM or lockdep is set
      mm: only assert VMA locks if CONFIG_DEBUG_VM is set
      mm: convert WARN_ON_ONCE()'s to VM_WARN_ON_ONCE() in VMA lock code

 include/linux/mmap_lock.h | 23 +++++++++++++++++++----
 mm/mmap_lock.c            |  6 +++---
 2 files changed, 22 insertions(+), 7 deletions(-)
---
base-commit: b82f7651c53524960fe38a54aa69677dcddff655
change-id: 20261110-tame-vma-asserts-480acf6aee20

Cheers,
-- 
Lorenzo Stoakes (ARM) <ljs@kernel.org>

[PATCH 1/3] mm: only assert mmap locks if CONFIG_DEBUG_VM or lockdep is set

[Lorenzo Stoakes (ARM)]

For a long while we have asserted the mmap lock being held regardless of
whether or not the kernel is a debug build or not.

In general, given that these have been added as programmatic asserts that
nobody did something stupid, production kernels are simply wasting their
time checking for this.

It also means there is a two tier system - where some programmatic
asserts (e.g. VM_WARN_ON_ONCE()-checked conditions) are only checked for if
CONFIG_DEBUG_VM is enabled, whereas others are checked unconditionally.

If we genuinely want to unconditionally check for the mmap lock being held
at runtime, then that should be made clear in the code doing it.

Regardless, if CONFIG_LOCKDEP is enabled, ensure that the lockdep
assertions are performed.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 include/linux/mmap_lock.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
index 9c3a9c6525c1..9367430ee53c 100644
--- a/include/linux/mmap_lock.h
+++ b/include/linux/mmap_lock.h
@@ -75,12 +75,14 @@ static inline void __mmap_lock_trace_released(struct mm_struct *mm, bool write)
 
 static inline void mmap_assert_locked(const struct mm_struct *mm)
 {
-	rwsem_assert_held(&mm->mmap_lock);
+	if (IS_ENABLED(CONFIG_DEBUG_VM) || IS_ENABLED(CONFIG_LOCKDEP))
+		rwsem_assert_held(&mm->mmap_lock);
 }
 
 static inline void mmap_assert_write_locked(const struct mm_struct *mm)
 {
-	rwsem_assert_held_write(&mm->mmap_lock);
+	if (IS_ENABLED(CONFIG_DEBUG_VM) || IS_ENABLED(CONFIG_LOCKDEP))
+		rwsem_assert_held_write(&mm->mmap_lock);
 }
 
 #ifdef CONFIG_LOCKDEP

-- 
2.56.0

[PATCH 2/3] mm: only assert VMA locks if CONFIG_DEBUG_VM is set

[Lorenzo Stoakes (ARM)]

Similar to the mmap lock asserts, we have also asserted VMA locks
regardless of whether or not the kernel is a debug build or not.

Similarly, these are asserting basic assumptions about kernel code which
should never assert in production code.

Since all such bugs are generally asserted using VM_WARN_ON_ONCE() or
similar, it creates an odd and inconsistent situation where some
programmatic asserts are only checked under CONFIG_DEBUG_VM and some are
always checked.

Since the intent is the same as any VM_WARN_ON_ONCE() check, do not assert
if CONFIG_DEBUG_VM is not set.

Implement by updating vma_assert_attached() and vma_assert_detached() to
use VM_WARN_ON_ONCE() rather than WARN_ON_ONCE(), and gating
vma_assert_[write_]locked() and vma_assert_can_modify() on CONFIG_DEBUG_VM.

nommu loses its CONFIG_LOCKDEP asserts if !CONFIG_DEBUG_VM in
vma_assert_[write_]locked(), but this doesn't seem to be a problem.

In addition, there were already inconsistencies - vma_assert_write_locked()
uses VM_WARN_ON_ONCE_VMA() and performs no actions if CONFIG_DEBUG_VM is
not set, and vma_assert_locked() looked up vma->vm_refcnt but also only
asserted if CONFIG_DEBUG_VM was set.

Therefore simply update these functions such that they're predicated on
CONFIG_DEBUG_VM and update their kdoc descriptions accordingly.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 include/linux/mmap_lock.h | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
index 9367430ee53c..bada7a03ddf4 100644
--- a/include/linux/mmap_lock.h
+++ b/include/linux/mmap_lock.h
@@ -354,9 +354,14 @@ int vma_start_write_killable(struct vm_area_struct *vma)
 /**
  * vma_assert_write_locked() - assert that @vma holds a VMA write lock.
  * @vma: The VMA to assert.
+ *
+ * Assert only if CONFIG_DEBUG_VM is set.
  */
 static inline void vma_assert_write_locked(const struct vm_area_struct *vma)
 {
+	if (!IS_ENABLED(CONFIG_DEBUG_VM))
+		return;
+
 	if (!IS_ENABLED(CONFIG_MMU)) {
 		mmap_assert_write_locked(vma->vm_mm);
 		return;
@@ -369,11 +374,16 @@ static inline void vma_assert_write_locked(const struct vm_area_struct *vma)
  * vma_assert_locked() - assert that @vma holds either a VMA read or a VMA write
  * lock and is not detached.
  * @vma: The VMA to assert.
+ *
+ * Assert only if CONFIG_DEBUG_VM is set.
  */
 static inline void vma_assert_locked(const struct vm_area_struct *vma)
 {
 	unsigned int refcnt;
 
+	if (!IS_ENABLED(CONFIG_DEBUG_VM))
+		return;
+
 	if (!IS_ENABLED(CONFIG_MMU)) {
 		mmap_assert_locked(vma->vm_mm);
 		return;
@@ -480,12 +490,12 @@ static inline void vma_assert_attached(struct vm_area_struct *vma)
 	if (!IS_ENABLED(CONFIG_MMU))
 		return;
 
-	WARN_ON_ONCE(!vma_is_attached(vma));
+	VM_WARN_ON_ONCE(!vma_is_attached(vma));
 }
 
 static inline void vma_assert_detached(struct vm_area_struct *vma)
 {
-	WARN_ON_ONCE(vma_is_attached(vma));
+	VM_WARN_ON_ONCE(vma_is_attached(vma));
 }
 
 static inline void vma_mark_attached(struct vm_area_struct *vma)
@@ -549,6 +559,9 @@ struct vm_area_struct *lock_next_vma(struct vma_rcu_iter *rvmi,
 
 static inline void vma_assert_can_modify(struct vm_area_struct *vma)
 {
+	if (!IS_ENABLED(CONFIG_DEBUG_VM))
+		return;
+
 	if (vma_is_attached(vma))
 		vma_assert_write_locked(vma);
 }

-- 
2.56.0

[PATCH 3/3] mm: convert WARN_ON_ONCE()'s to VM_WARN_ON_ONCE() in VMA lock code

[Lorenzo Stoakes (ARM)]

Now that VMA assertions are predicated on CONFIG_DEBUG_VM, it makes sense
to convert existing unconditional WARN_ON_ONCE()'s in VMA lock code to
VM_WARN_ON_ONCE()'s.

These are asserting programmatic invariants which are in most other cases
asserted using VM_WARN_ON_ONCE(), and there is simply no need for these to
be unconditionally checked, so no longer do so.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 mm/mmap_lock.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/mmap_lock.c b/mm/mmap_lock.c
index 834bbf86e004..073c5125b424 100644
--- a/mm/mmap_lock.c
+++ b/mm/mmap_lock.c
@@ -146,7 +146,7 @@ int __vma_start_write(struct vm_area_struct *vma, int state)
 
 	err = __vma_start_exclude_readers(&ves);
 	if (err) {
-		WARN_ON_ONCE(ves.detached);
+		VM_WARN_ON_ONCE(ves.detached);
 		return err;
 	}
 
@@ -161,7 +161,7 @@ int __vma_start_write(struct vm_area_struct *vma, int state)
 	if (ves.exclusive) {
 		__vma_end_exclude_readers(&ves);
 		/* VMA should remain attached. */
-		WARN_ON_ONCE(ves.detached);
+		VM_WARN_ON_ONCE(ves.detached);
 	}
 
 	return 0;
@@ -193,7 +193,7 @@ void __vma_exclude_readers_for_detach(struct vm_area_struct *vma)
 		__vma_end_exclude_readers(&ves);
 	}
 	/* If an error arose but we were detached anyway, we don't care. */
-	WARN_ON_ONCE(!ves.detached);
+	VM_WARN_ON_ONCE(!ves.detached);
 }
 
 /* Do the VMA, mm seqcounts indicate the VMA is write locked? */

-- 
2.56.0