On Tue, 30 Mar 2021 09:51:46 -0700 Yang Shi <shy828301@xxxxxxxxx> wrote: > On Tue, Mar 30, 2021 at 7:42 AM Gerald Schaefer > <gerald.schaefer@xxxxxxxxxxxxx> wrote: > > > > On Mon, 29 Mar 2021 11:33:06 -0700 > > Yang Shi <shy828301@xxxxxxxxx> 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. > > > > > 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.