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

[David Hildenbrand <david@redhat.com>]

On 20.01.25 20:18, Usama Arif wrote:
> 
> 
> On 20/01/2025 18:57, David Hildenbrand 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.
>>
>>
> 
> I always think of gigantic page as PUD size, hugepage as PMD size (and not anything smaller), mTHP as smaller than PMD :)

And cont-PMD ... hugetlb, cont-PTE hugetlb ... are ? :)

> But I don't think thats the official term or standardized across the kernel.
> 

We didn't know better when we designed some of the old interfaces, 
that's why I am hoping that we can do better with new interfaces. Either 
spell out the PMD size, or have filters for size lists / size ranges.

> I can send a v4, or maybe Andrew could apply the diff below?
> 
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 1d94d7d88b36..261c0741dd0f 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -326,7 +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_PMD_HUGEPAGE:    Page is part of a hugepage (THP/hugetlb).
>    * @DAMOS_FILTER_TYPE_ADDR:    Address range.
>    * @DAMOS_FILTER_TYPE_TARGET:  Data Access Monitoring target.
>    * @NR_DAMOS_FILTER_TYPES:     Number of filter types.
> @@ -346,7 +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_PMD_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 224308140441..e374ea952308 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -223,7 +223,7 @@ static bool damos_pa_filter_match(struct damos_filter *filter,
>                          damon_folio_mkold(folio);
>                  break;
>   #if defined(CONFIG_PGTABLE_HAS_HUGE_LEAVES)
> -       case DAMOS_FILTER_TYPE_HUGEPAGE:
> +       case DAMOS_FILTER_TYPE_PMD_HUGEPAGE:
>                  matched = folio_size(folio) == HPAGE_PMD_SIZE;
>                  break;
>   #endif
> 


-- 
Cheers,

David / dhildenb