Received-SPF: pass (google.com: domain of linux-kernel+bounces-163254-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240429132308.38794-3-ioworker0@gmail.com> <20240429202040.187453-1-sj@kernel.org>
In-Reply-To: <20240429202040.187453-1-sj@kernel.org>
From: Lance Yang <ioworker0@gmail.com>
Date: Tue, 30 Apr 2024 10:03:31 +0800
Message-ID: <CAK1f24=y3S_=ABUtzet9d2gftnb2j107Y-t+J8KzYR5ttcMgpg@mail.gmail.com>
Subject: Re: [PATCH v3 2/3] mm/rmap: integrate PMD-mapped folio splitting into
 pagewalk loop
To: SeongJae Park <sj@kernel.org>
Cc: akpm@linux-foundation.org, willy@infradead.org, maskray@google.com, 
	ziy@nvidia.com, ryan.roberts@arm.com, david@redhat.com, 21cnbao@gmail.com, 
	mhocko@suse.com, fengwei.yin@intel.com, zokeefe@google.com, 
	shy828301@gmail.com, xiehuan09@gmail.com, libang.li@antgroup.com, 
	wangkefeng.wang@huawei.com, songmuchun@bytedance.com, peterx@redhat.com, 
	minchan@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hey SJ,

Thanks a lot for reporting!

On Tue, Apr 30, 2024 at 4:20=E2=80=AFAM SeongJae Park <sj@kernel.org> wrote=
:
>
> Hi Lance,
>
> On Mon, 29 Apr 2024 21:23:07 +0800 Lance Yang <ioworker0@gmail.com> 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@redhat.com>
> > Signed-off-by: Lance Yang <ioworker0@gmail.com>
> > ---
> >  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 =3D 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 invalidat=
ion.
> >        * 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 =3D 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 =3D address & HPAGE_PMD_MASK;
> > +             range.end =3D (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 '__co=
mpiletime_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 o=
f 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 macr=
o '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 macr=
o '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_M=
ASK'
>      1651 |   range.start =3D 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_arm=
64.sh

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

Thanks again for reporting!
Lance

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