[PATCH 0/5] mm/khugepaged: bound mmap_lock hold time when walking an mm's VMAs

[PATCH 0/5] mm/khugepaged: bound mmap_lock hold time when walking an mm's VMAs

[Lorenzo Stoakes (ARM)]

This series introduces a couple small changes to khugepaged to improve its
behaviour both in the face of mm->mmap_lock contention and to reduce that
lock contention when possible.

It also updates other places which release the mmap read lock to allow mmap
contended mmap write locks a chance to make progress (BPF iterators and
/proc/$pid/maps + friends).

Looking at the first change - if the mmap lock is contended, an attempted
mmap trylock in collapse_scan_mm_slot() will result in the scan being
aborted wherever it happens to be, and when next the mm is processed it
starts from the beginning again.

If an mm has a lot of ineligible VMAs and is undergoing significant
mmap_lock contention, collapsing the eligible VMAs there can take a very
long time.

Patch 2 of the series fixes this (patch 1 is a prerequisite) by keeping
track of where khugepaged was in the mm before aborting the scan.

This issue was noticed during some work in which an mm was instantiated
with a large number of small VMAs under significant mmap_lock contention,
during which collapse was observed to take 10.8s. With this patch applied
it took 3.6s to collapse the same mm.

Patch 3 introduces new rwsem primitives - down_read_nosteal() and
down_read_killable_nosteal() - to allow for read locks to be taken without
stealing them from a queued write lock.

Patch 4 adds mmap lock wrappers for this - mmap_read_lock_nosteal() and
mmap_read_lock_killable_nosteal() and updates callers which release the mmap
read lock on contention - BPF iterators + /proc/$pid/maps et al.

Finally, patch 5 uses this to have khugepaged release + reacquire the mmap
read lock in the face of a large number of ineligible VMAs.

This was also observed as part of the previously mentioned work in the form
of mmap lock stalls, which patch 5 resolves.

Importantly, none of the changes introduces any regressions for khugepaged
anywhere. It is all upside, and implemented conservatively.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
Lorenzo Stoakes (ARM) (5):
      Revert "mm/khugepaged: remove definition of struct khugepaged_mm_slot"
      mm/khugepaged: scan mm's more efficiently under mmap_lock contention
      locking/rwsem: add down_read[_killable]_nosteal()
      mm: add and use mmap_read_lock[_killable]_nosteal() on contention
      mm/khugepaged: drop/reacquire mmap read lock if skipped enough VMAs

 include/linux/mmap_lock.h |  44 +++++++++++++++++++
 include/linux/rwsem.h     |   2 +
 kernel/bpf/task_iter.c    |   2 +-
 kernel/locking/rwsem.c    |  71 ++++++++++++++++++++++++------
 mm/khugepaged.c           | 108 ++++++++++++++++++++++++++++++----------------
 mm/mmap_lock.c            |   2 +-
 6 files changed, 178 insertions(+), 51 deletions(-)
---
base-commit: 34f88a114888c9dba49e45f2b0c2a3e83c4c871c
change-id: 20261029-khugepaged-latency-fix-40495d66ea50

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

[PATCH 1/5] Revert "mm/khugepaged: remove definition of struct khugepaged_mm_slot"

[Lorenzo Stoakes (ARM)]

This partially reverts commit b4c9ffb54b32 ("mm/khugepaged: remove
definition of struct khugepaged_mm_slot"), as a subsequent commit needs to
resurrect this type to store additional data alongside the mm_slot.

The stated intent of the commit was to address a NULL check in
__khugepaged_exit():

	mm_slot = mm_slot_entry(slot, struct khugepaged_mm_slot, slot);

slot can be NULL and mm_slot_entry() is an alias for container_of(), so
this is container_of() a NULL value.

This was benign as there was only 1 field in khugepaged_mm_slot (offsetting
NULL at 0 results in NULL), however it was right to fix this, so retain the
fix.

It also unwrapped KMEM_CACHE() in khugepaged_init(). This was a mistake,
and was itself reverted in commit c14bdcc9f274 ("mm/khugepaged: use
KMEM_CACHE()"). Therefore, retain the KMEM_CACHE() form in this revert.

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

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index da91762f6358..aad8797f8d5a 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -66,6 +66,14 @@ static DEFINE_READ_MOSTLY_HASHTABLE(mm_slots_hash, MM_SLOTS_HASH_BITS);
 
 static struct kmem_cache *mm_slot_cache __ro_after_init;
 
+/**
+ * struct khugepaged_mm_slot - khugepaged information per mm that is being scanned
+ * @slot: hash lookup from mm to mm_slot
+ */
+struct khugepaged_mm_slot {
+	struct mm_slot slot;
+};
+
 /**
  * struct khugepaged_scan - cursor for scanning
  * @mm_head: the head of the mm list to scan
@@ -76,7 +84,7 @@ static struct kmem_cache *mm_slot_cache __ro_after_init;
  */
 struct khugepaged_scan {
 	struct list_head mm_head;
-	struct mm_slot *mm_slot;
+	struct khugepaged_mm_slot *mm_slot;
 	unsigned long address;
 };
 
@@ -386,7 +394,7 @@ int hugepage_madvise(struct vm_area_struct *vma,
 
 int __init khugepaged_init(void)
 {
-	mm_slot_cache = KMEM_CACHE(mm_slot, 0);
+	mm_slot_cache = KMEM_CACHE(khugepaged_mm_slot, 0);
 	if (!mm_slot_cache)
 		return -ENOMEM;
 
@@ -446,20 +454,22 @@ static bool hugepage_enabled(void)
 
 void __khugepaged_enter(struct mm_struct *mm)
 {
+	struct khugepaged_mm_slot *mm_slot;
 	struct mm_slot *slot;
 	int wakeup;
 
 	/* __khugepaged_exit() must not run from under us */
 	VM_BUG_ON_MM(collapse_test_exit(mm), mm);
 
-	slot = mm_slot_alloc(mm_slot_cache);
-	if (!slot)
+	mm_slot = mm_slot_alloc(mm_slot_cache);
+	if (!mm_slot)
 		return;
 
 	if (unlikely(mm_flags_test_and_set(MMF_VM_HUGEPAGE, mm))) {
-		mm_slot_free(mm_slot_cache, slot);
+		mm_slot_free(mm_slot_cache, mm_slot);
 		return;
 	}
+	slot = &mm_slot->slot;
 
 	spin_lock(&khugepaged_mm_lock);
 	mm_slot_insert(mm_slots_hash, mm, slot);
@@ -504,12 +514,15 @@ void khugepaged_enter_vma(struct vm_area_struct *vma,
 
 void __khugepaged_exit(struct mm_struct *mm)
 {
+	struct khugepaged_mm_slot *mm_slot;
 	struct mm_slot *slot;
 	int free = 0;
 
 	spin_lock(&khugepaged_mm_lock);
 	slot = mm_slot_lookup(mm_slots_hash, mm);
-	if (slot && khugepaged_scan.mm_slot != slot) {
+
+	mm_slot = slot ? mm_slot_entry(slot, struct khugepaged_mm_slot, slot) : NULL;
+	if (mm_slot && khugepaged_scan.mm_slot != mm_slot) {
 		mm_slot_remove(slot);
 		free = 1;
 	}
@@ -517,9 +530,9 @@ void __khugepaged_exit(struct mm_struct *mm)
 
 	if (free) {
 		mm_flags_clear(MMF_VM_HUGEPAGE, mm);
-		mm_slot_free(mm_slot_cache, slot);
+		mm_slot_free(mm_slot_cache, mm_slot);
 		mmdrop(mm);
-	} else if (slot) {
+	} else if (mm_slot) {
 		/*
 		 * This is required to serialize against
 		 * collapse_test_exit() (which is guaranteed to run
@@ -1773,8 +1786,9 @@ static enum scan_result collapse_scan_anon(struct vm_area_struct *vma,
 	return result;
 }
 
-static void collect_mm_slot(struct mm_slot *slot)
+static void collect_mm_slot(struct khugepaged_mm_slot *mm_slot)
 {
+	struct mm_slot *slot = &mm_slot->slot;
 	struct mm_struct *mm = slot->mm;
 
 	lockdep_assert_held(&khugepaged_mm_lock);
@@ -1790,7 +1804,7 @@ static void collect_mm_slot(struct mm_slot *slot)
 		 */
 
 		/* khugepaged_mm_lock actually not necessary for the below */
-		mm_slot_free(mm_slot_cache, slot);
+		mm_slot_free(mm_slot_cache, mm_slot);
 		mmdrop(mm);
 	}
 }
@@ -2891,6 +2905,7 @@ static void collapse_scan_mm_slot(unsigned int progress_max,
 	__acquires(&khugepaged_mm_lock)
 {
 	struct vma_iterator vmi;
+	struct khugepaged_mm_slot *mm_slot;
 	struct mm_slot *slot;
 	struct mm_struct *mm;
 	struct vm_area_struct *vma;
@@ -2900,12 +2915,14 @@ static void collapse_scan_mm_slot(unsigned int progress_max,
 	*result = SCAN_FAIL;
 
 	if (khugepaged_scan.mm_slot) {
-		slot = khugepaged_scan.mm_slot;
+		mm_slot = khugepaged_scan.mm_slot;
+		slot = &mm_slot->slot;
 	} else {
 		slot = list_first_entry(&khugepaged_scan.mm_head,
 				     struct mm_slot, mm_node);
+		mm_slot = mm_slot_entry(slot, struct khugepaged_mm_slot, slot);
 		khugepaged_scan.address = 0;
-		khugepaged_scan.mm_slot = slot;
+		khugepaged_scan.mm_slot = mm_slot;
 	}
 	spin_unlock(&khugepaged_mm_lock);
 
@@ -2981,7 +2998,7 @@ static void collapse_scan_mm_slot(unsigned int progress_max,
 breakouterloop_mmap_lock:
 
 	spin_lock(&khugepaged_mm_lock);
-	VM_BUG_ON(khugepaged_scan.mm_slot != slot);
+	VM_BUG_ON(khugepaged_scan.mm_slot != mm_slot);
 	/*
 	 * Release the current mm_slot if this mm is about to die, or
 	 * if we scanned all vmas of this mm, or THP got disabled.
@@ -2993,14 +3010,16 @@ static void collapse_scan_mm_slot(unsigned int progress_max,
 		 * mm_slot not pointing to the exiting mm.
 		 */
 		if (!list_is_last(&slot->mm_node, &khugepaged_scan.mm_head)) {
-			khugepaged_scan.mm_slot = list_next_entry(slot, mm_node);
+			slot = list_next_entry(slot, mm_node);
+			khugepaged_scan.mm_slot =
+				mm_slot_entry(slot, struct khugepaged_mm_slot, slot);
 			khugepaged_scan.address = 0;
 		} else {
 			khugepaged_scan.mm_slot = NULL;
 			khugepaged_full_scans++;
 		}
 
-		collect_mm_slot(slot);
+		collect_mm_slot(mm_slot);
 	}
 
 	trace_mm_khugepaged_scan(mm, cc->progress - progress_prev,
@@ -3090,7 +3109,7 @@ static void khugepaged_wait_work(void)
 
 static int khugepaged(void *none)
 {
-	struct mm_slot *slot;
+	struct khugepaged_mm_slot *mm_slot;
 
 	set_freezable();
 	set_user_nice(current, MAX_NICE);
@@ -3101,10 +3120,10 @@ static int khugepaged(void *none)
 	}
 
 	spin_lock(&khugepaged_mm_lock);
-	slot = khugepaged_scan.mm_slot;
+	mm_slot = khugepaged_scan.mm_slot;
 	khugepaged_scan.mm_slot = NULL;
-	if (slot)
-		collect_mm_slot(slot);
+	if (mm_slot)
+		collect_mm_slot(mm_slot);
 	spin_unlock(&khugepaged_mm_lock);
 	return 0;
 }

-- 
2.56.0

[PATCH 2/5] mm/khugepaged: scan mm's more efficiently under mmap_lock contention

[Lorenzo Stoakes (ARM)]

Ostensibly khugepaged collapse traverses eligible mm's, scanning up to
/sys/kernel/mm/transparent_hugepage/khugepaged/pages_to_scan eligible pages
at a time, sleeping for /sys/.../scan_sleep_millisecs between each
traversal.

mm's are scanned in collapse_scan_mm_slot() under the mmap read lock,
obtained using mmap_read_trylock(), skipping the mm if there is lock
contention, and dropping the lock before allocation.

khugepaged will keep scanning the same mm until it is about to be torn
down, prctl(PR_SET_THP_DISABLE) disables THP for it, mm->mmap_lock
contention occurs, or all its VMAs have been scanned.

When any of these occur, khugepaged doesn't keep track of where it left
off, as the current address is tracked in
khugepaged_scan.address (essentially a global value), which is reset.

This is a problem when lock contention occurs, as when the mm is next
scanned, it starts from address 0 again.

If there's a lot of mmap_lock contention this can significantly slow down
collapse later in the mm, since the /sys/.../pages_to_scan budget can run
out before the end of the mm is reached.

The budget is incremented (as tracked in cc->progress) at a rate of 1 page
per ineligible VMA, but the budget is only checked once eligible VMAs are
encountered.

Then, it only takes the combined total of ineligible VMAs and eligible but
non-collapsible pages to exceed the /sys/.../pages_to_scan budget for the
rest of the mm to be skipped.

For a system with 4 KiB page size and default sysctl settings this only
requires 4096 ineligible VMAs/uncollapsible pages to be skipped before this
happens.

Examples of eligible but uncollapsible ranges are those containing a large
number of forked PTE entries (more than /sys/.../max_ptes_shared), swapped
out PTE entries (more than /sys/.../max_ptes_swap) or none or zero-page PTE
entries (more than /sys/.../max_ptes_none).

The latter might seem unlikely to happen given /sys/.../max_ptes_none
defaults to HPAGE_PMD_NR - 1 and CONFIG_PT_RECLAIM frees PTE page tables,
however CONFIG_PT_RECLAIM only removes the page table if the zapped range
spans at least a PMD size (see pte_table_reclaim_possible()).

Thus, userland software that applies madvise(..., MADV_DONTNEED) across
sub-PMD ranges can thwart this, which is quite feasible for memory
allocators and GC heaps, for instance.

Resolve the issue by simply tracking the last examined address in the
khugepaged_mm_slot object, so when the mm is resumed, the scan starts from
where it left off.

Additionally, skip to the next candidate if a VMA cannot be collapsed.

This issue was noticed during some work in which an mm was instantiated
with a large number of small VMAs under significant mmap_lock contention,
during which collapse was observed to take 10.8s. With this patch applied
it took 3.6s to collapse the same mm.

Also, while here, update a couple VM_BUG_ON()'s to VM_WARN_ON_ONCE()'s.

This patch is also a pre-requisite for a later patch in this series.

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

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index aad8797f8d5a..216c4b8d28c4 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -69,23 +69,23 @@ static struct kmem_cache *mm_slot_cache __ro_after_init;
 /**
  * struct khugepaged_mm_slot - khugepaged information per mm that is being scanned
  * @slot: hash lookup from mm to mm_slot
+ * @address: the next address inside that to be scanned
  */
 struct khugepaged_mm_slot {
 	struct mm_slot slot;
+	unsigned long address;
 };
 
 /**
  * struct khugepaged_scan - cursor for scanning
  * @mm_head: the head of the mm list to scan
  * @mm_slot: the current mm_slot we are scanning
- * @address: the next address inside that to be scanned
  *
  * There is only the one khugepaged_scan instance of this cursor structure.
  */
 struct khugepaged_scan {
 	struct list_head mm_head;
 	struct khugepaged_mm_slot *mm_slot;
-	unsigned long address;
 };
 
 static struct khugepaged_scan khugepaged_scan = {
@@ -2921,7 +2921,6 @@ static void collapse_scan_mm_slot(unsigned int progress_max,
 		slot = list_first_entry(&khugepaged_scan.mm_head,
 				     struct mm_slot, mm_node);
 		mm_slot = mm_slot_entry(slot, struct khugepaged_mm_slot, slot);
-		khugepaged_scan.address = 0;
 		khugepaged_scan.mm_slot = mm_slot;
 	}
 	spin_unlock(&khugepaged_mm_lock);
@@ -2939,7 +2938,7 @@ static void collapse_scan_mm_slot(unsigned int progress_max,
 	if (unlikely(collapse_test_exit_or_disable(mm)))
 		goto breakouterloop;
 
-	vma_iter_init(&vmi, mm, khugepaged_scan.address);
+	vma_iter_init(&vmi, mm, mm_slot->address);
 	for_each_vma(vmi, vma) {
 		unsigned long hstart, hend;
 
@@ -2948,38 +2947,38 @@ static void collapse_scan_mm_slot(unsigned int progress_max,
 			cc->progress++;
 			break;
 		}
-		if (!collapse_possible_orders(vma, vma->vm_flags,
-					      TVA_KHUGEPAGED)) {
+		if (!collapse_possible_orders(vma, vma->vm_flags, TVA_KHUGEPAGED)) {
+			/* Unable to collapse, skip to the next candidate. */
+			mm_slot->address = ALIGN(vma->vm_end, HPAGE_PMD_SIZE);
 			cc->progress++;
 			continue;
 		}
 		hstart = ALIGN(vma->vm_start, HPAGE_PMD_SIZE);
 		hend = ALIGN_DOWN(vma->vm_end, HPAGE_PMD_SIZE);
-		if (khugepaged_scan.address > hend) {
+		if (mm_slot->address > hend) {
 			cc->progress++;
 			continue;
 		}
-		if (khugepaged_scan.address < hstart)
-			khugepaged_scan.address = hstart;
-		VM_BUG_ON(khugepaged_scan.address & ~HPAGE_PMD_MASK);
+		if (mm_slot->address < hstart)
+			mm_slot->address = hstart;
+		VM_WARN_ON_ONCE(mm_slot->address & ~HPAGE_PMD_MASK);
 
-		while (khugepaged_scan.address < hend) {
+		while (mm_slot->address < hend) {
 			bool lock_dropped = false;
 
 			cond_resched();
 			if (unlikely(collapse_test_exit_or_disable(mm)))
 				goto breakouterloop;
 
-			VM_WARN_ON_ONCE(khugepaged_scan.address < hstart ||
-				  khugepaged_scan.address + HPAGE_PMD_SIZE >
-				  hend);
+			VM_WARN_ON_ONCE(mm_slot->address < hstart ||
+					mm_slot->address + HPAGE_PMD_SIZE > hend);
 
-			*result = collapse_single_pmd(khugepaged_scan.address,
+			*result = collapse_single_pmd(mm_slot->address,
 						      vma, &lock_dropped, cc);
 			if (*result == SCAN_SUCCEED)
 				khugepaged_pages_collapsed++;
 			/* move to next address */
-			khugepaged_scan.address += HPAGE_PMD_SIZE;
+			mm_slot->address += HPAGE_PMD_SIZE;
 			if (lock_dropped)
 				/*
 				 * We released mmap_lock so break loop.  Note
@@ -2993,12 +2992,16 @@ static void collapse_scan_mm_slot(unsigned int progress_max,
 				goto breakouterloop;
 		}
 	}
+	/* Every VMA has been examined, so reset. */
+	if (!vma)
+		mm_slot->address = 0;
+
 breakouterloop:
 	mmap_read_unlock(mm); /* exit_mmap will destroy ptes after this */
 breakouterloop_mmap_lock:
 
 	spin_lock(&khugepaged_mm_lock);
-	VM_BUG_ON(khugepaged_scan.mm_slot != mm_slot);
+	VM_WARN_ON_ONCE(khugepaged_scan.mm_slot != mm_slot);
 	/*
 	 * Release the current mm_slot if this mm is about to die, or
 	 * if we scanned all vmas of this mm, or THP got disabled.
@@ -3013,7 +3016,6 @@ static void collapse_scan_mm_slot(unsigned int progress_max,
 			slot = list_next_entry(slot, mm_node);
 			khugepaged_scan.mm_slot =
 				mm_slot_entry(slot, struct khugepaged_mm_slot, slot);
-			khugepaged_scan.address = 0;
 		} else {
 			khugepaged_scan.mm_slot = NULL;
 			khugepaged_full_scans++;

-- 
2.56.0

[PATCH 3/5] locking/rwsem: add down_read[_killable]_nosteal()

[Lorenzo Stoakes (ARM)]

Since commit 1a728dff855a ("locking/rwsem: Enable reader optimistic lock
stealing") a sleeping, queued writer whose handoff timeout hasn't expired
can have its lock stolen by a read lock issued before a woken queued writer
was able to acquire a lock.

To avoid write lock starvation, this is only permitted if no other read
locks are held.

However, this behaviour defeats the purpose of a reader periodically
dropping the lock to allow writers in, as doing so only works if the write
lock is acquired first, another read lock is held, or a queued writer's
handoff timeout has expired.

Provide a means of indicating this intent via down_read_nosteal() and
down_read_killable_nosteal().

Implement it by exposing a steal parameter to rwsem_down_read_slowpath(),
update the starvation logic to modify this parameter and modify the steal
branch to check it first.

Also drop the duplicated RWSEM_WRITER_LOCKED check and remove the
goto as the steal condition makes this redundant.

Exposing this function is safe as it only makes the lock more pessimistic,
so does not impact lock contention.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 include/linux/rwsem.h  |  2 ++
 kernel/locking/rwsem.c | 71 +++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h
index 6a1a7bae5f81..66ab6fad5e24 100644
--- a/include/linux/rwsem.h
+++ b/include/linux/rwsem.h
@@ -227,8 +227,10 @@ static inline void rwsem_assert_held_write(const struct rw_semaphore *sem)
  * lock for reading
  */
 extern void down_read(struct rw_semaphore *sem) __acquires_shared(sem);
+void down_read_nosteal(struct rw_semaphore *sem) __acquires_shared(sem);
 extern int __must_check down_read_interruptible(struct rw_semaphore *sem) __cond_acquires_shared(0, sem);
 extern int __must_check down_read_killable(struct rw_semaphore *sem) __cond_acquires_shared(0, sem);
+int __must_check down_read_killable_nosteal(struct rw_semaphore *sem) __cond_acquires_shared(0, sem);
 
 /*
  * trylock for reading -- returns 1 if successful, 0 if contention
diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index b9c180ac1eee..75fa9fcdee91 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -1014,7 +1014,8 @@ static inline void rwsem_cond_wake_waiter(struct rw_semaphore *sem, long count,
  * Wait for the read lock to be granted
  */
 static struct rw_semaphore __sched *
-rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int state)
+rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int state,
+			 bool steal)
 {
 	long adjustment = -RWSEM_READER_BIAS;
 	long rcnt = (count >> RWSEM_READER_SHIFT);
@@ -1023,17 +1024,16 @@ rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int stat
 
 	/*
 	 * To prevent a constant stream of readers from starving a sleeping
-	 * writer, don't attempt optimistic lock stealing if the lock is
-	 * very likely owned by readers.
+	 * writer, only attempt optimistic lock stealing if the lock is
+	 * unlikely to be owned by readers.
 	 */
-	if ((atomic_long_read(&sem->owner) & RWSEM_READER_OWNED) &&
-	    (rcnt > 1) && !(count & RWSEM_WRITER_LOCKED))
-		goto queue;
+	steal &= !(atomic_long_read(&sem->owner) & RWSEM_READER_OWNED) ||
+		rcnt <= 1;
 
 	/*
 	 * Reader optimistic lock stealing.
 	 */
-	if (!(count & (RWSEM_WRITER_LOCKED | RWSEM_FLAG_HANDOFF))) {
+	if (steal && !(count & (RWSEM_WRITER_LOCKED | RWSEM_FLAG_HANDOFF))) {
 		rwsem_set_reader_owned(sem);
 		lockevent_inc(rwsem_rlock_steal);
 
@@ -1052,7 +1052,6 @@ rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int stat
 		return sem;
 	}
 
-queue:
 	waiter.task = current;
 	waiter.type = RWSEM_WAITING_FOR_READ;
 	waiter.timeout = jiffies + RWSEM_WAIT_TIMEOUT;
@@ -1281,14 +1280,15 @@ static struct rw_semaphore *rwsem_downgrade_wake(struct rw_semaphore *sem)
 /*
  * lock for reading
  */
-static __always_inline int __down_read_common(struct rw_semaphore *sem, int state)
+static __always_inline int __down_read_common(struct rw_semaphore *sem, int state,
+					      bool steal)
 {
 	int ret = 0;
 	long count;
 
 	preempt_disable();
 	if (!rwsem_read_trylock(sem, &count)) {
-		if (IS_ERR(rwsem_down_read_slowpath(sem, count, state))) {
+		if (IS_ERR(rwsem_down_read_slowpath(sem, count, state, steal))) {
 			ret = -EINTR;
 			goto out;
 		}
@@ -1301,17 +1301,27 @@ static __always_inline int __down_read_common(struct rw_semaphore *sem, int stat
 
 static __always_inline void __down_read(struct rw_semaphore *sem)
 {
-	__down_read_common(sem, TASK_UNINTERRUPTIBLE);
+	__down_read_common(sem, TASK_UNINTERRUPTIBLE, true);
+}
+
+static __always_inline void __down_read_nosteal(struct rw_semaphore *sem)
+{
+	__down_read_common(sem, TASK_UNINTERRUPTIBLE, false);
 }
 
 static __always_inline int __down_read_interruptible(struct rw_semaphore *sem)
 {
-	return __down_read_common(sem, TASK_INTERRUPTIBLE);
+	return __down_read_common(sem, TASK_INTERRUPTIBLE, true);
 }
 
 static __always_inline int __down_read_killable(struct rw_semaphore *sem)
 {
-	return __down_read_common(sem, TASK_KILLABLE);
+	return __down_read_common(sem, TASK_KILLABLE, true);
+}
+
+static __always_inline int __down_read_killable_nosteal(struct rw_semaphore *sem)
+{
+	return __down_read_common(sem, TASK_KILLABLE, false);
 }
 
 static inline int __down_read_trylock(struct rw_semaphore *sem)
@@ -1502,6 +1512,11 @@ static inline void __down_read(struct rw_semaphore *sem)
 	rwbase_read_lock(&sem->rwbase, TASK_UNINTERRUPTIBLE);
 }
 
+static inline void __down_read_nosteal(struct rw_semaphore *sem)
+{
+	__down_read(sem);
+}
+
 static inline int __down_read_interruptible(struct rw_semaphore *sem)
 {
 	return rwbase_read_lock(&sem->rwbase, TASK_INTERRUPTIBLE);
@@ -1512,6 +1527,11 @@ static inline int __down_read_killable(struct rw_semaphore *sem)
 	return rwbase_read_lock(&sem->rwbase, TASK_KILLABLE);
 }
 
+static inline int __down_read_killable_nosteal(struct rw_semaphore *sem)
+{
+	return __down_read_killable(sem);
+}
+
 static inline int __down_read_trylock(struct rw_semaphore *sem)
 {
 	return rwbase_read_trylock(&sem->rwbase);
@@ -1577,6 +1597,16 @@ void __sched down_read(struct rw_semaphore *sem)
 }
 EXPORT_SYMBOL(down_read);
 
+void __sched down_read_nosteal(struct rw_semaphore *sem)
+	__no_context_analysis
+{
+	might_sleep();
+	rwsem_acquire_read(&sem->dep_map, 0, 0, _RET_IP_);
+
+	LOCK_CONTENDED(sem, __down_read_trylock, __down_read_nosteal);
+}
+EXPORT_SYMBOL(down_read_nosteal);
+
 int __sched down_read_interruptible(struct rw_semaphore *sem)
 	__no_context_analysis
 {
@@ -1607,6 +1637,21 @@ int __sched down_read_killable(struct rw_semaphore *sem)
 }
 EXPORT_SYMBOL(down_read_killable);
 
+int __sched down_read_killable_nosteal(struct rw_semaphore *sem)
+	__no_context_analysis
+{
+	might_sleep();
+	rwsem_acquire_read(&sem->dep_map, 0, 0, _RET_IP_);
+
+	if (LOCK_CONTENDED_RETURN(sem, __down_read_trylock, __down_read_killable_nosteal)) {
+		rwsem_release(&sem->dep_map, _RET_IP_);
+		return -EINTR;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(down_read_killable_nosteal);
+
 /*
  * trylock for reading -- returns 1 if successful, 0 if contention
  */

-- 
2.56.0

[PATCH 4/5] mm: add and use mmap_read_lock[_killable]_nosteal() on contention

[Lorenzo Stoakes (ARM)]

With down_read[_killable]_nosteal() available, it becomes possible to
release the mmap read lock in such a way that a queued write lock always
takes precedence.

Expose this to mm through two new wrappers - mmap_read_lock_nosteal() and
mmap_read_lock_killable_nosteal().

Make use of this in scenarios where the mmap read lock is dropped
explicitly for the purposes of relieving mmap_lock contention - the BPF
task VMA iterator, and lock_next_vma_under_mmap_lock(), which is where
/proc/$pid/smaps_rollup re-acquires the mmap lock after dropping it on
contention and falling back from per-VMA locking.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 include/linux/mmap_lock.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
 kernel/bpf/task_iter.c    |  2 +-
 mm/mmap_lock.c            |  2 +-
 3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
index 9ccbbcbd7aca..88906ecd6ca3 100644
--- a/include/linux/mmap_lock.h
+++ b/include/linux/mmap_lock.h
@@ -627,6 +627,31 @@ static inline int __must_check mmap_read_lock_killable(struct mm_struct *mm)
 	return ret;
 }
 
+/**
+ * mmap_read_lock_killable_nosteal() - Acquire a killable mmap read lock, but
+ * yield to any queued write locks.
+ * @mm: The mm whose mmap read lock is required.
+ *
+ * If an mmap write lock is queued behind readers, but its handoff timeout has
+ * yet to expire, releasing then re-acquiring a read lock can steal the lock
+ * from it.
+ *
+ * mmap_read_lock_killable_nosteal() will queue in this case instead, which is
+ * useful when releasing the mmap read lock specifically to relieve lock
+ * contention.
+ *
+ * Returns: 0 on success, or an error otherwise.
+ */
+static inline int __must_check mmap_read_lock_killable_nosteal(struct mm_struct *mm)
+{
+	int ret;
+
+	__mmap_lock_trace_start_locking(mm, false);
+	ret = down_read_killable_nosteal(&mm->mmap_lock);
+	__mmap_lock_trace_acquire_returned(mm, false, ret == 0);
+	return ret;
+}
+
 static inline bool __must_check mmap_read_trylock(struct mm_struct *mm)
 {
 	bool ret;
@@ -637,6 +662,25 @@ static inline bool __must_check mmap_read_trylock(struct mm_struct *mm)
 	return ret;
 }
 
+/**
+ * mmap_read_lock_nosteal() - Acquire the mmap read lock, but yield to
+ * any queued write locks.
+ * @mm: The mm whose mmap read lock is required.
+ *
+ * If an mmap write lock is queued behind readers, but its handoff timeout has
+ * yet to expire, releasing then re-acquiring a read lock can steal the lock
+ * from it.
+ *
+ * mmap_read_lock_nosteal() will queue in this case instead, which is useful when
+ * releasing the mmap read lock specifically to relieve lock contention.
+ */
+static inline void mmap_read_lock_nosteal(struct mm_struct *mm)
+{
+	__mmap_lock_trace_start_locking(mm, false);
+	down_read_nosteal(&mm->mmap_lock);
+	__mmap_lock_trace_acquire_returned(mm, false, true);
+}
+
 static inline void mmap_read_unlock(struct mm_struct *mm)
 {
 	__mmap_lock_trace_released(mm, false);
diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index 2ee7b3b3c10b..b455306a6b52 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -486,7 +486,7 @@ task_vma_seq_get_next(struct bpf_iter_seq_task_vma_info *info)
 			info->prev_vm_end = curr_vma->vm_end;
 			op = task_vma_iter_find_vma;
 			mmap_read_unlock(curr_mm);
-			if (mmap_read_lock_killable(curr_mm)) {
+			if (mmap_read_lock_killable_nosteal(curr_mm)) {
 				mmput(curr_mm);
 				goto finish;
 			}
diff --git a/mm/mmap_lock.c b/mm/mmap_lock.c
index 834bbf86e004..41b11b843211 100644
--- a/mm/mmap_lock.c
+++ b/mm/mmap_lock.c
@@ -412,7 +412,7 @@ static struct vm_area_struct *lock_next_vma_under_mmap_lock(struct mm_struct *mm
 	struct vm_area_struct *vma;
 	int ret;
 
-	ret = mmap_read_lock_killable(mm);
+	ret = mmap_read_lock_killable_nosteal(mm);
 	if (ret)
 		return ERR_PTR(ret);
 

-- 
2.56.0

[PATCH 5/5] mm/khugepaged: drop/reacquire mmap read lock if skipped enough VMAs

[Lorenzo Stoakes (ARM)]

khugepaged walks an mm's VMAs under the mmap read lock in
collapse_scan_mm_slot(), checking for ineligible VMAs first, and skipping
them if encountered.

It charges cc->progress 1 page against the 8 * HPAGE_PMD_NR budget when
these are encountered, however only checking this budget once an eligible
PMD-spanning VMA is encountered.

It can therefore hold the mmap read lock for a long time if there are a
large number of ineligible VMAs present in an mm, over which time no mmap
write locks (and thus no VMA write locks) can be acquired.

Address this by dropping + re-acquiring the mmap read lock after
KHUGEPAGED_MAX_SKIP_BATCH VMAs have been processed. Use the newly
introduced mmap_read_lock_nosteal() to ensure queued writers take
precedence when doing so.

This was tested locally with ~1M ineligible VMAs mapped ahead of a
collapsible region of memory, and resulted in a reduced mmap_lock hold time
of 170us down from 95ms.

This was also observed to reduce mmap_lock stalls in concurrent threads of
a similar duration.

KHUGEPAGED_MAX_SKIP_BATCH was derived empirically to a conservative value
of 1024 VMAs, which should not interfere with mm's that have an ordinary
number of VMAs.

Local measurements indicated no change for ordinary use cases, so this is
all upside.

This work came out of investigations into mmap stalls when getting latency
numbers for an unrelated patch series.

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

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 216c4b8d28c4..b9c72ee8b421 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -66,6 +66,12 @@ 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 with nothing to scan to walk before dropping and
+ * reacquiring the mmap_lock to relieve lock contention.
+ */
+#define KHUGEPAGED_MAX_SKIP_BATCH 1024
+
 /**
  * struct khugepaged_mm_slot - khugepaged information per mm that is being scanned
  * @slot: hash lookup from mm to mm_slot
@@ -2910,6 +2916,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 skipped = 0;
 
 	lockdep_assert_held(&khugepaged_mm_lock);
 	*result = SCAN_FAIL;
@@ -2951,6 +2958,14 @@ static void collapse_scan_mm_slot(unsigned int progress_max,
 			/* Unable to collapse, skip to the next candidate. */
 			mm_slot->address = ALIGN(vma->vm_end, HPAGE_PMD_SIZE);
 			cc->progress++;
+			if (++skipped >= KHUGEPAGED_MAX_SKIP_BATCH) {
+				/* Skipped a lot of VMAs, so wait for writers. */
+				mmap_read_unlock(mm);
+				cond_resched();
+				mmap_read_lock_nosteal(mm);
+				vma_iter_set(&vmi, mm_slot->address);
+				skipped = 0;
+			}
 			continue;
 		}
 		hstart = ALIGN(vma->vm_start, HPAGE_PMD_SIZE);

-- 
2.56.0

Re: [PATCH 3/5] locking/rwsem: add down_read[_killable]_nosteal()

[Peter Zijlstra]

On Tue, Nov 03, 2026 at 08:27:44AM +0000, Lorenzo Stoakes (ARM) wrote:
> Since commit 1a728dff855a ("locking/rwsem: Enable reader optimistic lock
> stealing") a sleeping, queued writer whose handoff timeout hasn't expired
> can have its lock stolen by a read lock issued before a woken queued writer
> was able to acquire a lock.
> 
> To avoid write lock starvation, this is only permitted if no other read
> locks are held.
> 
> However, this behaviour defeats the purpose of a reader periodically
> dropping the lock to allow writers in, as doing so only works if the write
> lock is acquired first, another read lock is held, or a queued writer's
> handoff timeout has expired.
> 
> Provide a means of indicating this intent via down_read_nosteal() and
> down_read_killable_nosteal().
> 
> Implement it by exposing a steal parameter to rwsem_down_read_slowpath(),
> update the starvation logic to modify this parameter and modify the steal
> branch to check it first.
> 
> Also drop the duplicated RWSEM_WRITER_LOCKED check and remove the
> goto as the steal condition makes this redundant.
> 
> Exposing this function is safe as it only makes the lock more pessimistic,
> so does not impact lock contention.

Not generally a fan of this -- can't we tweak the steal thing to never
steal if there are pending writers?

Re: [PATCH 3/5] locking/rwsem: add down_read[_killable]_nosteal()

[Lorenzo Stoakes (ARM)]

On Tue, Nov 03, 2026 at 12:51:59PM +0100, Peter Zijlstra wrote:
> On Tue, Nov 03, 2026 at 08:27:44AM +0000, Lorenzo Stoakes (ARM) wrote:
> > Since commit 1a728dff855a ("locking/rwsem: Enable reader optimistic lock
> > stealing") a sleeping, queued writer whose handoff timeout hasn't expired
> > can have its lock stolen by a read lock issued before a woken queued writer
> > was able to acquire a lock.
> >
> > To avoid write lock starvation, this is only permitted if no other read
> > locks are held.
> >
> > However, this behaviour defeats the purpose of a reader periodically
> > dropping the lock to allow writers in, as doing so only works if the write
> > lock is acquired first, another read lock is held, or a queued writer's
> > handoff timeout has expired.
> >
> > Provide a means of indicating this intent via down_read_nosteal() and
> > down_read_killable_nosteal().
> >
> > Implement it by exposing a steal parameter to rwsem_down_read_slowpath(),
> > update the starvation logic to modify this parameter and modify the steal
> > branch to check it first.
> >
> > Also drop the duplicated RWSEM_WRITER_LOCKED check and remove the
> > goto as the steal condition makes this redundant.
> >
> > Exposing this function is safe as it only makes the lock more pessimistic,
> > so does not impact lock contention.
>
> Not generally a fan of this -- can't we tweak the steal thing to never
> steal if there are pending writers?

Oh, well, I had assumed I couldn't fiddle with that behaviour across
rwsem's in general, so if I can yeah that's preferable!

I could send that change separately then and drop it out of this series.

Something like this?

diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index b9c180ac1eee..d6572e207523 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -1034,22 +1034,29 @@ rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int stat
 	 * Reader optimistic lock stealing.
 	 */
 	if (!(count & (RWSEM_WRITER_LOCKED | RWSEM_FLAG_HANDOFF))) {
-		rwsem_set_reader_owned(sem);
-		lockevent_inc(rwsem_rlock_steal);
+		bool steal = true;

-		/*
-		 * Wake up other readers in the wait queue if it is
-		 * the first reader.
-		 */
-		if ((rcnt == 1) && (count & RWSEM_FLAG_WAITERS)) {
+		if (count & RWSEM_FLAG_WAITERS) {
 			raw_spin_lock_irq(&sem->wait_lock);
-			if (sem->first_waiter)
+			first = sem->first_waiter;
+			if (first && first->type == RWSEM_WAITING_FOR_WRITE) {
+				/* Writers always take precedence. */
+				steal = false;
+			} else if (first && rcnt == 1) {
+				/* First reader, wake other readers in queue. */
 				rwsem_mark_wake(sem, RWSEM_WAKE_READ_OWNED,
 						&wake_q);
+			}
 			raw_spin_unlock_irq(&sem->wait_lock);
-			wake_up_q(&wake_q);
 		}
-		return sem;
+
+		if (steal) {
+			rwsem_set_reader_owned(sem);
+			lockevent_inc(rwsem_rlock_steal);
+			if (!wake_q_empty(&wake_q))
+				wake_up_q(&wake_q);
+			return sem;
+		}
 	}

 queue:
--
Cheers, Lorenzo