Re: [RFC PATCH] madvise: make madvise_cold_or_pageout_pte_range() support large folio

[David Hildenbrand]

On 14.07.23 15:58, Yin, Fengwei wrote:
> On 7/14/2023 5:25 PM, David Hildenbrand wrote:
> > (1) We're unmapping a single subpage, the compound_mapcount == 0
> >      and the total_mapcount > 0. If the subpage mapcount is now 0, add it
> >      to the deferred split queue.
> >
> > (2) We're unmapping a complete folio (PMD mapping / compound), the
> >      compound_mapcount is 0 and the total_mapcount > 0.
> >
> >   (a) If total mapcount < folio_nr_pages, add it
> >       to the deferred split queue.
> >
> >   (b) If total mapcount >= folio_nr_pages , we have to scan all subpage
> >       mapcounts. If any subpage mapcount == 0, add it to the deferred
> >       split queue.
> >
> >
> > (b) is a bit nasty. It would happen when we fork() with a PMD-mapped THP, the parent splits the THP due to COW, and then our child unmaps or splits the PMD-mapped THP (unmap easily happening during exec()). Fortunately, we'd only scan once when unmapping the PMD.
> >
> >
> > Getting rid of the subpage mapcount usage in (1) would mean that we have to do exactly what we do in (2). But then we'd need to ha handle (2) (B) differently as well.
> >
> > So, for 2 (b) we would either need some other heuristic, or we add it to the deferred split queue more frequently and let that one detect using an rmap walk "well, every subpage is still mapped, let's abort the split".
>
> Or another option for 2 (b): don't add it to deferred split queue. We
> know the folio in deferred list is mainly scanned when system needs to
> reclaim memory.
>
> Maybe it's better to let page reclaim choose the large folio here because
> page reclaiming will call folio_referenced() which does rmap_walk()->pvmw().
> And we can reuse rmap_walk() in folio_referenced() to detect whether there
> are pages of folio are not mapped. If it's the case, we can split it then.
>
> Comparing to deferred list, the advantage is that folio_referenced() doesn't
> unmap page. While in deferred list, we need to add extra rmap_walk() to
> check whether there is page not mapped.

Right, I also came to the conclusion that the unmapping is undesirable. 
However, once benefit of the unmap is that we know when to stop scanning 
(due to page_mapped()). But maybe the temporary unmapping is actually 
counter-productive.

> Just a thought. I could miss something here. Thanks.

Interesting idea.

I also had the thought that adding folios to the deferred split queue 
when removing the rmap is semantically questionable.

Yes, we remove the rmap when zapping/unmapping a pte/pmd. But we also 
(eventually only temporarily!) unmap when splitting a THP or when 
installing migration entries.

Maybe we can flag folios when zapping PTEs that they are a reasonable 
candidate to tell page reclaim code "this one was partially zapped, 
maybe there is some memory to reclaim there, now". Maybe that involves 
the deferred split queue.

--
Cheers,

David / dhildenb