linux-kernel - Re: [RFC PATCH] madvise: make madvise_cold_or_pageout_pte

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8547495c-9051-faab-a47d-1962f2e0b1da@intel.com>
Date:   Fri, 14 Jul 2023 11:09:45 +0800
From:   "Yin, Fengwei" <fengwei.yin@...el.com>
To:     Yu Zhao <yuzhao@...gle.com>, Minchan Kim <minchan@...nel.org>
CC:     <linux-mm@...ck.org>, <linux-kernel@...r.kernel.org>,
        <akpm@...ux-foundation.org>, <willy@...radead.org>,
        <david@...hat.com>, <ryan.roberts@....com>, <shy828301@...il.com>
Subject: Re: [RFC PATCH] madvise: make madvise_cold_or_pageout_pte_range()
 support large folio



On 7/14/2023 10:08 AM, Yu Zhao wrote:
> On Thu, Jul 13, 2023 at 9:06 AM Yin Fengwei <fengwei.yin@...el.com> wrote:
>>
>> Current madvise_cold_or_pageout_pte_range() has two problems for
>> large folio support:
>>   - Using folio_mapcount() with large folio prevent large folio from
>>     picking up.
>>   - If large folio is in the range requested, shouldn't split it
>>     in madvise_cold_or_pageout_pte_range().
>>
>> Fix them by:
>>   - Use folio_estimated_sharers() with large folio
>>   - If large folio is in the range requested, don't split it. Leave
>>     to page reclaim phase.
>>
>> For large folio cross boundaries of requested range, skip it if it's
>> page cache. Try to split it if it's anonymous folio. If splitting
>> fails, skip it.
> 
> For now, we may not want to change the existing semantic (heuristic).
> IOW, we may want to stick to the "only owner" condition:
> 
>   - if (folio_mapcount(folio) != 1)
>   + if (folio_entire_mapcount(folio) ||
>   +     (any_page_within_range_has_mapcount > 1))
> 
> +Minchan Kim
The folio_estimated_sharers() was discussed here:
https://lore.kernel.org/linux-mm/20230118232219.27038-6-vishal.moola@gmail.com/
https://lore.kernel.org/linux-mm/20230124012210.13963-2-vishal.moola@gmail.com/

Yes. It's accurate to check each page of large folio. But it may be over killed in
some cases (And I think madvise is one of the cases not necessary to be accurate.
So folio_estimated_sharers() is enough. Correct me if I am wrong).

> 
> Also there is an existing bug here: the later commit 07e8c82b5eff8
> ("madvise: convert madvise_cold_or_pageout_pte_range() to use folios")
> is incorrect for sure; the original commit 9c276cc65a58f ("mm:
> introduce MADV_COLD") seems incorrect too.
> 
> +Vishal Moola (Oracle)
> 
> The "any_page_within_range_has_mapcount" test above seems to be the
> only correct to meet condition claimed by the comments, before or
> after the folio conversion, assuming here a THP page means the
> compound page without PMD mappings (PMD-split). Otherwise the test is
> always false (if it's also PMD mapped somewhere else).
> 
>   /*
>    * Creating a THP page is expensive so split it only if we
>    * are sure it's worth. Split it if we are only owner.
>    */
> 
>> The main reason to call folio_referenced() is to clear the yong of
>> conresponding PTEs. So in page reclaim phase, there is good chance
>> the folio can be reclaimed.
>>
>> Signed-off-by: Yin Fengwei <fengwei.yin@...el.com>
>> ---
>> This patch is based on mlock large folio support rfc2 as it depends
>> on the folio_in_range() added by that patchset
>>
>> Also folio_op_size() can be unitfied with get_folio_mlock_step().
>>
>> Testing done:
>>   - kselftest: No new regression introduced.
>>
>>  mm/madvise.c | 133 ++++++++++++++++++++++++++++++++-------------------
>>  1 file changed, 84 insertions(+), 49 deletions(-)
> 
> Also the refactor looks fine to me but it'd be better if it's a separate patch.
OK. I will split the bug fix and refactor to two different patches. Thanks.


Regards
Yin, Fengwei