linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Wei Yang <richard.weiyang@gmail.com>
To: Lance Yang <lance.yang@linux.dev>
Cc: Shivank Garg <shivankg@amd.com>, Zi Yan <ziy@nvidia.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Baolin Wang <baolin.wang@linux.alibaba.com>,
	"Liam R . Howlett" <Liam.Howlett@oracle.com>,
	Nico Pache <npache@redhat.com>,
	Ryan Roberts <ryan.roberts@arm.com>, Dev Jain <dev.jain@arm.com>,
	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	David Hildenbrand <david@kernel.org>,
	Barry Song <baohua@kernel.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Wei Yang <richard.weiyang@gmail.com>
Subject: Re: [PATCH V2 2/5] mm/khugepaged: count small VMAs towards scan limit
Date: Wed, 24 Dec 2025 14:49:54 +0000	[thread overview]
Message-ID: <20251224144954.2bhei4bk27f462nb@master> (raw)
In-Reply-To: <6b408736-978a-4d40-adfc-97819951c3a6@linux.dev>

On Wed, Dec 24, 2025 at 07:51:36PM +0800, Lance Yang wrote:
>
>
>On 2025/12/24 19:13, Shivank Garg wrote:
>> The khugepaged_scan_mm_slot() uses a 'progress' counter to limit the
>> amount of work performed and consists of three components:
>> 1. Transitioning to a new mm (+1).

Hmm... maybe not only a new mm, but also we start another scan from last mm.

Since default khugepaged_pages_to_scan is 8 PMD, it looks very possible.

>> 2. Skipping an unsuitable VMA (+1).
>> 3. Scanning a PMD-sized range (+HPAGE_PMD_NR).
>> 
>> Consider a 1MB VMA sitting between two 2MB alignment boundaries:
>> 
>>       vma1       vma2   vma3
>>      +----------+------+----------+
>>      |2M        |1M    |2M        |
>>      +----------+------+----------+
>>                 ^      ^
>>                 start  end
>>                 ^
>>            hstart,hend
>> 
>> In this case, for vma2:
>>    hstart = round_up(start, HPAGE_PMD_SIZE)  -> Next 2MB alignment
>>    hend   = round_down(end, HPAGE_PMD_SIZE) -> Prev 2MB alignment
>> 
>> Currently, since `hend <= hstart`, VMAs that are too small or unaligned
>> to contain a hugepage are skipped without incrementing 'progress'.
>> A process containing a large number of such small VMAs will unfairly
>> consume more CPU cycles before yielding compared to a process with
>> fewer, larger, or aligned VMAs.
>> 
>> Fix this by incrementing progress when the `hend <= hstart` condition
>> is met.
>> 
>> Additionally, change 'progress' type to `unsigned int` to match both
>> the 'pages' type and the function return value.
>> 
>> Suggested-by: Wei Yang <richard.weiyang@gmail.com>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>>   mm/khugepaged.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 107146f012b1..0b549c3250f9 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -2403,7 +2403,7 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>>   	struct mm_slot *slot;
>>   	struct mm_struct *mm;
>>   	struct vm_area_struct *vma;
>> -	int progress = 0;
>> +	unsigned int progress = 0;
>>   	VM_BUG_ON(!pages);
>>   	lockdep_assert_held(&khugepaged_mm_lock);
>> @@ -2447,7 +2447,7 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>>   		}
>>   		hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE);
>>   		hend = round_down(vma->vm_end, HPAGE_PMD_SIZE);
>> -		if (khugepaged_scan.address > hend) {
>
>Maybe add a short comment explaining why we increment progress for small VMAs
>;)
>
>Something like this:
>
>		/* Count small VMAs that can't hold a hugepage towards scan limit */
>> +		if (khugepaged_scan.address > hend || hend <= hstart) {
>>   			progress++;
>>   			continue;
>>   		}
>
>Otherwise, looks good to me.
>
>Reviewed-by: Lance Yang <lance.yang@linux.dev>
>

The code change LGTM.

Reviewed-by: Wei Yang <richard.weiyang@gmail.com>

-- 
Wei Yang
Help you, Help me


  reply	other threads:[~2025-12-24 14:50 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-24 11:13 [PATCH V2 0/5] mm/khugepaged: cleanups and scan limit fix Shivank Garg
2025-12-24 11:13 ` [PATCH V2 1/5] mm/khugepaged: remove unnecessary goto 'skip' label Shivank Garg
2025-12-24 11:34   ` Lance Yang
2025-12-24 11:13 ` [PATCH V2 2/5] mm/khugepaged: count small VMAs towards scan limit Shivank Garg
2025-12-24 11:51   ` Lance Yang
2025-12-24 14:49     ` Wei Yang [this message]
2025-12-28 17:58       ` Garg, Shivank
2025-12-24 11:13 ` [PATCH V2 3/5] mm/khugepaged: change collapse_pte_mapped_thp() to return void Shivank Garg
2025-12-24 12:21   ` Lance Yang
2025-12-29 16:40   ` Zi Yan
2025-12-24 11:13 ` [PATCH V2 4/5] mm/khugepaged: use enum scan_result for result variables and return types Shivank Garg
2025-12-29 16:41   ` Zi Yan
2025-12-24 11:13 ` [PATCH V2 5/5] mm/khugepaged: make khugepaged_collapse_control static Shivank Garg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251224144954.2bhei4bk27f462nb@master \
    --to=richard.weiyang@gmail.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=lance.yang@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=npache@redhat.com \
    --cc=ryan.roberts@arm.com \
    --cc=shivankg@amd.com \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox