linux-kernel - Re: [PATCH v2 00/10] mm/thp: fix THP splitting unmap BUGs and related

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-next>] [day] [month] [year] [list]
Message-ID: <b27e866-a06c-d32-20aa-3b16f58549@google.com>
Date:   Thu, 10 Jun 2021 17:15:51 -0700 (PDT)
From:   Hugh Dickins <hughd@...gle.com>
To:     Alistair Popple <apopple@...dia.com>
cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Hugh Dickins <hughd@...gle.com>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Yang Shi <shy828301@...il.com>,
        Wang Yugui <wangyugui@...-tech.com>,
        Matthew Wilcox <willy@...radead.org>,
        Naoya Horiguchi <naoya.horiguchi@....com>,
        Ralph Campbell <rcampbell@...dia.com>, Zi Yan <ziy@...dia.com>,
        Miaohe Lin <linmiaohe@...wei.com>,
        Minchan Kim <minchan@...nel.org>, Jue Wang <juew@...gle.com>,
        Peter Xu <peterx@...hat.com>, Jan Kara <jack@...e.cz>,
        Shakeel Butt <shakeelb@...gle.com>,
        Oscar Salvador <osalvador@...e.de>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 00/10] mm/thp: fix THP splitting unmap BUGs and
 related

On Fri, 11 Jun 2021, Alistair Popple wrote:
> On Friday, 11 June 2021 8:15:05 AM AEST Andrew Morton wrote:
> > On Tue, 8 Jun 2021 20:57:34 -0700 (PDT) Hugh Dickins <hughd@...gle.com> wrote:
> > 
> > > These are against 5.13-rc5: expect mmotm conflicts with a couple of
> > > Alistair Popple's "Add support for SVM atomics in Nouveau" series:
> > > mm-remove-special-swap-entry-functions.patch
> > > mm-rmap-split-try_to_munlock-from-try_to_unmap.patch
> > 
> > I came unstuck at "mm/rmap: split migration into its own function".

Sorry about that, I hadn't yet gotten to trying my latest with mmotm.
And I think my previous mmotm-adjust.tar must have been incomplete;
and even if it were complete, would no longer apply properly anyway.

> > 
> > --- mm/huge_memory.c~mm-rmap-split-migration-into-its-own-function
> > +++ mm/huge_memory.c
> > @@ -2345,16 +2345,21 @@ void vma_adjust_trans_huge(struct vm_are
> > 
> >  static void unmap_page(struct page *page)
> >  {
> > -       enum ttu_flags ttu_flags = TTU_IGNORE_MLOCK |
> > -               TTU_RMAP_LOCKED | TTU_SPLIT_HUGE_PMD;
> > +       enum ttu_flags ttu_flags = TTU_RMAP_LOCKED | TTU_SPLIT_HUGE_PMD;
> >         bool unmap_success;
> > 
> >         VM_BUG_ON_PAGE(!PageHead(page), page);
> > 
> >         if (PageAnon(page))
> > -               ttu_flags |= TTU_SPLIT_FREEZE;
> > -
> > -       unmap_success = try_to_unmap(page, ttu_flags);
> > +               unmap_success = try_to_migrate(page, ttu_flags);
> > +       else
> > +               /*
> > +                * Don't install migration entries for file backed pages. This
> > +                * helps handle cases when i_size is in the middle of the page
> > +                * as there is no need to unmap pages beyond i_size manually.
> > +                */
> > +               unmap_success = try_to_unmap(page, ttu_flags |
> > +                                               TTU_IGNORE_MLOCK);
> >         VM_BUG_ON_PAGE(!unmap_success, page);
> >  }
> > 
> > 
> > Sigh.  I have a few todo's against Alastair's "Add support for SVM
> > atomics in Nouveau v9".  Including

Sigh shared!

> > 
> > https://lkml.kernel.org/r/20210525183710.fa2m2sbfixnhz7g5@revolver
> > https://lkml.kernel.org/r/20210604204934.sbspsmwdqdtmz73d@revolver
> > https://lkml.kernel.org/r/YK6mbf967dV0ljHn@t490s
> > https://lkml.kernel.org/r/2005328.bFqPmhE5MS@nvdebian
> > https://lkml.kernel.org/r/202105262107.LkxpsZsV-lkp@intel.com
> > https://lkml.kernel.org/r/YK6hYGEx+XzeZELV@t490s
> > 
> > So I think I'll drop that series and shall ask for it to be redone
> > against this lot, please.

Thank you, Andrew: that's certainly easiest for you and for me:
and I think the right thing to do for now.

> > 
> 
> I believe v10 of the series posted earlier this week should address those
> todo's. I will double check though and resend based on top of mmotm. Thanks.

Sorry to give you the bother, Alistair: it's worked out as a bad moment
to rewrite swapops.h and rmap.c, I'm afraid.

And the only help I've had time to give you was pointing Peter at your
series - many thanks to Peter, and to Shakeel.

Several times I've been on the point of asking you to keep the familiar
migration_entry_to_page(), along with your new pfn_swap_entry_to_page();
but each time I've looked, seen that it's hard to retain it sensibly at
the same time as overdue cleanup of the device_private_entry_to_page()s.

So I guess I'm resigned to losing it; but there are at least three
bugs currently under discussion or fixes in flight, which border on
migration_entry_to_page() - Jann Horn's smaps syzbot bug, Xu Yu's
__migration_entry_wait() fix, my __split_huge_pmd_locked() fix
(and page_vma_mapped_walk() cleanup).

And regarding huge_memory.c's unmap_page(): I did not recognize the
"helps handle cases when i_size" comment you added there.  What I
ended up with (and thought was in mmotm-adjust.tar but seems not):

	/*
	 * Anon pages need migration entries to preserve them, but file
	 * pages can simply be left unmapped, then faulted back on demand.
	 * If that is ever changed (perhaps for mlock), update remap_page().
	 */
	if (PageAnon(page))
		try_to_migrate(page, ttu_flags);
	else
		try_to_unmap(page, ttu_flags | TTU_IGNORE_MLOCK);

with
	/* If try_to_migrate() is used on file, remove this check */
in remap_page() to replace the
	/* If TTU_SPLIT_FREEZE is ever extended to file, remove this check */
comment my series puts there (since you delete TTU_SPLIT_FREEZE altogether).

Hugh