Re: (resend)WARNING: trying to isolate tail page in isolate_lru_page

["Yin, Fengwei" <fengwei.yin@intel.com>]

On 8/27/2022 12:56 AM, Minchan Kim wrote:
> On Fri, Aug 26, 2022 at 11:20:58AM +0800, Yin, Fengwei wrote:
>>
>>
>> On 8/26/2022 2:46 AM, Matthew Wilcox wrote:
>>>>> Looks like my analysis from yesterday was dropped:
>>>>>
>>>>> : This all seems quite plausible.  The reproducer seems to (correct me
>>>>> : if I'm wrong) create an AF_PACKET socket and mmap it.  af_packet.c
>>>>> : seems to create compound pages and mmap them.  This isn't folio-related
>>>>> : at all; I just moved the code that warns about it from mm/vmscan.c to
>>>>> : folio-compat.c.
>>>>> :
>>>>> : Looks like a long-standing bug in MADV_PAGEOUT to me.
>>>> Such page should never be on lru, right? We could test lru before
>>>> calling isolate_lru_page() for this case? I know isolate_lru_page()
>>>> does the check, but the tail page warning is raised before the check.
>>>>
>>>> Could the tail page warning be moved under the lru flag test? Seems
>>>> possible, but it should need extra handling (re-set lru flag). Seems a
>>>> little bit overkilling.
>>> There's a number of ways of solving this.  I'm interested in seeing
>>> which one Minchan thinks is best.
>>>
>>
>> My understanding is:
>> PageTransCompound() return false for compound page if THP is disabled
>> in kernel config. Replacing PageTransCompound() with PageCompound() 
>> could work here. But for the long term, folio should be the answer. :).
> 
> Thanks for reporting and analysis, folks,
> 
> I agree with Yang since the MADV_PAGEOUT should work with only
> LRU pages.
Yes. Yang's suggestion has wider coverage.

I am still wondering whether we need change the PageTransCompound()
to PageCompound(). large folios depend on THP now:

commit 421f1ab48452af48b64e205de1caca3d1ba415f4
Author: Matthew Wilcox (Oracle) <willy@infradead.org>
Date:   Sat Jan 15 23:27:08 2022 -0500

    mm: Make large folios depend on THP

    Some parts of the VM still depend on THP to handle large folios
    correctly.  Until those are fixed, prevent creating large folios
    if THP are disabled.


Another thing: maybe move the !LRU(page) check before PageTransCompound()
check? Avoid trying to split the page if it's none-lru page? Just one 
thought. Thanks.


Regards
Yin, Fengwei

> 
> From 0a43ac31c903bc23299a868a6d6724ff5b807e3d Mon Sep 17 00:00:00 2001
> From: Minchan Kim <minchan@kernel.org>
> Date: Fri, 26 Aug 2022 09:37:34 -0700
> Subject: [PATCH] mm: fix madivse_pageout mishandling on non-LRU page
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> MADV_PAGEOUT tries to isolate non-LRU pages and get the warning
> from isolate_lru_page below.
> Fix it with checking PageLRU in advance.
> 
> ------------[ cut here ]------------
> trying to isolate tail page
> WARNING: CPU: 0 PID: 6175 at mm/folio-compat.c:158 isolate_lru_page+0x130/0x140
> Modules linked in:
> CPU: 0 PID: 6175 Comm: syz-executor.0 Not tainted 5.18.12 #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> RIP: 0010:isolate_lru_page+0x130/0x140
> 
> Link: https://lore.kernel.org/linux-mm/485f8c33.2471b.182d5726afb.Coremail.hantianshuo@iie.ac.cn/
> Reported-by: 韩天硕 <hantianshuo@iie.ac.cn>
> Suggested-by: Yang Shi <shy828301@gmail.com>
> Fixes: 1a4e58cce84e ("mm: introduce MADV_PAGEOUT")
> Cc: stable@vger.kernel.org
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>  mm/madvise.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 682e1d161aef..a3fc4cd32ed3 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -452,8 +452,11 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
>  			continue;
>  		}
>  
> -		/* Do not interfere with other mappings of this page */
> -		if (page_mapcount(page) != 1)
> +		/*
> +		 * Do not interfere with other mappings of this page and
> +		 * non-LRU page.
> +		 */
> +		if (!PageLRU(page) || page_mapcount(page) != 1)
>  			continue;
>  
>  		VM_BUG_ON_PAGE(PageTransCompound(page), page);