On Tue, 30 Mar 2021 09:51:46 -0700
Yang Shi [off-list ref] wrote:

On Tue, Mar 30, 2021 at 7:42 AM Gerald Schaefer
[off-list ref] wrote:

quoted

On Mon, 29 Mar 2021 11:33:06 -0700
Yang Shi [off-list ref] wrote:
 

quoted

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.

quoted

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.

quoted

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.