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

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

On Wed, Nov 11, 2026 at 10:27:50AM +0100, David Hildenbrand (Arm) wrote:
> On 11/10/26 18:19, Lorenzo Stoakes (ARM) wrote:
> > 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 /sys/.../pages_to_scan 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 checking whether the mmap lock is contended after 1-10
> > ms (depending on CONFIG_HZ), i.e. the granularity of jiffies. This is a
> > long time for a lock to be held, so is a reasonable compromise for
> > determining when best to do this.
> >
> > Skipping an ineligible VMA costs ~100 ns on tested hardware (x86 server),
> > so a batch of 1024 bounds the lock hold to ~100 us without expensive
> > high-grained timer checks.
> >
> > This scales for large numbers of VMAs, and tests with ~1M ineligible VMAs
> > reduced the maximum mmap read lock hold time from ~93ms to ~110us.
> >
> > This was also observed to reduce mmap_lock stalls in concurrent threads of
> > a similar duration. Note that a reader re-acquiring the lock can currently
> > steal it from a writer that is queued but not yet running, so the full
> > benefit to writers depends on the linked rwsem change.
> >
> > 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.
> >
> > Link: https://lore.kernel.org/all/20261105-rwsem-dont-steal-from-queued-writer-v1-1-275e070f426f@kernel.org/
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > ---
> >  mm/khugepaged.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > 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;
>
>
> We could simplify by just doing
>
> 	if (++nr_vmas_processed % KHUGEPAGED_MAX_VMA_SCAN_BATCH == 0)
>
> I guess. saves one LOC ;)

Could do, but I kinda like the 'keep it simple' approach often :P

>
> > +			if (mmap_lock_is_contended(mm)) {
> > +				mmap_read_unlock(mm);
> > +				cond_resched();
> > +				mmap_read_lock(mm);
> > +				vma_iter_set(&vmi, mm_slot->address);
> > +				continue;
> > +			}
> > +		}
> > +
> Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>

Thanks! :)

--
Cheers, Lorenzo