Received-SPF: pass (google.com: domain of linux-kernel+bounces-163010-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: SeongJae Park <sj@kernel.org>
To: Lance Yang <ioworker0@gmail.com>
Cc: SeongJae Park <sj@kernel.org>,
	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
Subject: Re: [PATCH v3 2/3] mm/rmap: integrate PMD-mapped folio splitting into pagewalk loop
Date: Mon, 29 Apr 2024 13:20:40 -0700
Message-Id: <20240429202040.187453-1-sj@kernel.org>
In-Reply-To: <20240429132308.38794-3-ioworker0@gmail.com>
References: 
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

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 = 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.

      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


Thanks,
SJ
[...]