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

[Usama Arif <usamaarif642@gmail.com>]

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?