Re: [v1 2/2] mm/damon: introduce DAMOS filter type hugepage

[SeongJae Park <sj@kernel.org>]

On Fri, 17 Jan 2025 11:02:05 +0000 Usama Arif <usamaarif642@gmail.com> wrote:

> 
> 
> On 16/01/2025 19:05, SeongJae Park wrote:
> > On Thu, 16 Jan 2025 14:44:36 +0000 Usama Arif <usamaarif642@gmail.com> wrote:
> > 
> >> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> >> index c0ccf4fade24..a9e69179d45c 100644
> >> --- a/mm/damon/paddr.c
> >> +++ b/mm/damon/paddr.c
> >> @@ -222,6 +222,9 @@ static bool damos_pa_filter_match(struct damos_filter *filter,
> >>  		if (matched)
> >>  			damon_folio_mkold(folio);
> >>  		break;
> >> +	case DAMOS_FILTER_TYPE_HUGEPAGE:
> >> +		matched = folio_size(folio) == HPAGE_PMD_SIZE;
> >> +		break;
> > 
> > This will make build fails when CONFIG_PGTABLE_HAS_HUGE_LEAVES is not set, like
> > below:
> > 
> >  /include/linux/compiler_types.h:542:45: error: call to ‘__compiletime_assert_321’ declared with attribute error: BUILD_BUG failed
> >    542 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >        |                                             ^
> > [...]
> >  /include/linux/huge_mm.h:111:35: note: in expansion of macro ‘HPAGE_PMD_SHIFT’
> >    111 | #define HPAGE_PMD_SIZE  ((1UL) << HPAGE_PMD_SHIFT)
> >        |                                   ^~~~~~~~~~~~~~~
> >  /mm/damon/paddr.c:321:48: note: in expansion of macro ‘HPAGE_PMD_SIZE’
> >    321 |                 matched = folio_size(folio) == HPAGE_PMD_SIZE;
> >        |                                                ^~~~~~~~~~~~~~
> > 
> > 
> > This simple test might help you reproducing it:
> > https://github.com/damonitor/damon-tests/blob/master/corr/tests/build_i386.sh
> > 
> > What about enclosing the entire case with
> > 
> >     #if defined(CONFIG_TRANSPARENT_HUGEPAGE) || defined(CONFIG_HUGETLB_PAGE) ?
> > 
> 
> Ah Thanks for pointing this out! Would 
> 
> #if defined(CONFIG_PGTABLE_HAS_HUGE_LEAVES)
> 
> be better?

Yes, that also looks good to me :)


Thanks,
SJ