On Wed, Jun 23, 2021 at 04:39:56PM -0700, Nick Desaulniers wrote:

An additional report:
https://lore.kernel.org/lkml/20210623223015.GA315292@paulmck-ThinkPad-P17-Gen-1/ (local)
EOM

On Fri, Jun 18, 2021 at 4:05 PM Nathan Chancellor [off-list ref] wrote:

quoted

On Fri, Jun 18, 2021 at 10:32:42AM +0530, Aneesh Kumar K.V wrote:

quoted

On 6/17/21 11:32 PM, Nathan Chancellor wrote:

quoted

Rebuilt the CC list because most people were added based on the
incorrect bisect result.

On Thu, Jun 17, 2021 at 02:51:49PM +0100, Matthew Wilcox wrote:

quoted

On Thu, Jun 17, 2021 at 06:15:45PM +0530, Naresh Kamboju wrote:

quoted

On Thu, 17 Jun 2021 at 17:41, Naresh Kamboju [off-list ref] wrote:

quoted

x86_64-linux-gnu-ld: mm/mremap.o: in function `move_pgt_entry':
mremap.c:(.text+0x763): undefined reference to `__compiletime_assert_342'

The git bisect pointed out the first bad commit.

The first bad commit:
commit 928cf6adc7d60c96eca760c05c1000cda061604e
Author: Stephen Boyd [off-list ref]
Date:   Thu Jun 17 15:21:35 2021 +1000
     module: add printk formats to add module build ID to stacktraces

Your git bisect probably went astray.  There's no way that commit
caused that regression.

My bisect landed on commit 83f85ac75855 ("mm/mremap: convert huge PUD
move to separate helper"). flush_pud_tlb_range() evaluates to
BUILD_BUG() when CONFIG_TRANSPARENT_HUGEPAGE is unset but this function
is present just based on the value of
CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD.

$ make -skj(nproc) ARCH=x86_64 CC=clang O=build/x86_64 distclean allnoconfig mm/mremap.o

$ llvm-readelf -s build/x86_64/mm/mremap.o &| rg __compiletime_assert
     21: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT   UND __compiletime_assert_337

$ rg TRANSPARENT_ build/x86_64/.config
450:CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE=y
451:CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD=y
562:# CONFIG_TRANSPARENT_HUGEPAGE is not set

Not sure why this does not happen on newer clang versions, presumably
something with inlining decisions? Still seems like a legitimate issue
to me.

gcc 10 also doesn't give a build error. I guess that is because we evaluate

     if (pud_trans_huge(*old_pud) || pud_devmap(*old_pud)) {

 to if (0) with CONFIG_TRANSPARENT_HUGEPAGE disabled.

switching that to if (1) do results in BUILD_BUG triggering.

Thanks for pointing that out. I think what happens with clang-10 and
clang-11 is that move_huge_pud() gets inlined into move_pgt_entry() but
then the compiler does not figure out that the HPAGE_PUD case is dead so
the code sticks around, where as GCC and newer clang versions can figure
that out and eliminate that case.

quoted

Should we fix this ?

Yes, I believe that we should.

quoted

modified   mm/mremap.c

@@ -336,7 +336,7 @@ static inline bool move_normal_pud(struct vm_area_struct

*vma,
 }
 #endif

-#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
+#if defined(CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD) &&
defined(CONFIG_TRANSPARENT_HUGEPAGE)
 static bool move_huge_pud(struct vm_area_struct *vma, unsigned long
old_addr,
                        unsigned long new_addr, pud_t *old_pud, pud_t *new_pud)
 {

Making the above change does the trick for my repeat-by, thank you!

quoted

That works or we could mirror what has already been done for the
HPAGE_PMD case. No personal preference.

diff --git a/mm/mremap.c b/mm/mremap.c
index 9a7fbec31dc9..5989d3990020 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c

@@ -460,7 +460,8 @@ static bool move_pgt_entry(enum pgt_entry entry, struct vm_area_struct *vma,
                                      new_entry);
                break;
        case HPAGE_PUD:
-               moved = move_huge_pud(vma, old_addr, new_addr, old_entry,
+               moved = IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
+                       move_huge_pud(vma, old_addr, new_addr, old_entry,
                                      new_entry);
                break;

This one is already in -next, but you knew that already.  I am happy to
test the resulting patch, when and if.

						Thanx, Paul