mm/khugepaged.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)
mthp_collapse() tries to swap in PTEs when collapsing a mTHP if there are any
swap PTEs in the PMD range, even if none of those swap PTEs are
actually part of the mTHP's range.
Track swap PTEs in cc->mthp_unmapped_ptes so that mthp_collapse() can
tell whether the mTHP's own range contains a swap PTE, and skip the
swapin check when it doesn't.
Signed-off-by: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
---
mm/khugepaged.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 11ff98d55c76..38596172f7dc 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -117,6 +117,9 @@ struct collapse_control {
/* Each bit represents a single occupied (!none/zero) page. */
DECLARE_BITMAP(mthp_present_ptes, MAX_PTRS_PER_PTE);
+
+ /* Each bit represents a single not present and not none/zero pte. */
+ DECLARE_BITMAP(mthp_unmapped_ptes, MAX_PTRS_PER_PTE);
};
/**
@@ -634,6 +637,7 @@ static void collapse_control_init_scan(struct collapse_control *cc)
memset(cc->node_load, 0, sizeof(cc->node_load));
nodes_clear(cc->alloc_nmask);
bitmap_zero(cc->mthp_present_ptes, MAX_PTRS_PER_PTE);
+ bitmap_zero(cc->mthp_unmapped_ptes, MAX_PTRS_PER_PTE);
}
static void release_pte_folio(struct folio *folio)
@@ -1291,7 +1295,7 @@ static enum scan_result alloc_charge_folio(struct folio **foliop, struct mm_stru
* Note that the VMA must be rechecked after grabbing the mmap_lock again.
*/
static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long start_addr,
- int referenced, int unmapped, struct collapse_control *cc,
+ int referenced, bool swapin, struct collapse_control *cc,
unsigned int order)
{
const unsigned long pmd_addr = start_addr & HPAGE_PMD_MASK;
@@ -1330,7 +1334,7 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long s
goto out_nolock;
}
- if (unmapped) {
+ if (swapin) {
/*
* __collapse_huge_page_swapin() will return with mmap_lock
* released when it fails. So we jump out_nolock directly in
@@ -1502,10 +1506,10 @@ static unsigned int max_order_from_offset(unsigned int offset)
* mTHP.
*/
static enum scan_result mthp_collapse(struct mm_struct *mm,
- unsigned long address, int referenced, int unmapped,
+ unsigned long address, int referenced,
struct collapse_control *cc, unsigned long enabled_orders)
{
- unsigned int nr_occupied_ptes, nr_ptes, max_ptes_none;
+ unsigned int nr_occupied_ptes, nr_unmapped_ptes, nr_ptes, max_ptes_none;
enum scan_result last_result = SCAN_FAIL;
int collapsed = 0;
bool alloc_failed = false;
@@ -1522,21 +1526,25 @@ static enum scan_result mthp_collapse(struct mm_struct *mm,
max_ptes_none = collapse_max_ptes_none(cc, NULL, order);
nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
offset + nr_ptes);
+ nr_unmapped_ptes = bitmap_weight_from(cc->mthp_unmapped_ptes, offset,
+ offset + nr_ptes);
+
/*
- * Swap PTEs accepted during the scan are counted in @unmapped,
- * not in the present-PTE bitmap. Account them for the PMD-order
- * candidate.
+ * Swap PTEs accepted during the scan are counted in
+ * nr_unmapped_ptes, not in the present-PTE bitmap. Account
+ * them for the PMD-order candidate.
*/
if (is_pmd_order(order))
- nr_occupied_ptes += unmapped;
+ nr_occupied_ptes += nr_unmapped_ptes;
if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
enum scan_result ret;
+ bool swapin = nr_unmapped_ptes > 0;
collapse_address = address + offset * PAGE_SIZE;
ret = collapse_huge_page(mm, collapse_address, referenced,
- unmapped, cc, order);
+ swapin, cc, order);
switch (ret) {
/* Cases where we continue to next collapse candidate */
@@ -1667,6 +1675,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
continue;
}
if (!pte_present(pteval)) {
+ __set_bit(i, cc->mthp_unmapped_ptes);
if (++unmapped > max_ptes_swap) {
result = SCAN_EXCEED_SWAP_PTE;
count_collapse_event(HPAGE_PMD_ORDER, THP_SCAN_EXCEED_SWAP_PTE,
@@ -1785,7 +1794,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
/* collapse_huge_page() expects the lock to be dropped before calling */
mmap_read_unlock(mm);
result = mthp_collapse(mm, start_addr, referenced,
- unmapped, cc, enabled_orders);
+ cc, enabled_orders);
/* mmap_lock was released above, set lock_dropped */
*lock_dropped = true;
}
--
2.55.0
On 8/26/26 3:24 AM, Pedro Demarchi Gomes wrote:
> mthp_collapse() tries to swap in PTEs when collapsing a mTHP if there are any
> swap PTEs in the PMD range, even if none of those swap PTEs are
> actually part of the mTHP's range.
Are you sure? I wonder how you tested your patch? Because we never
swapin PTEs for mTHP collapse, see the code in
__collapse_huge_page_swapin():
/*
* TODO: Support swapin without leading to further mTHP
* collapses. Currently bringing in new pages via swapin may
* cause a future higher order collapse on a rescan of the same
* range.
*/
if (!is_pmd_order(order)) {
count_mthp_stat(order, MTHP_STAT_COLLAPSE_EXCEED_SWAP);
pte_unmap(pte);
mmap_read_unlock(mm);
result = SCAN_EXCEED_SWAP_PTE;
goto out;
}
>
> Track swap PTEs in cc->mthp_unmapped_ptes so that mthp_collapse() can
> tell whether the mTHP's own range contains a swap PTE, and skip the
> swapin check when it doesn't.
>
> Signed-off-by: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
> ---
> mm/khugepaged.c | 29 +++++++++++++++++++----------
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 11ff98d55c76..38596172f7dc 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -117,6 +117,9 @@ struct collapse_control {
>
> /* Each bit represents a single occupied (!none/zero) page. */
> DECLARE_BITMAP(mthp_present_ptes, MAX_PTRS_PER_PTE);
> +
> + /* Each bit represents a single not present and not none/zero pte. */
> + DECLARE_BITMAP(mthp_unmapped_ptes, MAX_PTRS_PER_PTE);
> };
>
> /**
> @@ -634,6 +637,7 @@ static void collapse_control_init_scan(struct collapse_control *cc)
> memset(cc->node_load, 0, sizeof(cc->node_load));
> nodes_clear(cc->alloc_nmask);
> bitmap_zero(cc->mthp_present_ptes, MAX_PTRS_PER_PTE);
> + bitmap_zero(cc->mthp_unmapped_ptes, MAX_PTRS_PER_PTE);
> }
>
> static void release_pte_folio(struct folio *folio)
> @@ -1291,7 +1295,7 @@ static enum scan_result alloc_charge_folio(struct folio **foliop, struct mm_stru
> * Note that the VMA must be rechecked after grabbing the mmap_lock again.
> */
> static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long start_addr,
> - int referenced, int unmapped, struct collapse_control *cc,
> + int referenced, bool swapin, struct collapse_control *cc,
> unsigned int order)
> {
> const unsigned long pmd_addr = start_addr & HPAGE_PMD_MASK;
> @@ -1330,7 +1334,7 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long s
> goto out_nolock;
> }
>
> - if (unmapped) {
> + if (swapin) {
> /*
> * __collapse_huge_page_swapin() will return with mmap_lock
> * released when it fails. So we jump out_nolock directly in
> @@ -1502,10 +1506,10 @@ static unsigned int max_order_from_offset(unsigned int offset)
> * mTHP.
> */
> static enum scan_result mthp_collapse(struct mm_struct *mm,
> - unsigned long address, int referenced, int unmapped,
> + unsigned long address, int referenced,
> struct collapse_control *cc, unsigned long enabled_orders)
> {
> - unsigned int nr_occupied_ptes, nr_ptes, max_ptes_none;
> + unsigned int nr_occupied_ptes, nr_unmapped_ptes, nr_ptes, max_ptes_none;
> enum scan_result last_result = SCAN_FAIL;
> int collapsed = 0;
> bool alloc_failed = false;
> @@ -1522,21 +1526,25 @@ static enum scan_result mthp_collapse(struct mm_struct *mm,
> max_ptes_none = collapse_max_ptes_none(cc, NULL, order);
> nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
> offset + nr_ptes);
> + nr_unmapped_ptes = bitmap_weight_from(cc->mthp_unmapped_ptes, offset,
> + offset + nr_ptes);
> +
>
> /*
> - * Swap PTEs accepted during the scan are counted in @unmapped,
> - * not in the present-PTE bitmap. Account them for the PMD-order
> - * candidate.
> + * Swap PTEs accepted during the scan are counted in
> + * nr_unmapped_ptes, not in the present-PTE bitmap. Account
> + * them for the PMD-order candidate.
> */
> if (is_pmd_order(order))
> - nr_occupied_ptes += unmapped;
> + nr_occupied_ptes += nr_unmapped_ptes;
>
> if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> enum scan_result ret;
> + bool swapin = nr_unmapped_ptes > 0;
>
> collapse_address = address + offset * PAGE_SIZE;
> ret = collapse_huge_page(mm, collapse_address, referenced,
> - unmapped, cc, order);
> + swapin, cc, order);
>
> switch (ret) {
> /* Cases where we continue to next collapse candidate */
> @@ -1667,6 +1675,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> continue;
> }
> if (!pte_present(pteval)) {
> + __set_bit(i, cc->mthp_unmapped_ptes);
> if (++unmapped > max_ptes_swap) {
> result = SCAN_EXCEED_SWAP_PTE;
> count_collapse_event(HPAGE_PMD_ORDER, THP_SCAN_EXCEED_SWAP_PTE,
> @@ -1785,7 +1794,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> /* collapse_huge_page() expects the lock to be dropped before calling */
> mmap_read_unlock(mm);
> result = mthp_collapse(mm, start_addr, referenced,
> - unmapped, cc, enabled_orders);
> + cc, enabled_orders);
> /* mmap_lock was released above, set lock_dropped */
> *lock_dropped = true;
> }
On 8/27/26 10:06, Baolin Wang wrote: > > > On 8/26/26 3:24 AM, Pedro Demarchi Gomes wrote: >> mthp_collapse() tries to swap in PTEs when collapsing a mTHP if there are any >> swap PTEs in the PMD range, even if none of those swap PTEs are >> actually part of the mTHP's range. > > Are you sure? I wonder how you tested your patch? Because we never swapin PTEs > for mTHP collapse, see the code in __collapse_huge_page_swapin(): I'm confused as well, this doesn't really make sense. -- Cheers, David
On Thu, Aug 27, 2026 at 01:02:14PM +0200, David Hildenbrand (Arm) wrote:
>On 8/27/26 10:06, Baolin Wang wrote:
>>
>>
>> On 8/26/26 3:24 AM, Pedro Demarchi Gomes wrote:
>>> mthp_collapse() tries to swap in PTEs when collapsing a mTHP if there are any
>>> swap PTEs in the PMD range, even if none of those swap PTEs are
>>> actually part of the mTHP's range.
>>
>> Are you sure? I wonder how you tested your patch? Because we never swapin PTEs
>> for mTHP collapse, see the code in __collapse_huge_page_swapin():
>
>I'm confused as well, this doesn't really make sense.
Well ... the change does remove a redundant PTE walk, IIUC ...
I think Pedro's wording is causing the confusion :)
Yeah, Baolin is right that __collapse_huge_page_swapin() never reaches
do_swap_page() for a mTHP. For an otherwise eligible lower-order
candidate, current code still calls it and walks the candidate's PTE range
when an unrelated swap PTE exists elsewhere in the same PMD :)
Assume an otherwise eligible lower-order candidate has no swap PTE, while
another subrange in the PMD has one. collapse_scan_pmd() counts unmapped
over the full PMD and passes that PMD-wide value into mthp_collapse():
static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
struct vm_area_struct *vma, unsigned long start_addr,
bool *lock_dropped, struct collapse_control *cc)
{
...
int node = NUMA_NO_NODE, unmapped = 0;
...
for (i = 0; i < HPAGE_PMD_NR; i++) {
_pte = pte + i;
addr = start_addr + i * PAGE_SIZE;
pteval = ptep_get(_pte);
...
if (pte_none_or_zero(pteval)) {
...
continue;
}
if (!pte_present(pteval)) {
if (++unmapped > max_ptes_swap) {
...
}
...
if (pte_swp_uffd_any(pteval)) {
result = SCAN_PTE_UFFD;
goto out_unmap;
}
continue;
}
...
}
if (cc->is_khugepaged &&
(!referenced ||
(unmapped && referenced < HPAGE_PMD_NR / 2))) {
result = SCAN_LACK_REFERENCED_PAGE;
} else {
result = SCAN_SUCCEED;
}
...
if (result == SCAN_SUCCEED) {
...
result = mthp_collapse(mm, start_addr, referenced,
unmapped, cc, enabled_orders);
...
}
...
return result;
}
unmapped is PMD-wide above. mthp_collapse() then passes the same value to
every attempted candidate:
static enum scan_result mthp_collapse(struct mm_struct *mm,
unsigned long address, int referenced, int unmapped,
struct collapse_control *cc, unsigned long enabled_orders)
{
unsigned int nr_occupied_ptes, nr_ptes, max_ptes_none;
...
unsigned int order = HPAGE_PMD_ORDER;
while (offset < HPAGE_PMD_NR) {
nr_ptes = 1UL << order;
if (!test_bit(order, &enabled_orders))
goto next_order;
max_ptes_none = collapse_max_ptes_none(cc, NULL, order);
nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
offset + nr_ptes);
/*
* Swap PTEs accepted during the scan are counted in @unmapped,
* not in the present-PTE bitmap. Account them for the PMD-order
* candidate.
*/
if (is_pmd_order(order))
nr_occupied_ptes += unmapped;
if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
enum scan_result ret;
collapse_address = address + offset * PAGE_SIZE;
ret = collapse_huge_page(mm, collapse_address, referenced,
unmapped, cc, order);
...
}
next_order:
...
if (order > KHUGEPAGED_MIN_MTHP_ORDER &&
(enabled_orders & GENMASK(order - 1, 0))) {
order--;
continue;
}
next_offset:
...
offset += nr_ptes;
order = max_order_from_offset(offset);
}
...
}
Once it reaches the lower-order candidate with no swap PTE, unmapped is
still nonzero and collapse_huge_page() calls the swapin helper:
static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long start_addr,
int referenced, int unmapped, struct collapse_control *cc,
unsigned int order)
{
...
if (unmapped) {
...
result = __collapse_huge_page_swapin(mm, vma, start_addr, pmd,
referenced, order);
...
}
...
}
__collapse_huge_page_swapin() only calls do_swap_page() after finding a
non-present, non-none PTE, and lower orders return before that call:
static enum scan_result __collapse_huge_page_swapin(struct mm_struct *mm,
struct vm_area_struct *vma, unsigned long start_addr,
pmd_t *pmd, int referenced, unsigned int order)
{
...
unsigned long addr, end = start_addr + (PAGE_SIZE << order);
enum scan_result result;
pte_t *pte = NULL;
spinlock_t *ptl;
for (addr = start_addr; addr < end; addr += PAGE_SIZE) {
...
vmf.orig_pte = ptep_get_lockless(pte);
if (pte_none(vmf.orig_pte) ||
pte_present(vmf.orig_pte))
continue;
...
if (!is_pmd_order(order)) {
...
result = SCAN_EXCEED_SWAP_PTE;
goto out;
}
vmf.pte = pte;
vmf.ptl = ptl;
ret = do_swap_page(&vmf);
...
}
...
result = SCAN_SUCCEED;
out:
...
return result;
}
For the candidate above, the loop only reads its PTEs and returns
SCAN_SUCCEED. Pedro's bitmap lets mthp_collapse() determine that upfront
and skip the walk.
Emm ... as I asked before[1], any numbers showing how much the extra scan
costs?
[1] https://lore.kernel.org/lkml/20260825192433.3185880-1-pedrodemargomes@gmail.com/
Cheers, Lance
On Thu, Aug 27, 2026 at 08:11:51PM +0800, Lance Yang wrote:
>
> On Thu, Aug 27, 2026 at 01:02:14PM +0200, David Hildenbrand (Arm) wrote:
> >On 8/27/26 10:06, Baolin Wang wrote:
> >>
> >>
> >> On 8/26/26 3:24 AM, Pedro Demarchi Gomes wrote:
> >>> mthp_collapse() tries to swap in PTEs when collapsing a mTHP if there are any
> >>> swap PTEs in the PMD range, even if none of those swap PTEs are
> >>> actually part of the mTHP's range.
> >>
> >> Are you sure? I wonder how you tested your patch? Because we never swapin PTEs
> >> for mTHP collapse, see the code in __collapse_huge_page_swapin():
> >
> >I'm confused as well, this doesn't really make sense.
>
> Well ... the change does remove a redundant PTE walk, IIUC ...
>
> I think Pedro's wording is causing the confusion :)
Sorry for the confusion. My patch description was not clear enough.
Thanks for taking the time to clarify the intent of the patch and explain the
code path in more detail.
>
> Yeah, Baolin is right that __collapse_huge_page_swapin() never reaches
> do_swap_page() for a mTHP. For an otherwise eligible lower-order
> candidate, current code still calls it and walks the candidate's PTE range
> when an unrelated swap PTE exists elsewhere in the same PMD :)
>
> Assume an otherwise eligible lower-order candidate has no swap PTE, while
> another subrange in the PMD has one. collapse_scan_pmd() counts unmapped
> over the full PMD and passes that PMD-wide value into mthp_collapse():
>
> static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> struct vm_area_struct *vma, unsigned long start_addr,
> bool *lock_dropped, struct collapse_control *cc)
> {
> ...
> int node = NUMA_NO_NODE, unmapped = 0;
> ...
> for (i = 0; i < HPAGE_PMD_NR; i++) {
> _pte = pte + i;
> addr = start_addr + i * PAGE_SIZE;
> pteval = ptep_get(_pte);
> ...
> if (pte_none_or_zero(pteval)) {
> ...
> continue;
> }
> if (!pte_present(pteval)) {
> if (++unmapped > max_ptes_swap) {
> ...
> }
> ...
> if (pte_swp_uffd_any(pteval)) {
> result = SCAN_PTE_UFFD;
> goto out_unmap;
> }
> continue;
> }
> ...
> }
> if (cc->is_khugepaged &&
> (!referenced ||
> (unmapped && referenced < HPAGE_PMD_NR / 2))) {
> result = SCAN_LACK_REFERENCED_PAGE;
> } else {
> result = SCAN_SUCCEED;
> }
> ...
> if (result == SCAN_SUCCEED) {
> ...
> result = mthp_collapse(mm, start_addr, referenced,
> unmapped, cc, enabled_orders);
> ...
> }
> ...
> return result;
> }
>
> unmapped is PMD-wide above. mthp_collapse() then passes the same value to
> every attempted candidate:
>
> static enum scan_result mthp_collapse(struct mm_struct *mm,
> unsigned long address, int referenced, int unmapped,
> struct collapse_control *cc, unsigned long enabled_orders)
> {
> unsigned int nr_occupied_ptes, nr_ptes, max_ptes_none;
> ...
> unsigned int order = HPAGE_PMD_ORDER;
>
> while (offset < HPAGE_PMD_NR) {
> nr_ptes = 1UL << order;
>
> if (!test_bit(order, &enabled_orders))
> goto next_order;
>
> max_ptes_none = collapse_max_ptes_none(cc, NULL, order);
> nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
> offset + nr_ptes);
>
> /*
> * Swap PTEs accepted during the scan are counted in @unmapped,
> * not in the present-PTE bitmap. Account them for the PMD-order
> * candidate.
> */
> if (is_pmd_order(order))
> nr_occupied_ptes += unmapped;
>
> if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> enum scan_result ret;
>
> collapse_address = address + offset * PAGE_SIZE;
> ret = collapse_huge_page(mm, collapse_address, referenced,
> unmapped, cc, order);
> ...
> }
>
> next_order:
> ...
> if (order > KHUGEPAGED_MIN_MTHP_ORDER &&
> (enabled_orders & GENMASK(order - 1, 0))) {
> order--;
> continue;
> }
> next_offset:
> ...
> offset += nr_ptes;
> order = max_order_from_offset(offset);
> }
> ...
> }
>
> Once it reaches the lower-order candidate with no swap PTE, unmapped is
> still nonzero and collapse_huge_page() calls the swapin helper:
>
> static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long start_addr,
> int referenced, int unmapped, struct collapse_control *cc,
> unsigned int order)
> {
> ...
> if (unmapped) {
> ...
> result = __collapse_huge_page_swapin(mm, vma, start_addr, pmd,
> referenced, order);
> ...
> }
> ...
> }
>
> __collapse_huge_page_swapin() only calls do_swap_page() after finding a
> non-present, non-none PTE, and lower orders return before that call:
>
> static enum scan_result __collapse_huge_page_swapin(struct mm_struct *mm,
> struct vm_area_struct *vma, unsigned long start_addr,
> pmd_t *pmd, int referenced, unsigned int order)
> {
> ...
> unsigned long addr, end = start_addr + (PAGE_SIZE << order);
> enum scan_result result;
> pte_t *pte = NULL;
> spinlock_t *ptl;
>
> for (addr = start_addr; addr < end; addr += PAGE_SIZE) {
> ...
> vmf.orig_pte = ptep_get_lockless(pte);
> if (pte_none(vmf.orig_pte) ||
> pte_present(vmf.orig_pte))
> continue;
> ...
> if (!is_pmd_order(order)) {
> ...
> result = SCAN_EXCEED_SWAP_PTE;
> goto out;
> }
>
> vmf.pte = pte;
> vmf.ptl = ptl;
> ret = do_swap_page(&vmf);
> ...
> }
> ...
> result = SCAN_SUCCEED;
> out:
> ...
> return result;
> }
>
> For the candidate above, the loop only reads its PTEs and returns
> SCAN_SUCCEED. Pedro's bitmap lets mthp_collapse() determine that upfront
> and skip the walk.
>
> Emm ... as I asked before[1], any numbers showing how much the extra scan
> costs?
I tested this on my machine with a 2 MB PMD size. In my measurements, removing
the extra PTE walk did not show a significant reduction in the overall scan
time. Maybe for machines with a bigger PMD size this can make a difference.
>
> [1] https://lore.kernel.org/lkml/20260825192433.3185880-1-pedrodemargomes@gmail.com/
>
> Cheers, Lance
>
On Thu, Aug 27, 2026 at 08:11:51PM +0800, Lance Yang wrote: > For the candidate above, the loop only reads its PTEs and returns > SCAN_SUCCEED. Pedro's bitmap lets mthp_collapse() determine that upfront > and skip the walk. > > Emm ... as I asked before[1], any numbers showing how much the extra scan > costs? With the win being only some scan cost, I don't think it is going to be measurable. It can open a path to future work. For example, we could find fully populated mTHP windows in a PMD that is not eligible for collapse because unmapped > max_ptes_swap. Right now we just give up on the whole PMD in such cases. But there are more wins to gain there. Just saying :P -- Kiryl Shutsemau / Kirill A. Shutemov
On Thu, Aug 27, 2026 at 03:07:10PM +0100, Kiryl Shutsemau wrote: > On Thu, Aug 27, 2026 at 08:11:51PM +0800, Lance Yang wrote: > > For the candidate above, the loop only reads its PTEs and returns > > SCAN_SUCCEED. Pedro's bitmap lets mthp_collapse() determine that upfront > > and skip the walk. > > > > Emm ... as I asked before[1], any numbers showing how much the extra scan > > costs? > > With the win being only some scan cost, I don't think it is going to be > measurable. > > It can open a path to future work. For example, we could find fully > populated mTHP windows in a PMD that is not eligible for collapse > because unmapped > max_ptes_swap. Right now we just give up on the whole > PMD in such cases. > > But there are more wins to gain there. Just saying :P Hi Kiryl, I devised a patch based on your proposed future work idea. If you think it makes sense, I can send it to the mailing list with a Suggested-by: tag. Thanks!
On Mon, Aug 31, 2026 at 03:53:34PM -0300, Pedro Demarchi Gomes wrote: > On Thu, Aug 27, 2026 at 03:07:10PM +0100, Kiryl Shutsemau wrote: > > On Thu, Aug 27, 2026 at 08:11:51PM +0800, Lance Yang wrote: > > > For the candidate above, the loop only reads its PTEs and returns > > > SCAN_SUCCEED. Pedro's bitmap lets mthp_collapse() determine that upfront > > > and skip the walk. > > > > > > Emm ... as I asked before[1], any numbers showing how much the extra scan > > > costs? > > > > With the win being only some scan cost, I don't think it is going to be > > measurable. > > > > It can open a path to future work. For example, we could find fully > > populated mTHP windows in a PMD that is not eligible for collapse > > because unmapped > max_ptes_swap. Right now we just give up on the whole > > PMD in such cases. > > > > But there are more wins to gain there. Just saying :P > > Hi Kiryl, > > I devised a patch based on your proposed future work idea. If you think it > makes sense, I can send it to the mailing list with a Suggested-by: tag. I am not sure it is worth as the moment. There's lower handing fruits in khugepaged that has to be addressed. -- Kiryl Shutsemau / Kirill A. Shutemov
On Tue, Aug 25, 2026 at 04:24:33PM -0300, Pedro Demarchi Gomes wrote:
>mthp_collapse() tries to swap in PTEs when collapsing a mTHP if there are any
>swap PTEs in the PMD range, even if none of those swap PTEs are
>actually part of the mTHP's range.
>
>Track swap PTEs in cc->mthp_unmapped_ptes so that mthp_collapse() can
>tell whether the mTHP's own range contains a swap PTE, and skip the
>swapin check when it doesn't.
>
>Signed-off-by: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
>---
Yeah, the code itself is fine, but ...
Any numbers showing how much the extra scan costs? khugepaged is already
quite a mess and badly needs some cleanup. TBH, another bitmap + more
bookkeeping doesn't feel worth it unless the win is pretty clear :)
> mm/khugepaged.c | 29 +++++++++++++++++++----------
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
>diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>index 11ff98d55c76..38596172f7dc 100644
>--- a/mm/khugepaged.c
>+++ b/mm/khugepaged.c
>@@ -117,6 +117,9 @@ struct collapse_control {
>
> /* Each bit represents a single occupied (!none/zero) page. */
> DECLARE_BITMAP(mthp_present_ptes, MAX_PTRS_PER_PTE);
>+
>+ /* Each bit represents a single not present and not none/zero pte. */
>+ DECLARE_BITMAP(mthp_unmapped_ptes, MAX_PTRS_PER_PTE);
BTW, the comment is a bit of a mouthful :)
How about "Each bit represents a non-present, non-none PTE." ?
Cheers, Lance
[...]
© 2016 - 2026 Red Hat, Inc.