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

[Zi Yan]

On 11 Apr 2025, at 7:51, Barry Song wrote:

> On Fri, Apr 11, 2025 at 4:42 PM David Hildenbrand <david@xxxxxxxxxx> wrote:
>>
>> On 11.04.25 03:20, Baolin Wang wrote:
>>>
>>>
>>> On 2025/4/11 05:56, Barry Song wrote:
>>>> On Fri, Apr 11, 2025 at 3:13 AM Zi Yan <ziy@xxxxxxxxxx> wrote:
>>>>>
>>>>> On 10 Apr 2025, at 6:29, Barry Song wrote:
>>>>>
>>>>>> On Thu, Apr 10, 2025 at 9:05 PM Baolin Wang
>>>>>> <baolin.wang@xxxxxxxxxxxxxxxxx> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 2025/4/10 16:14, Barry Song wrote:
>>>>>>>> On Wed, Apr 9, 2025 at 1:16 AM Baolin Wang
>>>>>>>> <baolin.wang@xxxxxxxxxxxxxxxxx> 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
>>>>>>>>> added for PMD-sized file folios?
>>>>>>>>>
>>>>>>>>> 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);
>>>>>>>>
>>>>>>>> Acked-by: Barry Song <baohua@xxxxxxxxxx>
>>>>>>>
>>>>>>> Thanks.
>>>>>>>
>>>>>>>> I also came across an interesting observation: on a memory-limited system,
>>>>>>>> demoting unmapped file folios in the LRU—specifically when their mapcount
>>>>>>>> drops from 1 to 0—can actually improve performance.
>>>>>>>
>>>>>>> These file folios are used only once? Can folio_set_dropbehind() be used
>>>>>>> to optimize it, which can avoid the LRU activity movement in
>>>>>>> folio_mark_accessed()?
>>>>>>
>>>>>> For instance, when a process, such as a game, just exits, it can be expected
>>>>>> that it won't be used again in the near future. As a result, demoting
>>>>>> its previously
>>>>>> unmapped file pages can improve performance.
>>>>>
>>>>> Is it possible to mark the dying VMAs either VM_SEQ_READ or VM_RAND_READ
>>>>> so that folio_mark_accessed() will be skipped? Or a new vm_flag?
>>>>> Will it work?
>>>>
>>>> Actually took a more aggressive approach and observed good performance
>>>> improvements on phones. After zap_pte_range() called remove_rmap(),
>>>> the following logic was added:
>>>>
>>>> if (file_folio && !folio_mapped())
>>>>       deactivate_file_folio();
>>>>
>>>> This helps file folios from exiting processes get reclaimed more quickly
>>>> during the MGLRU's min generation scan while the folios are probably
>>>> in max gen.
>>>>
>>>> I'm not entirely sure if this is universally applicable or worth submitting as
>>>> a patch.
>>>
>>> IMHO, I'm afraid this is not universally applicable. Although these file
>>> folios have been unmapped, it's not certain that they won't be accessed
>>> again. These file folios might be remapped and accessed again soon, or
>>> accessed through read()/write() operations using a file descriptor.
>>>
>>> I agree with Zi's suggestion. Using some kind of madvise() hint to mark
>>> these file folios as those that won't be accessed after being unmapped,
>>> seems can work?
>>
>> Is that similar to MADV_COLD before unmap?
>
> I'm not convinced that's the case. Although the previous app exits,
> its exclusive
> folios aren't useful to the newly launched app. For instance, Firefox's text and
> other exclusive file-backed folios have no relevance to LibreOffice. If a user
> terminates Firefox and then launches LibreOffice, marking Firefox’s young
> PTE-mapped folios as accessed—thus activating them in the LRU—is
> meaningless for LibreOffice.

In terms of marking VMAs, can you do it in exit_mmap() by passing a new
parameter, like bool dying_vma, to unmap_vmas()? So that unmap_vmas()
can change exclusive file-backed VMAs to !vma_has_recency() to avoid
folio_mark_accessed().


Best Regards,
Yan, Zi