Message-ID: <CAHbLzkquYxq_eXoVhUCib9qu_aMS9U2XXjb5pop9JtJ8uco_vg@mail.gmail.com>
Date:   Thu, 1 Apr 2021 13:10:49 -0700
From:   Yang Shi <shy828301@...il.com>
To:     Gerald Schaefer <gerald.schaefer@...ux.ibm.com>
Cc:     Mel Gorman <mgorman@...e.de>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Zi Yan <ziy@...dia.com>, Michal Hocko <mhocko@...e.com>,
        Huang Ying <ying.huang@...el.com>,
        Hugh Dickins <hughd@...gle.com>, hca@...ux.ibm.com,
        gor@...ux.ibm.com, borntraeger@...ibm.com,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux MM <linux-mm@...ck.org>, linux-s390@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Alexander Gordeev <agordeev@...ux.ibm.com>
Subject: Re: [RFC PATCH 0/6] mm: thp: use generic THP migration for NUMA
 hinting fault

On Wed, Mar 31, 2021 at 4:47 AM Gerald Schaefer
<gerald.schaefer@...ux.ibm.com> wrote:
>
> On Tue, 30 Mar 2021 09:51:46 -0700
> Yang Shi <shy828301@...il.com> wrote:
>
> > On Tue, Mar 30, 2021 at 7:42 AM Gerald Schaefer
> > <gerald.schaefer@...ux.ibm.com> wrote:
> > >
> > > On Mon, 29 Mar 2021 11:33:06 -0700
> > > Yang Shi <shy828301@...il.com> wrote:
> > >
> > > >
> > > > When the THP NUMA fault support was added THP migration was not supported yet.
> > > > So the ad hoc THP migration was implemented in NUMA fault handling.  Since v4.14
> > > > THP migration has been supported so it doesn't make too much sense to still keep
> > > > another THP migration implementation rather than using the generic migration
> > > > code.  It is definitely a maintenance burden to keep two THP migration
> > > > implementation for different code paths and it is more error prone.  Using the
> > > > generic THP migration implementation allows us remove the duplicate code and
> > > > some hacks needed by the old ad hoc implementation.
> > > >
> > > > A quick grep shows x86_64, PowerPC (book3s), ARM64 ans S390 support both THP
> > > > and NUMA balancing.  The most of them support THP migration except for S390.
> > > > Zi Yan tried to add THP migration support for S390 before but it was not
> > > > accepted due to the design of S390 PMD.  For the discussion, please see:
> > > > https://lkml.org/lkml/2018/4/27/953.
> > > >
> > > > I'm not expert on S390 so not sure if it is feasible to support THP migration
> > > > for S390 or not.  If it is not feasible then the patchset may make THP NUMA
> > > > balancing not be functional on S390.  Not sure if this is a show stopper although
> > > > the patchset does simplify the code a lot.  Anyway it seems worth posting the
> > > > series to the mailing list to get some feedback.
> > >
> > > The reason why THP migration cannot work on s390 is because the migration
> > > code will establish swap ptes in a pmd. The pmd layout is very different from
> > > the pte layout on s390, so you cannot simply write a swap pte into a pmd.
> > > There are no separate swp primitives for swap/migration pmds, IIRC. And even
> > > if there were, we'd still need to find some space for a present bit in the
> > > s390 pmd, and/or possibly move around some other bits.
> > >
> > > A lot of things can go wrong here, even if it could be possible in theory,
> > > by introducing separate swp primitives in common code for pmd entries, along
> > > with separate offset, type, shift, etc. I don't see that happening in the
> > > near future.
> >
> > Thanks a lot for elaboration. IIUC, implementing migration PMD entry
> > is *not* prevented from by hardware, it may be very tricky to
> > implement it, right?
>
> Well, it depends. The HW is preventing proper full-blown swap + migration
> support for PMD, similar to what we have for PTE, because we simply don't
> have enough OS-defined bits in the PMD. A 5-bit swap type for example,
> similar to a PTE, plus the PFN would not be possible.
>
> The HW would not prevent a similar mechanism in principle, i.e. we could
> mark it as invalid to trigger a fault, and have some magic bits that tell
> the fault handler or migration code what it is about.
>
> For handling migration aspects only, w/o any swap device or other support, a
> single type bit could already be enough, to indicate read/write migration,
> plus a "present" bit similar to PTE. But even those 2 bits would be hard to
> find, though I would not entirely rule that out. That would be the tricky
> part.
>
> Then of course, common code would need some changes, to reflect the
> different swap/migration (type) capabilities of PTE and PMD entries.
> Not sure if such an approach would be acceptable for common code.
>
> But this is just some very abstract and optimistic view, I have not
> really properly looked into the details. So it might be even more
> tricky, or not possible at all.

Thanks a lot for the elaboration.

>
> >
> > >
> > > Not sure if this is a show stopper, but I am not familiar enough with
> > > NUMA and migration code to judge. E.g., I do not see any swp entry action
> > > in your patches, but I assume this is implicitly triggered by the switch
> > > to generic THP migration code.
> >
> > Yes, exactly. The migrate_pages() called by migrate_misplaced_page()
> > takes care of everything.
> >
> > >
> > > Could there be a work-around by splitting THP pages instead of marking them
> > > as migrate pmds (via pte swap entries), at least when THP migration is not
> > > supported? I guess it could also be acceptable if THP pages were simply not
> > > migrated for NUMA balancing on s390, but then we might need some extra config
> > > option to make that behavior explicit.
> >
> > Yes, it could be. The old behavior of migration was to return -ENOMEM
> > if THP migration is not supported then split THP. That behavior was
> > not very friendly to some usecases, for example, memory policy and
> > migration lieu of reclaim (the upcoming). But I don't mean we restore
> > the old behavior. We could split THP if it returns -ENOSYS and the
> > page is THP.
>
> OK, as long as we don't get any broken PMD migration entries established
> for s390, some extra THP splitting would be acceptable I guess.

There will be no migration PMD installed. The current behavior is a
no-op if THP migration is not supported.

Powered by blists - more mailing lists