Re: [PATCH 1/1] mm/madvise: enhance lazyfreeing with mTHP in madvise_free

[Barry Song]

On Tue, Feb 27, 2024 at 2:51 PM Yin Fengwei <fengwei.yin@xxxxxxxxx> wrote:
>
>
>
> On 2/27/24 04:49, Barry Song wrote:
> > On Tue, Feb 27, 2024 at 2:04 AM Ryan Roberts <ryan.roberts@xxxxxxx> wrote:
> >>
> >> On 26/02/2024 08:55, Lance Yang wrote:
> >>> Hey David,
> >>>
> >>> Thanks for your suggestion!
> >>>
> >>> On Mon, Feb 26, 2024 at 4:41 PM David Hildenbrand <david@xxxxxxxxxx> wrote:
> >>>>
> >>> [...]
> >>>>> On Mon, Feb 26, 2024 at 12:00 PM Barry Song <21cnbao@xxxxxxxxx> wrote:
> >>>>> [...]
> >>>>>> On Mon, Feb 26, 2024 at 1:33 AM Lance Yang <ioworker0@xxxxxxxxx> wrote:
> >>>>> [...]
> >>> [...]
> >>>>> +static inline bool pte_range_cont_mapped(pte_t *pte, unsigned long nr)
> >>>>> +{
> >>>>> +     pte_t pte_val;
> >>>>> +     unsigned long pfn = pte_pfn(pte);
> >>>>> +     for (int i = 0; i < nr; i++) {
> >>>>> +             pte_val = ptep_get(pte + i);
> >>>>> +             if (pte_none(pte_val) || pte_pfn(pte_val) != (pfn + i))
> >>>>> +                     return false;
> >>>>> +     }
> >>>>> +     return true;
> >>>>> +}
> >>>>
> >>>> I dislike the "cont mapped" terminology.
> >>>>
> >>>> Maybe folio_pte_batch() does what you want?
> >>>
> >>> folio_pte_batch() is a good choice. Appreciate it!
> >>
> >> Agreed, folio_pte_batch() is likely to be widely useful for this change and
> >> others, so suggest exporting it from memory.c and reusing as is if possible.
> >
> > I actually missed folio_pte_batch() in cont-pte series and re-invented
> > a function
> > to check if a large folio is entirely mapped in MADV_PAGEOUT[1]. exporting
> > folio_pte_batch() will also benefit that case. The problem space is same.
> >
> > [1] https://lore.kernel.org/linux-mm/20240118111036.72641-7-21cnbao@xxxxxxxxx/
> I am wondering whether we can delay large folio split till page reclaim phase
> for madvise cases.
>
> Like if we hit folio which is partially mapped to the range, don't split it but
> just unmap the mapping part from the range. Let page reclaim decide whether
> split the large folio or not (If it's not mapped to any other range,it will be
> freed as whole large folio. If part of it still mapped to other range,page reclaim
> can decide whether to split it or ignore it for current reclaim cycle).

Yes, we can. but we still have to play the ptes check game to avoid adding
folios multiple times to reclaim the list.

I don't see too much difference between splitting in madvise and splitting
in vmscan.  as our real purpose is avoiding splitting entirely mapped
large folios. for partial mapped large folios, if we split in madvise, then
we don't need to play the game of skipping folios while iterating PTEs.
if we don't split in madvise, we have to make sure the large folio is only
added in reclaimed list one time by checking if PTEs belong to the
previous added folio.

>
> Splitting does work here. But it just drops all the benefits of large folio.
>
>
> Regards
> Yin, Fengwei
>
> >
> >>
> >>>
> >>> Best,
> >>> Lance
> >>>
> >>>>
> >>>> --
> >>>> Cheers,
> >>>>
> >>>> David / dhildenb
> >
Thanks
Barry