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

[David Hildenbrand <david@redhat.com>]

On 20.01.25 20:16, SeongJae Park wrote:
> On Mon, 20 Jan 2025 19:57:10 +0100 David Hildenbrand <david@redhat.com> wrote:
> 
>> On 20.01.25 19:19, Usama Arif wrote:
>>> This is to gather statistics to check if memory regions of specific
>>> access tempratures are backed by hugepages. This includes both THPs
>>> and hugetlbfs.
>>> This filter can help to observe and prove the effectivenes of
>>> different schemes for shrinking/collapsing hugepages.
>>>
>>> Signed-off-by: Usama Arif <usamaarif642@gmail.com>
>>> ---
>>> v2 -> v3:
>>> - expose hugepage via sysfs even if the kernel is
>>>     built without hugepage support. DAMON will just
>>>     just return 0. (SJ Park)
>>>
>>> v1 -> v2:
>>> - Wrap DAMOS_FILTER_TYPE_HUGEPAGE case with
>>>     CONFIG_PGTABLE_HAS_HUGE_LEAVES (SJ Park)
>>> ---
>>>    include/linux/damon.h    | 2 ++
>>>    mm/damon/paddr.c         | 5 +++++
>>>    mm/damon/sysfs-schemes.c | 1 +
>>>    3 files changed, 8 insertions(+)
>>>
>>> diff --git a/include/linux/damon.h b/include/linux/damon.h
>>> index af525252b853..1d94d7d88b36 100644
>>> --- a/include/linux/damon.h
>>> +++ b/include/linux/damon.h
>>> @@ -326,6 +326,7 @@ struct damos_stat {
>>>     * @DAMOS_FILTER_TYPE_ANON:	Anonymous pages.
>>>     * @DAMOS_FILTER_TYPE_MEMCG:	Specific memcg's pages.
>>>     * @DAMOS_FILTER_TYPE_YOUNG:	Recently accessed pages.
>>> + * @DAMOS_FILTER_TYPE_HUGEPAGE:	Page is part of a hugepage.
>>>     * @DAMOS_FILTER_TYPE_ADDR:	Address range.
>>>     * @DAMOS_FILTER_TYPE_TARGET:	Data Access Monitoring target.
>>>     * @NR_DAMOS_FILTER_TYPES:	Number of filter types.
>>> @@ -345,6 +346,7 @@ enum damos_filter_type {
>>>    	DAMOS_FILTER_TYPE_ANON,
>>>    	DAMOS_FILTER_TYPE_MEMCG,
>>>    	DAMOS_FILTER_TYPE_YOUNG,
>>> +	DAMOS_FILTER_TYPE_HUGEPAGE,
>>>    	DAMOS_FILTER_TYPE_ADDR,
>>>    	DAMOS_FILTER_TYPE_TARGET,
>>>    	NR_DAMOS_FILTER_TYPES,
>>> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
>>> index c0ccf4fade24..224308140441 100644
>>> --- a/mm/damon/paddr.c
>>> +++ b/mm/damon/paddr.c
>>> @@ -222,6 +222,11 @@ static bool damos_pa_filter_match(struct damos_filter *filter,
>>>    		if (matched)
>>>    			damon_folio_mkold(folio);
>>>    		break;
>>> +#if defined(CONFIG_PGTABLE_HAS_HUGE_LEAVES)
>>> +	case DAMOS_FILTER_TYPE_HUGEPAGE:
>>> +		matched = folio_size(folio) == HPAGE_PMD_SIZE;
>>
>>
>> Can we directly embed in the name and the comments/docs that we are only
>> talking about PMD size (both, THP and hugetlb)?
>>
>> DAMOS_FILTER_TYPE_PMD_HUGEPAGE or sth. like that.
> 
> Nice suggestion, thank you!  And we might later add more filter types for
> different size huge pages.  What about extending this to handle more general
> case, though?  That is, we can let the filter receives a range of the folio
> size to match, like DAMOS_FILTER_TYPE_ADDR does.  Then, the filter could be
> used for any size of interest.

That would probably be future proof: either a range or explicitly 
specified sizes (ranges?).

-- 
Cheers,

David / dhildenb