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

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 2025/4/9 00:12, Zi Yan wrote:
> 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@cmpxchg.org>

Thanks for taking a look.

>>> 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.

Yes, that's what I also understand. Thanks for the explanation.

> 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?

Sure. Will do.

> 
> Feel free to add Acked-by: Zi Yan <ziy@nvidia.com>

Thanks for reviewing.

>>>> added for PMD-sized file folios?
>>>
>>> Do you see any performance change after your patch?

Not yet, just some theoretical analysis from code inspection.

>>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>>>> ---
>>>>   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