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

[Yang Shi <shy828301@gmail.com>]

On Fri, Aug 26, 2022 at 9:56 AM Minchan Kim <minchan@kernel.org> 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.
>
> 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>

Thanks for the patch, looks good to me. Will you post it to the
mailing list? Anyway you could have my ack.

> ---
>  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);
> --
> 2.37.2.672.g94769d06f0-goog
>