Re: [PATCH v3 2/3] mm/rmap: integrate PMD-mapped folio splitting into pagewalk loop

[Lance Yang]

Hey SJ,

Thanks a lot for reporting!

On Tue, Apr 30, 2024 at 4:20 AM SeongJae Park <sj@xxxxxxxxxx> wrote:
>
> Hi Lance,
>
> On Mon, 29 Apr 2024 21:23:07 +0800 Lance Yang <ioworker0@xxxxxxxxx> wrote:
>
> > In preparation for supporting try_to_unmap_one() to unmap PMD-mapped
> > folios, start the pagewalk first, then call split_huge_pmd_address()
> > to split the folio.
> >
> > Suggested-by: David Hildenbrand <david@xxxxxxxxxx>
> > Signed-off-by: Lance Yang <ioworker0@xxxxxxxxx>
> > ---
> >  include/linux/huge_mm.h |  2 ++
> >  mm/huge_memory.c        | 42 +++++++++++++++++++++--------------------
> >  mm/rmap.c               | 26 +++++++++++++++++++------
> >  3 files changed, 44 insertions(+), 26 deletions(-)
> >
> [...]
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index 7e2575d669a9..e42f436c7ff3 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -1636,9 +1636,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> >       if (flags & TTU_SYNC)
> >               pvmw.flags = PVMW_SYNC;
> >
> > -     if (flags & TTU_SPLIT_HUGE_PMD)
> > -             split_huge_pmd_address(vma, address, false, folio);
> > -
> >       /*
> >        * For THP, we have to assume the worse case ie pmd for invalidation.
> >        * For hugetlb, it could be much worse if we need to do pud
> > @@ -1650,6 +1647,10 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> >       range.end = vma_address_end(&pvmw);
> >       mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma->vm_mm,
> >                               address, range.end);
> > +     if (flags & TTU_SPLIT_HUGE_PMD) {
> > +             range.start = address & HPAGE_PMD_MASK;
> > +             range.end = (address & HPAGE_PMD_MASK) + HPAGE_PMD_SIZE;
> > +     }
>
> I found the latest mm-unstable fails one[1] of my build configuration
> with below error message.  And 'git bisect' points this patch.

Thanks for taking time to 'git bisect' and identify this bug!

>
>       CC      mm/rmap.o
>     In file included from <command-line>:
>     .../linux/mm/rmap.c: In function 'try_to_unmap_one':
>     .../linux/include/linux/compiler_types.h:460:38: error: call to '__compiletime_assert_455' declared with attribute error: BUILD_BUG failed
>       460 |  _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>           |                                      ^
>     .../linux/include/linux/compiler_types.h:441:4: note: in definition of macro '__compiletime_assert'
>       441 |    prefix ## suffix();    \
>           |    ^~~~~~
>     .../linux/include/linux/compiler_types.h:460:2: note: in expansion of macro '_compiletime_assert'
>       460 |  _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>           |  ^~~~~~~~~~~~~~~~~~~
>     .../linux/include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
>        39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
>           |                                     ^~~~~~~~~~~~~~~~~~
>     .../linux/include/linux/build_bug.h:59:21: note: in expansion of macro 'BUILD_BUG_ON_MSG'
>        59 | #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
>           |                     ^~~~~~~~~~~~~~~~
>     .../linux/include/linux/huge_mm.h:97:28: note: in expansion of macro 'BUILD_BUG'
>        97 | #define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
>           |                            ^~~~~~~~~
>     .../linux/include/linux/huge_mm.h:104:34: note: in expansion of macro 'HPAGE_PMD_SHIFT'
>       104 | #define HPAGE_PMD_SIZE ((1UL) << HPAGE_PMD_SHIFT)
>           |                                  ^~~~~~~~~~~~~~~
>     .../linux/include/linux/huge_mm.h:103:27: note: in expansion of macro 'HPAGE_PMD_SIZE'
>       103 | #define HPAGE_PMD_MASK (~(HPAGE_PMD_SIZE - 1))
>           |                           ^~~~~~~~~~~~~~
>     .../linux/mm/rmap.c:1651:27: note: in expansion of macro 'HPAGE_PMD_MASK'
>      1651 |   range.start = address & HPAGE_PMD_MASK;
>           |                           ^~~~~~~~~~~~~~
>
> I haven't looked into the code yet, but seems this code need to handle
> CONFIG_PGTABLE_HAS_HUGE_LEAVES undefined case?  May I ask your opinion?
>
> [1] https://github.com/awslabs/damon-tests/blob/next/corr/tests/build_arm64.sh

I'll fix this bug and rebuild using the config you've provided above.

Thanks again for reporting!
Lance

>
>
> Thanks,
> SJ
> [...]