* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-05 20:57 ` [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs Lorenzo Stoakes (ARM)
@ 2026-11-05 21:53 ` Jose A. Perez de Azpillaga
2026-11-06 10:03 ` Lorenzo Stoakes (ARM)
2026-11-06 2:12 ` Vernon Yang
` (3 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: Jose A. Perez de Azpillaga @ 2026-11-05 21:53 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM), David Hildenbrand, Zi Yan, Baolin Wang,
Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Usama Arif, Kiryl Shutsemau, Andrew Morton
Cc: linux-mm, linux-kernel
On 11/5/26 9:57 PM, 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 every
> KHUGEPAGED_MAX_VMA_SCAN_BATCH VMAs walked, that is a quarter of the
> pages_to_scan budget, and if so dropping and re-acquiring the mmap read
> lock before resuming the walk from the per-mm cursor.
>
> As the walk is resumed from the cursor rather than the VMA that was current
> when the lock was dropped, the cursor must never move backwards. A VMA
> ending at the top of the address space causes ALIGN() to wrap the cursor to
> zero, so treat such a VMA as the last in the mm rather than skipping past
> it.
>
> 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. 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 separate rwsem change at [0].
>
> 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.
>
> [0]: 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 | 31 +++++++++++++++++++++++++++++--
> 1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 216c4b8d28c4..eaa01dae5a03 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 to walk before checking for, and relieving, mmap lock
> + * contention, as a function of the pages_to_scan budget.
> + */
> +#define KHUGEPAGED_MAX_VMA_SCAN_BATCH(pages_to_scan) ((pages_to_scan) / 4)
> +
> /**
> * 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 nr_vmas_batch = 0;
>
> lockdep_assert_held(&khugepaged_mm_lock);
> *result = SCAN_FAIL;
> @@ -2942,15 +2949,35 @@ 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(progress_max)) {
> + 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;
> + }
> + }
> +
with pages_to_scan below 4 the batch is 0, so under constant contention
we keep dropping the lock at the cursor and never scan anything, don't we?
--
cheers,
jose a. p-a
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-05 21:53 ` Jose A. Perez de Azpillaga
@ 2026-11-06 10:03 ` Lorenzo Stoakes (ARM)
2026-11-06 10:29 ` Jose A. Perez de Azpillaga
0 siblings, 1 reply; 24+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-11-06 10:03 UTC (permalink / raw)
To: Jose A. Perez de Azpillaga
Cc: David Hildenbrand, Zi Yan, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton, linux-mm,
linux-kernel
On Thu, Nov 05, 2026 at 10:53:13PM +0100, Jose A. Perez de Azpillaga wrote:
> On 11/5/26 9:57 PM, 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 every
> > KHUGEPAGED_MAX_VMA_SCAN_BATCH VMAs walked, that is a quarter of the
> > pages_to_scan budget, and if so dropping and re-acquiring the mmap read
> > lock before resuming the walk from the per-mm cursor.
> >
> > As the walk is resumed from the cursor rather than the VMA that was current
> > when the lock was dropped, the cursor must never move backwards. A VMA
> > ending at the top of the address space causes ALIGN() to wrap the cursor to
> > zero, so treat such a VMA as the last in the mm rather than skipping past
> > it.
> >
> > 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. 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 separate rwsem change at [0].
> >
> > 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.
> >
> > [0]: 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 | 31 +++++++++++++++++++++++++++++--
> > 1 file changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 216c4b8d28c4..eaa01dae5a03 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 to walk before checking for, and relieving, mmap lock
> > + * contention, as a function of the pages_to_scan budget.
> > + */
> > +#define KHUGEPAGED_MAX_VMA_SCAN_BATCH(pages_to_scan) ((pages_to_scan) / 4)
> > +
> > /**
> > * 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 nr_vmas_batch = 0;
> >
> > lockdep_assert_held(&khugepaged_mm_lock);
> > *result = SCAN_FAIL;
> > @@ -2942,15 +2949,35 @@ 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(progress_max)) {
> > + 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;
> > + }
> > + }
> > +
>
> with pages_to_scan below 4 the batch is 0, so under constant contention
> we keep dropping the lock at the cursor and never scan anything, don't we?
This feels like repackaged AI review...
The mmap lock won't be eternally contended and if it is, with pages_to_scan
under 4, I think that's fine.
Alternatively could make that >= an == and the post-++ a pre-++ and the 0 will
never be hit in this case.
Or could have a minimum cap.
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-06 10:03 ` Lorenzo Stoakes (ARM)
@ 2026-11-06 10:29 ` Jose A. Perez de Azpillaga
2026-11-06 10:33 ` Lorenzo Stoakes (ARM)
0 siblings, 1 reply; 24+ messages in thread
From: Jose A. Perez de Azpillaga @ 2026-11-06 10:29 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: David Hildenbrand, Zi Yan, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton, linux-mm,
linux-kernel
On 11/6/26 11:03 AM, Lorenzo Stoakes (ARM) wrote:
> On Thu, Nov 05, 2026 at 10:53:13PM +0100, Jose A. Perez de Azpillaga wrote:
>> On 11/5/26 9:57 PM, 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 every
>>> KHUGEPAGED_MAX_VMA_SCAN_BATCH VMAs walked, that is a quarter of the
>>> pages_to_scan budget, and if so dropping and re-acquiring the mmap read
>>> lock before resuming the walk from the per-mm cursor.
>>>
>>> As the walk is resumed from the cursor rather than the VMA that was current
>>> when the lock was dropped, the cursor must never move backwards. A VMA
>>> ending at the top of the address space causes ALIGN() to wrap the cursor to
>>> zero, so treat such a VMA as the last in the mm rather than skipping past
>>> it.
>>>
>>> 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. 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 separate rwsem change at [0].
>>>
>>> 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.
>>>
>>> [0]: 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 | 31 +++++++++++++++++++++++++++++--
>>> 1 file changed, 29 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index 216c4b8d28c4..eaa01dae5a03 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 to walk before checking for, and relieving, mmap lock
>>> + * contention, as a function of the pages_to_scan budget.
>>> + */
>>> +#define KHUGEPAGED_MAX_VMA_SCAN_BATCH(pages_to_scan) ((pages_to_scan) / 4)
>>> +
>>> /**
>>> * 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 nr_vmas_batch = 0;
>>>
>>> lockdep_assert_held(&khugepaged_mm_lock);
>>> *result = SCAN_FAIL;
>>> @@ -2942,15 +2949,35 @@ 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(progress_max)) {
>>> + 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;
>>> + }
>>> + }
>>> +
>>
>> with pages_to_scan below 4 the batch is 0, so under constant contention
>> we keep dropping the lock at the cursor and never scan anything, don't we?
>
> This feels like repackaged AI review...
>
> The mmap lock won't be eternally contended and if it is, with pages_to_scan
> under 4, I think that's fine.
>
> Alternatively could make that >= an == and the post-++ a pre-++ and the 0 will
> never be hit in this case.
>
> Or could have a minimum cap.
with >= a minimum cap needs to be 2, since the cursor can stop at hend
inside the VMA we just scanned and the walk lands on it again after the
drop. with == only batch - 1 VMAs get scanned per cycle, so a batch of 1
scans nothing, today's batch-0 loop at pages_to_scan 4..7, and a batch
of 2 hits the hend case. it's safe from 3 up.
--
cheers,
jose a. p-a
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-06 10:29 ` Jose A. Perez de Azpillaga
@ 2026-11-06 10:33 ` Lorenzo Stoakes (ARM)
2026-11-06 10:49 ` Jose A. Perez de Azpillaga
0 siblings, 1 reply; 24+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-11-06 10:33 UTC (permalink / raw)
To: Jose A. Perez de Azpillaga
Cc: David Hildenbrand, Zi Yan, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton, linux-mm,
linux-kernel
On Fri, Nov 06, 2026 at 11:29:00AM +0100, Jose A. Perez de Azpillaga wrote:
> >> with pages_to_scan below 4 the batch is 0, so under constant contention
> >> we keep dropping the lock at the cursor and never scan anything, don't we?
> >
> > This feels like repackaged AI review...
> >
> > The mmap lock won't be eternally contended and if it is, with pages_to_scan
> > under 4, I think that's fine.
> >
> > Alternatively could make that >= an == and the post-++ a pre-++ and the 0 will
> > never be hit in this case.
> >
> > Or could have a minimum cap.
>
> with >= a minimum cap needs to be 2, since the cursor can stop at hend
> inside the VMA we just scanned and the walk lands on it again after the
> drop. with == only batch - 1 VMAs get scanned per cycle, so a batch of 1
> scans nothing, today's batch-0 loop at pages_to_scan 4..7, and a batch
> of 2 hits the hend case. it's safe from 3 up.
Please don't copy/paste LLM responses. I'm sure it's in good faith, but it's not
helpful. I will run any changes through AI myself.
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-06 10:33 ` Lorenzo Stoakes (ARM)
@ 2026-11-06 10:49 ` Jose A. Perez de Azpillaga
0 siblings, 0 replies; 24+ messages in thread
From: Jose A. Perez de Azpillaga @ 2026-11-06 10:49 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: David Hildenbrand, Zi Yan, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton, linux-mm,
linux-kernel
On 11/6/26 11:33 AM, Lorenzo Stoakes (ARM) wrote:
> On Fri, Nov 06, 2026 at 11:29:00AM +0100, Jose A. Perez de Azpillaga wrote:
>>>> with pages_to_scan below 4 the batch is 0, so under constant contention
>>>> we keep dropping the lock at the cursor and never scan anything, don't we?
>>>
>>> This feels like repackaged AI review...
>>>
>>> The mmap lock won't be eternally contended and if it is, with pages_to_scan
>>> under 4, I think that's fine.
>>>
>>> Alternatively could make that >= an == and the post-++ a pre-++ and the 0 will
>>> never be hit in this case.
>>>
>>> Or could have a minimum cap.
>>
>> with >= a minimum cap needs to be 2, since the cursor can stop at hend
>> inside the VMA we just scanned and the walk lands on it again after the
>> drop. with == only batch - 1 VMAs get scanned per cycle, so a batch of 1
>> scans nothing, today's batch-0 loop at pages_to_scan 4..7, and a batch
>> of 2 hits the hend case. it's safe from 3 up.
>
> Please don't copy/paste LLM responses. I'm sure it's in good faith, but it's not
> helpful. I will run any changes through AI myself.
it wasn't my intention. I am bad at writing reviews, so AI assists me to
reword it, so it's polished, and I still check all the output. now
onward, maybe my reviews get a bit clumsy, and not polished. I will stop
rewording them.
--
cheers,
jose a. p-a
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-05 20:57 ` [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs Lorenzo Stoakes (ARM)
2026-11-05 21:53 ` Jose A. Perez de Azpillaga
@ 2026-11-06 2:12 ` Vernon Yang
2026-11-06 9:29 ` Vernon Yang
2026-11-06 9:38 ` Baolin Wang
` (2 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: Vernon Yang @ 2026-11-06 2:12 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: David Hildenbrand, Zi Yan, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton, linux-mm,
linux-kernel
On Thu, Nov 05, 2026 at 08:57:24PM +0000, 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 every
> KHUGEPAGED_MAX_VMA_SCAN_BATCH VMAs walked, that is a quarter of the
> pages_to_scan budget, and if so dropping and re-acquiring the mmap read
> lock before resuming the walk from the per-mm cursor.
>
> As the walk is resumed from the cursor rather than the VMA that was current
> when the lock was dropped, the cursor must never move backwards. A VMA
> ending at the top of the address space causes ALIGN() to wrap the cursor to
> zero, so treat such a VMA as the last in the mm rather than skipping past
> it.
>
> 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. 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 separate rwsem change at [0].
>
> 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.
>
> [0]: 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 | 31 +++++++++++++++++++++++++++++--
> 1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 216c4b8d28c4..eaa01dae5a03 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 to walk before checking for, and relieving, mmap lock
> + * contention, as a function of the pages_to_scan budget.
> + */
> +#define KHUGEPAGED_MAX_VMA_SCAN_BATCH(pages_to_scan) ((pages_to_scan) / 4)
> +
When pages_to_scan to equate 1/2/3, KHUGEPAGED_MAX_VMA_SCAN_BATCH(progress_max)
always to 0, we will never scan anything.
The following code will solve this problem.
#define KHUGEPAGED_MAX_VMA_SCAN_BATCH(pages_to_scan) max((pages_to_scan) / 4, 1)
> /**
> * 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 nr_vmas_batch = 0;
>
> lockdep_assert_held(&khugepaged_mm_lock);
> *result = SCAN_FAIL;
> @@ -2942,15 +2949,35 @@ 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(progress_max)) {
> + 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++;
> break;
> }
> 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);
> + const unsigned long next =
> + ALIGN(vma->vm_end, HPAGE_PMD_SIZE);
> +
> cc->progress++;
> + /* Check for overflow. */
> + if (unlikely(next < vma->vm_end)) {
I think you want it to be like that.
if (unlikely(next >= vma->vm_end)) {
^^
> + vma = NULL;
> + break;
> + }
> + /* Unable to collapse, skip to the next candidate. */
> + mm_slot->address = next;
> continue;
> }
> hstart = ALIGN(vma->vm_start, HPAGE_PMD_SIZE);
>
> --
> 2.56.0
>
>
--
Cheers,
Vernon
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-06 2:12 ` Vernon Yang
@ 2026-11-06 9:29 ` Vernon Yang
2026-11-06 10:49 ` Lorenzo Stoakes (ARM)
0 siblings, 1 reply; 24+ messages in thread
From: Vernon Yang @ 2026-11-06 9:29 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: David Hildenbrand, Zi Yan, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton, linux-mm,
linux-kernel
On Fri, Nov 06, 2026 at 10:12:46AM +0800, Vernon Yang wrote:
> On Thu, Nov 05, 2026 at 08:57:24PM +0000, Lorenzo Stoakes (ARM) wrote:
> > 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);
> > + const unsigned long next =
> > + ALIGN(vma->vm_end, HPAGE_PMD_SIZE);
> > +
> > cc->progress++;
> > + /* Check for overflow. */
> > + if (unlikely(next < vma->vm_end)) {
>
> I think you want it to be like that.
>
> if (unlikely(next >= vma->vm_end)) {
> ^^
Don’t read this comment please. Your code is right.
The VMA at the top of mm, after aligning the end values according to the
PMD, may overflow to 0.
It might be easier to understand if this coding is used.
if (unlikely(next == 0)) {
--
Cheers,
Vernon
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-06 9:29 ` Vernon Yang
@ 2026-11-06 10:49 ` Lorenzo Stoakes (ARM)
0 siblings, 0 replies; 24+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-11-06 10:49 UTC (permalink / raw)
To: Vernon Yang
Cc: David Hildenbrand, Zi Yan, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton, linux-mm,
linux-kernel
On Fri, Nov 06, 2026 at 05:29:45PM +0800, Vernon Yang wrote:
> On Fri, Nov 06, 2026 at 10:12:46AM +0800, Vernon Yang wrote:
> > On Thu, Nov 05, 2026 at 08:57:24PM +0000, Lorenzo Stoakes (ARM) wrote:
> > > 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);
> > > + const unsigned long next =
> > > + ALIGN(vma->vm_end, HPAGE_PMD_SIZE);
> > > +
> > > cc->progress++;
> > > + /* Check for overflow. */
> > > + if (unlikely(next < vma->vm_end)) {
> >
> > I think you want it to be like that.
> >
> > if (unlikely(next >= vma->vm_end)) {
> > ^^
>
> Don’t read this comment please. Your code is right.
>
> The VMA at the top of mm, after aligning the end values according to the
> PMD, may overflow to 0.
>
> It might be easier to understand if this coding is used.
>
> if (unlikely(next == 0)) {
I'd rather keep < as it's the general form used for overflow checks and I don't
want to bake in assumptions about the nature of the overflow or make somebody
have to reason about that :)
>
> --
> Cheers,
> Vernon
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-05 20:57 ` [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs Lorenzo Stoakes (ARM)
2026-11-05 21:53 ` Jose A. Perez de Azpillaga
2026-11-06 2:12 ` Vernon Yang
@ 2026-11-06 9:38 ` Baolin Wang
2026-11-06 10:52 ` Lorenzo Stoakes (ARM)
2026-11-06 12:07 ` Lorenzo Stoakes (ARM)
2026-11-06 20:58 ` sashiko-bot
4 siblings, 1 reply; 24+ messages in thread
From: Baolin Wang @ 2026-11-06 9:38 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM), David Hildenbrand, Zi Yan, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton
Cc: linux-mm, linux-kernel
On 11/6/26 4:57 AM, 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 every
> KHUGEPAGED_MAX_VMA_SCAN_BATCH VMAs walked, that is a quarter of the
> pages_to_scan budget, and if so dropping and re-acquiring the mmap read
> lock before resuming the walk from the per-mm cursor.
>
> As the walk is resumed from the cursor rather than the VMA that was current
> when the lock was dropped, the cursor must never move backwards. A VMA
> ending at the top of the address space causes ALIGN() to wrap the cursor to
> zero, so treat such a VMA as the last in the mm rather than skipping past
> it.
>
> 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. 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 separate rwsem change at [0].
>
> 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.
>
> [0]: 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>
> ---
[snip]
> 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);
> + const unsigned long next =
> + ALIGN(vma->vm_end, HPAGE_PMD_SIZE);
> +
> cc->progress++;
> + /* Check for overflow. */
> + if (unlikely(next < vma->vm_end)) {
> + vma = NULL;
> + break;
> + }
> + /* Unable to collapse, skip to the next candidate. */
> + mm_slot->address = next;
> continue;
> }
IMO, these changes seem to belong in patch 2?
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-06 9:38 ` Baolin Wang
@ 2026-11-06 10:52 ` Lorenzo Stoakes (ARM)
0 siblings, 0 replies; 24+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-11-06 10:52 UTC (permalink / raw)
To: Baolin Wang
Cc: David Hildenbrand, Zi Yan, Liam R. Howlett, Nico Pache,
Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif,
Kiryl Shutsemau, Andrew Morton, linux-mm, linux-kernel
On Fri, Nov 06, 2026 at 05:38:27PM +0800, Baolin Wang wrote:
> > 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);
> > + const unsigned long next =
> > + ALIGN(vma->vm_end, HPAGE_PMD_SIZE);
> > +
> > cc->progress++;
> > + /* Check for overflow. */
> > + if (unlikely(next < vma->vm_end)) {
> > + vma = NULL;
> > + break;
> > + }
> > + /* Unable to collapse, skip to the next candidate. */
> > + mm_slot->address = next;
> > continue;
> > }
>
> IMO, these changes seem to belong in patch 2?
You're right, will move them there on next respin, thanks!
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-05 20:57 ` [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs Lorenzo Stoakes (ARM)
` (2 preceding siblings ...)
2026-11-06 9:38 ` Baolin Wang
@ 2026-11-06 12:07 ` Lorenzo Stoakes (ARM)
2026-11-06 12:23 ` Lorenzo Stoakes (ARM)
2026-11-06 13:26 ` Vernon Yang
2026-11-06 20:58 ` sashiko-bot
4 siblings, 2 replies; 24+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-11-06 12:07 UTC (permalink / raw)
To: David Hildenbrand, Zi Yan, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton
Cc: linux-mm, linux-kernel
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.
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-06 12:07 ` Lorenzo Stoakes (ARM)
@ 2026-11-06 12:23 ` Lorenzo Stoakes (ARM)
2026-11-06 13:26 ` Vernon Yang
1 sibling, 0 replies; 24+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-11-06 12:23 UTC (permalink / raw)
To: David Hildenbrand, Zi Yan, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton
Cc: linux-mm, linux-kernel
On Fri, Nov 06, 2026 at 12:07:53PM +0000, 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;
The rare unsigned int long type, rarely deployed but super effective when it is
:P
(sorry losing my mind, should be an unsigned long)
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-06 12:07 ` Lorenzo Stoakes (ARM)
2026-11-06 12:23 ` Lorenzo Stoakes (ARM)
@ 2026-11-06 13:26 ` Vernon Yang
2026-11-06 15:24 ` Lorenzo Stoakes (ARM)
1 sibling, 1 reply; 24+ messages in thread
From: Vernon Yang @ 2026-11-06 13:26 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: David Hildenbrand, Zi Yan, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton, linux-mm,
linux-kernel
On Fri, Nov 06, 2026 at 12:07:53PM +0000, 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.
Looks good to me. Is the test result of this solution the same as the
current one?
--
Cheers,
Vernon
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-06 13:26 ` Vernon Yang
@ 2026-11-06 15:24 ` Lorenzo Stoakes (ARM)
2026-11-06 15:39 ` Lorenzo Stoakes (ARM)
0 siblings, 1 reply; 24+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-11-06 15:24 UTC (permalink / raw)
To: Vernon Yang
Cc: David Hildenbrand, Zi Yan, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton, linux-mm,
linux-kernel
On Fri, Nov 06, 2026 at 09:26:01PM +0800, Vernon Yang wrote:
> On Fri, Nov 06, 2026 at 12:07:53PM +0000, 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.
>
> Looks good to me. Is the test result of this solution the same as the
> current one?
Thanks! I will update the commit to reflect the changes.
>
> --
> Cheers,
> Vernon
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-06 15:24 ` Lorenzo Stoakes (ARM)
@ 2026-11-06 15:39 ` Lorenzo Stoakes (ARM)
0 siblings, 0 replies; 24+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-11-06 15:39 UTC (permalink / raw)
To: Vernon Yang
Cc: David Hildenbrand, Zi Yan, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, Kiryl Shutsemau, Andrew Morton, linux-mm,
linux-kernel
On Fri, Nov 06, 2026 at 03:24:20PM +0000, Lorenzo Stoakes (ARM) wrote:
> On Fri, Nov 06, 2026 at 09:26:01PM +0800, Vernon Yang wrote:
> > On Fri, Nov 06, 2026 at 12:07:53PM +0000, 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.
> >
> > Looks good to me. Is the test result of this solution the same as the
> > current one?
>
> Thanks! I will update the commit to reflect the changes.
Hmm, actually the poor granularity of jiffies makes things measurably worse.
I think I have to just use a fixed constant still, and explain how it was
derived.
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs
2026-11-05 20:57 ` [PATCH v2 3/3] mm/khugepaged: release contended mmap read lock after scanning enough VMAs Lorenzo Stoakes (ARM)
` (3 preceding siblings ...)
2026-11-06 12:07 ` Lorenzo Stoakes (ARM)
@ 2026-11-06 20:58 ` sashiko-bot
4 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-11-06 20:58 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM), David Hildenbrand, Zi Yan, Baolin Wang,
Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Usama Arif, Kiryl Shutsemau, Andrew Morton
Cc: linux-mm
Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] mm/khugepaged: lack of forward progress in
collapse_scan_mm_slot()
- [Medium] mm/khugepaged: sub-PMD mTHP VMAs bypass cursor advance and
overflow check
--
commit 9bc41c7859cdd4759479e883a9c557ea64a1a2c7
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>
mm/khugepaged: release contended mmap read lock after scanning enough VMAs
This commit periodically checks whether mmap_lock is contended in
collapse_scan_mm_slot() after scanning a batch of VMAs, dropping and
re-acquiring the read lock and resetting the VMA iterator to
mm_slot->address. It also checks for unsigned long overflow when aligning
vma->vm_end for ineligible VMAs.
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 216c4b8d28c40..eaa01dae5a030 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 to walk before checking for, and relieving, mmap lock
> + * contention, as a function of the pages_to_scan budget.
> + */
> +#define KHUGEPAGED_MAX_VMA_SCAN_BATCH(pages_to_scan) ((pages_to_scan) / 4)
> +
> /**
> * 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 nr_vmas_batch = 0;
>
> lockdep_assert_held(&khugepaged_mm_lock);
> *result = SCAN_FAIL;
> @@ -2942,15 +2949,35 @@ 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(progress_max)) {
> + 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;
> + }
> + }
[Severity: High]
Can collapse_scan_mm_slot() fail to make forward progress here when
mmap_lock is contended?
In pages_to_scan_store(), khugepaged_pages_to_scan can be set to any positive
value:
mm/khugepaged.c:pages_to_scan_store() {
err = kstrtouint(buf, 10, &pages);
if (err || !pages)
return -EINVAL;
khugepaged_pages_to_scan = pages;
}
When khugepaged_pages_to_scan (passed as progress_max) is 1, 2, or 3,
KHUGEPAGED_MAX_VMA_SCAN_BATCH(progress_max) evaluates to 0. Because the batch
check runs at the top of the for_each_vma() loop before
collapse_test_exit_or_disable(mm) or any VMA handling, nr_vmas_batch++ >= 0
triggers on the first VMA before any VMA is scanned. If mmap_lock_is_contended()
is true, it resets nr_vmas_batch to 0, drops and re-acquires mmap_read_lock(mm),
rewinds vmi to the unchanged mm_slot->address, and executes continue without
advancing mm_slot->address, incrementing cc->progress, or checking
collapse_test_exit_or_disable(mm).
Even when progress_max >= 4, for an eligible VMA whose vm_end is not aligned
to HPAGE_PMD_SIZE:
mm/khugepaged.c:collapse_scan_mm_slot() {
hstart = ALIGN(vma->vm_start, HPAGE_PMD_SIZE);
hend = ALIGN_DOWN(vma->vm_end, HPAGE_PMD_SIZE);
if (mm_slot->address > hend) {
cc->progress++;
continue;
}
if (mm_slot->address < hstart)
mm_slot->address = hstart;
...
while (mm_slot->address < hend) {
...
mm_slot->address += HPAGE_PMD_SIZE;
...
}
}
Once the inner loop finishes (or if hstart == hend), mm_slot->address is left
at hend, which is less than vma->vm_end and still lies inside that VMA. If the
batch contention check triggers at the start of the next VMA (for example when
progress_max is 4..7 so the batch threshold is 1), vma_iter_set(&vmi,
mm_slot->address) rewinds the iterator back to the previous VMA where
mm_slot->address == hend, which then does no work and neither advances
mm_slot->address nor increments cc->progress.
> +
> cond_resched();
> if (unlikely(collapse_test_exit_or_disable(mm))) {
> cc->progress++;
> break;
> }
> 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);
> + const unsigned long next =
> + ALIGN(vma->vm_end, HPAGE_PMD_SIZE);
> +
> cc->progress++;
> + /* Check for overflow. */
> + if (unlikely(next < vma->vm_end)) {
> + vma = NULL;
> + break;
> + }
> + /* Unable to collapse, skip to the next candidate. */
> + mm_slot->address = next;
> continue;
> }
> hstart = ALIGN(vma->vm_start, HPAGE_PMD_SIZE);
[Severity: Medium]
As reported by Sashiko on v1 [1], what happens if an anonymous VMA does not
span a full PMD (so hstart >= hend), but sub-PMD anonymous mTHP orders are
enabled?
In collapse_possible_orders(), anonymous VMAs are checked against
THP_ORDERS_ALL_ANON:
mm/khugepaged.c:collapse_possible_orders() {
if ((tva_flags == TVA_KHUGEPAGED) && vma_is_anonymous(vma))
orders = THP_ORDERS_ALL_ANON;
else
orders = BIT(HPAGE_PMD_ORDER);
return thp_vma_allowable_orders(vma, vm_flags, tva_flags, orders);
}
A faulted-in anonymous VMA that is smaller than HPAGE_PMD_SIZE (or does not
span a full PMD) will return non-zero from collapse_possible_orders() when a
sub-PMD order is enabled, skipping the !collapse_possible_orders() block:
1. If the VMA starts at or straddles a PMD boundary (so hstart == hend), it
sets mm_slot->address = hstart (< vma->vm_end), skips the inner while
(mm_slot->address < hend) loop without incrementing cc->progress, and
leaves mm_slot->address inside the VMA, so vma_iter_set(&vmi,
mm_slot->address) will rewind back to this VMA on contention.
2. On architectures where user mappings can reach the top incomplete PMD range
above ULONG_MAX - HPAGE_PMD_SIZE + 1 (such as 64-bit s390 where TASK_SIZE
is -PAGE_SIZE), a sub-PMD anonymous VMA with vm_start > ULONG_MAX -
HPAGE_PMD_SIZE + 1 bypasses the next < vma->vm_end overflow check above.
Then hstart = ALIGN(vma->vm_start, HPAGE_PMD_SIZE) wraps to 0 while hend =
ALIGN_DOWN(vma->vm_end, HPAGE_PMD_SIZE) is ULONG_MAX - HPAGE_PMD_SIZE + 1,
causing the while (mm_slot->address < hend) loop to run from address 0 with
the wrong VMA and rewind mm_slot->address.
Should the hstart >= hend case also advance mm_slot->address to
ALIGN(vma->vm_end, HPAGE_PMD_SIZE) with the overflow check before computing
hstart?
[1] https://lore.kernel.org/r/20261103-khugepaged-latency-fix-v1-5-a07ab16ac860@kernel.org
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-khugepaged-latency-fix-v2-0-4384bbfe4606@kernel.org?part=3
^ permalink raw reply [flat|nested] 24+ messages in thread