[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f21ee001-91b5-4047-8e45-6e2ef320bcdf@linux.alibaba.com>
Date: Mon, 9 Feb 2026 17:25:33 +0800
From: Baolin Wang <baolin.wang@...ux.alibaba.com>
To: "David Hildenbrand (Arm)" <david@...nel.org>, akpm@...ux-foundation.org,
catalin.marinas@....com, will@...nel.org
Cc: lorenzo.stoakes@...cle.com, ryan.roberts@....com,
Liam.Howlett@...cle.com, vbabka@...e.cz, rppt@...nel.org, surenb@...gle.com,
mhocko@...e.com, riel@...riel.com, harry.yoo@...cle.com, jannh@...gle.com,
willy@...radead.org, baohua@...nel.org, dev.jain@....com,
linux-mm@...ck.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/5] mm: rmap: support batched checks of the references
for large folios
On 2/9/26 5:20 PM, David Hildenbrand (Arm) wrote:
> On 2/9/26 10:14, Baolin Wang wrote:
>>
>>
>> On 2/9/26 4:49 PM, David Hildenbrand (Arm) wrote:
>>> On 12/26/25 07:07, Baolin Wang wrote:
>>>> Currently, folio_referenced_one() always checks the young flag for
>>>> each PTE
>>>> sequentially, which is inefficient for large folios. This
>>>> inefficiency is
>>>> especially noticeable when reclaiming clean file-backed large
>>>> folios, where
>>>> folio_referenced() is observed as a significant performance hotspot.
>>>>
>>>> Moreover, on Arm64 architecture, which supports contiguous PTEs,
>>>> there is already
>>>> an optimization to clear the young flags for PTEs within a
>>>> contiguous range.
>>>> However, this is not sufficient. We can extend this to perform
>>>> batched operations
>>>> for the entire large folio (which might exceed the contiguous range:
>>>> CONT_PTE_SIZE).
>>>>
>>>> Introduce a new API: clear_flush_young_ptes() to facilitate batched
>>>> checking
>>>> of the young flags and flushing TLB entries, thereby improving
>>>> performance
>>>> during large folio reclamation. And it will be overridden by the
>>>> architecture
>>>> that implements a more efficient batch operation in the following
>>>> patches.
>>>>
>>>> While we are at it, rename ptep_clear_flush_young_notify() to
>>>> clear_flush_young_ptes_notify() to indicate that this is a batch
>>>> operation.
>>>>
>>>> Reviewed-by: Ryan Roberts <ryan.roberts@....com>
>>>> Signed-off-by: Baolin Wang <baolin.wang@...ux.alibaba.com>
>>>> ---
>>>> include/linux/mmu_notifier.h | 9 +++++----
>>>> include/linux/pgtable.h | 31 +++++++++++++++++++++++++++++++
>>>> mm/rmap.c | 31 ++++++++++++++++++++++++++++---
>>>> 3 files changed, 64 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/include/linux/mmu_notifier.h b/include/linux/
>>>> mmu_notifier.h
>>>> index d1094c2d5fb6..07a2bbaf86e9 100644
>>>> --- a/include/linux/mmu_notifier.h
>>>> +++ b/include/linux/mmu_notifier.h
>>>> @@ -515,16 +515,17 @@ static inline void mmu_notifier_range_init_owner(
>>>> range->owner = owner;
>>>> }
>>>> -#define ptep_clear_flush_young_notify(__vma, __address,
>>>> __ptep) \
>>>> +#define clear_flush_young_ptes_notify(__vma, __address, __ptep,
>>>> __nr) \
>>>> ({ \
>>>> int __young; \
>>>> struct vm_area_struct *___vma = __vma; \
>>>> unsigned long ___address = __address; \
>>>> - __young = ptep_clear_flush_young(___vma, ___address, __ptep); \
>>>> + unsigned int ___nr = __nr; \
>>>> + __young = clear_flush_young_ptes(___vma, ___address, __ptep,
>>>> ___nr); \
>>>> __young |= mmu_notifier_clear_flush_young(___vma->vm_mm, \
>>>> ___address, \
>>>> ___address + \
>>>> - PAGE_SIZE); \
>>>> + ___nr * PAGE_SIZE); \
>>>> __young; \
>>>> })
>>>
>>> Man that's ugly, Not your fault, but can this possibly be turned into
>>> an inline function in a follow-up patch.
>>
>> Yes, the cleanup of these macros is already in my follow-up patch set.
>>
>>>> +#ifndef clear_flush_young_ptes
>>>> +/**
>>>> + * clear_flush_young_ptes - Clear the access bit and perform a TLB
>>>> flush for PTEs
>>>> + * that map consecutive pages of the same folio.
>>>
>>> With clear_young_dirty_ptes() description in mind, this should
>>> probably be "Mark PTEs that map consecutive pages of the same folio
>>> as clean and flush the TLB" ?
>>
>> IMO, “clean” is confusing here, as it sounds like clear the dirty bit
>> to make the folio clean.
>
> "as old", sorry, I used the wrong part of the description.
OK.
>>>> + * @vma: The virtual memory area the pages are mapped into.
>>>> + * @addr: Address the first page is mapped at.
>>>> + * @ptep: Page table pointer for the first entry.
>>>> + * @nr: Number of entries to clear access bit.
>>>> + *
>>>> + * May be overridden by the architecture; otherwise, implemented as
>>>> a simple
>>>> + * loop over ptep_clear_flush_young().
>>>> + *
>>>> + * Note that PTE bits in the PTE range besides the PFN can differ.
>>>> For example,
>>>> + * some PTEs might be write-protected.
>>>> + *
>>>> + * Context: The caller holds the page table lock. The PTEs map
>>>> consecutive
>>>> + * pages that belong to the same folio. The PTEs are all in the
>>>> same PMD.
>>>> + */
>>>> +static inline int clear_flush_young_ptes(struct vm_area_struct *vma,
>>>> + unsigned long addr, pte_t *ptep,
>>>> + unsigned int nr)
>>>
>>> Two-tab alignment on second+ line like all similar functions here.
>>
>> Sure.
>>
>>>> +{
>>>> + int i, young = 0;
>>>> +
>>>> + for (i = 0; i < nr; ++i, ++ptep, addr += PAGE_SIZE)
>>>> + young |= ptep_clear_flush_young(vma, addr, ptep);
>>>> +
>>>
>>> Why don't we use a similar loop we use in clear_young_dirty_ptes() or
>>> clear_full_ptes() etc? It's not only consistent but also optimizes
>>> out the first check for nr.
>>> for (;;) {
>>> young |= ptep_clear_flush_young(vma, addr, ptep);
>>> if (--nr == 0)
>>> break;
>>> ptep++;
>>> addr += PAGE_SIZE;
>>> }
>>
>> We’ve discussed this loop pattern before [1], and it seems that people
>> prefer the ‘for (;;)’ loop. Do you have a strong preference for
>> changing it back?
>
> Yes, to make all such helpers look consistent. Note that your version
> was also not consistent with the other variants.
>
> Ryans point was about avoiding two ptep_clear_flush_young() calls, which
> the for(;;) avoids as well.
Actually my v2[1] is following the previous pattern, anyway let me
change it back.
[1]
https://lore.kernel.org/all/545dba5e899634bc6c8ca782417d16fef3bd049f.1765439381.git.baolin.wang@linux.alibaba.com/
Powered by blists - more mailing lists