Re: [RFC PATCH] mm: huge_memory: add folio_mark_accessed() when zapping file THP

[Zi Yan]

On 8 Apr 2025, at 12:02, Johannes Weiner wrote:

> On Tue, Apr 08, 2025 at 11:29:43AM -0400, Zi Yan wrote:
>> On 8 Apr 2025, at 9:16, Baolin Wang wrote:
>>
>>> When investigating performance issues during file folio unmap, I noticed some
>>> behavioral differences in handling non-PMD-sized folios and PMD-sized folios.
>>> For non-PMD-sized file folios, it will call folio_mark_accessed() to mark the
>>> folio as having seen activity, but this is not done for PMD-sized folios.
>>>
>>> This might not cause obvious issues, but a potential problem could be that,
>>> it might lead to more frequent refaults of PMD-sized file folios under memory
>>> pressure. Therefore, I am unsure whether the folio_mark_accessed() should be
>
> Acked-by: Johannes Weiner <hannes@xxxxxxxxxxx>
>
>> How likely will the system get PMD-sized file folios when it is under
>> memory pressure? Johannes’ recent patch increases THP allocation successful
>> rate, maybe it was not happening before but will be after the patch?
>
> It's not so much about whether the refault can construct a THP again,
> but whether we should have evicted this data under pressure to begin
> with. It's more about IO and paging. And it's the same consideration
> why we transfer the young bit for base pages.

Got it. It clarifies things a lot.

>
> Sometimes file contents are only accessed through relatively
> short-lived mappings. But they can nevertheless be accessed a lot and
> be hot. It's important to not lose that information on unmap, and end
> up kicking out a frequently used cache page.

So folio_mark_accessed() will prevent the folio from going down in
the LRU lists, when PTE access information is transferred to the folio.
The addition of folio_mark_accessed() makes sense to me now.

Baolin, can you include Johannes’s explanation in your commit log?

Feel free to add Acked-by: Zi Yan <ziy@xxxxxxxxxx>

>
>>> added for PMD-sized file folios?
>>
>> Do you see any performance change after your patch?
>>
>>>
>>> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
>>> ---
>>>  mm/huge_memory.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index 6ac6d468af0d..b3ade7ac5bbf 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -2262,6 +2262,10 @@ int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>>>  				zap_deposited_table(tlb->mm, pmd);
>>>  			add_mm_counter(tlb->mm, mm_counter_file(folio),
>>>  				       -HPAGE_PMD_NR);
>>> +
>>> +			if (flush_needed && pmd_young(orig_pmd) &&
>>> +			    likely(vma_has_recency(vma)))
>>> +				folio_mark_accessed(folio);
>>>  		}
>>>
>>>  		spin_unlock(ptl);
>>> -- 
>>> 2.43.5


Best Regards,
Yan, Zi