Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Mon, Nov 09, 2026 at 03:01:29PM +0100, David Hildenbrand (Arm) wrote:
> On 11/6/26 13:07, Lorenzo Stoakes (ARM) wrote:
> > On reflection, I think this would be better implemented as something like:
> >
> > static void collapse_scan_mm_slot(unsigned int progress_max, ...)
> > {
> > 	unsigned int long lock_start = jiffies;
> > 	...
> >   	for_each_vma(vmi, vma) {
> > 		...
> >                 if (time_after(jiffies, lock_start) && mmap_lock_is_contended(mm)) {
> >                         mmap_read_unlock(mm);
> >                         cond_resched();
> >                         mmap_read_lock(mm);
> >                         lock_start = jiffies;
> >                         vma_iter_set(&vmi, mm_slot->address);
> >                         continue;
> >                 }
> > 	}
> > }
> >
> > Which avoids magic numbers, and means if we hold the lock for 1-10ms (depending
> > on HZ), which is a long time in the kernel to hold a lock, AND the lock is
> > contended, we drop/reacquire.
>
> That approach indeed sounds better to me regarding lock contention.

Unfortunately it makes things measurably worse (it's not fine-grained
enough). Trying to get a fine-grained timer will involve preemption
disable/enable pairs, so a no go.

This is the alternative I have locally which explains the number, puts VMA in
the name and it applies to _all_ VMAs as requested.

Let me know what you think!

The jiffies version would obviously just be this but with the above, but that
one results in max ~2 ms-level stalls rather than ~200us.

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 7d1414a9b2a5..59bbef61f104 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -66,6 +66,13 @@ static DEFINE_READ_MOSTLY_HASHTABLE(mm_slots_hash, MM_SLOTS_HASH_BITS);

 static struct kmem_cache *mm_slot_cache __ro_after_init;

+/*
+ * The number of VMAs to walk before checking for, and relieving, mmap lock
+ * contention. This bounds the maximum mmap read lock to an estimated ~100 -
+ * 300us depending on hardware.
+ */
+#define KHUGEPAGED_MAX_VMA_SCAN_BATCH 1024
+
 /**
  * struct khugepaged_mm_slot - khugepaged information per mm that is being scanned
  * @slot: hash lookup from mm to mm_slot
@@ -2910,6 +2917,7 @@ static void collapse_scan_mm_slot(unsigned int progress_max,
 	struct mm_struct *mm;
 	struct vm_area_struct *vma;
 	unsigned int progress_prev = cc->progress;
+	unsigned int nr_vmas_batch = 0;

 	lockdep_assert_held(&khugepaged_mm_lock);
 	*result = SCAN_FAIL;
@@ -2942,6 +2950,18 @@ static void collapse_scan_mm_slot(unsigned int progress_max,
 	for_each_vma(vmi, vma) {
 		unsigned long hstart, hend;

+		/* Relieve mmap lock contention after walking a batch of VMAs. */
+		if (nr_vmas_batch++ >= KHUGEPAGED_MAX_VMA_SCAN_BATCH) {
+			nr_vmas_batch = 0;
+			if (mmap_lock_is_contended(mm)) {
+				mmap_read_unlock(mm);
+				cond_resched();
+				mmap_read_lock(mm);
+				vma_iter_set(&vmi, mm_slot->address);
+				continue;
+			}
+		}
+
 		cond_resched();
 		if (unlikely(collapse_test_exit_or_disable(mm))) {
 			cc->progress++;

--
Cheers, Lorenzo