Re: [PATCH v4 4/6] mm: Introduce a pageflag for partially mapped folios

[Bang Li]

hi, Usama

On 2024/8/22 3:04, Usama Arif wrote:

> On 20/08/2024 17:30, Barry Song wrote:
>
> > Hi Usama,
> > thanks! I can't judge if we need this partially_mapped flag. but if we
> > need, the code
> > looks correct to me. I'd like to leave this to David and other experts to ack.
> Thanks for the reviews!
>
> > an alternative approach might be two lists? one for entirely_mapped,
> > the other one
> > for split_deferred. also seems ugly ?
> That was my very first prototype! I shifted to using a bool which I sent in v1, and then a bit in _flags_1 as David suggested. I believe a bit in _flags_1 is the best way forward, as it leaves the most space in folio for future work.
>
> > On the other hand, when we want to extend your patchset to mTHP other than PMD-
> > order, will the only deferred_list create huge lock contention while
> > adding or removing
> > folios from it?
> Yes, I would imagine so. the deferred_split_queue is per memcg/node, so that helps.
>
> Also, this work is tied to khugepaged. So would need some thought when doing it for mTHP.
>
> I would imagine doing underused shrinker for mTHP would be less beneficial compared to doing it for 2M THP. But probably needs experimentation.
>
> Thanks

Below is the core code snippet to support "split underused mTHP". Can we extend the
khugepaged_max_ptes_none value to mthp and keep its semantics unchanged? With a small
modification, Only folios with page numbers greater than khugepaged_max_ptes_none - 1
can be added to the deferred_split list and can be split. What do you think?

diff --git a/mm/memory.c b/mm/memory.c
index b95fce7d190f..ef503958d6a0 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4789,6 +4789,8 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
        }

        folio_ref_add(folio, nr_pages - 1);
+       if (nr_pages > 1 && nr_pages > khugepaged_max_ptes_none - 1)
+               deferred_split_folio(folio, false);
        add_mm_counter(vma->vm_mm, MM_ANONPAGES, nr_pages);
        count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_FAULT_ALLOC);
        folio_add_new_anon_rmap(folio, vma, addr, RMAP_EXCLUSIVE);

shmem THP has the same memory expansion problem when the shmem_enabled configuration is
set to always. In my opinion, it is necessary to support "split underused shmem THP",
but I am not sure if there is any gap in the implementation?

Bang
Thanks