On 20/01/2025 18:08, SeongJae Park wrote:
> On Mon, 20 Jan 2025 10:16:50 +0000 Usama Arif <usamaarif642@xxxxxxxxx> 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@xxxxxxxxx>
>> ---
>> v1 -> v2:
>> - Wrap DAMOS_FILTER_TYPE_HUGEPAGE case with
>> CONFIG_PGTABLE_HAS_HUGE_LEAVES (SJ Park)
>> ---
>> include/linux/damon.h | 4 ++++
>> mm/damon/paddr.c | 5 +++++
>> mm/damon/sysfs-schemes.c | 3 +++
>> 3 files changed, 12 insertions(+)
>>
>> diff --git a/include/linux/damon.h b/include/linux/damon.h
>> index af525252b853..b0dbf380ab76 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,9 @@ enum damos_filter_type {
>> DAMOS_FILTER_TYPE_ANON,
>> DAMOS_FILTER_TYPE_MEMCG,
>> DAMOS_FILTER_TYPE_YOUNG,
>> +#if defined(CONFIG_PGTABLE_HAS_HUGE_LEAVES)
>> + DAMOS_FILTER_TYPE_HUGEPAGE,
>> +#endif
>
> I'd prefer not enclosing this part with CONFIG_PGTABLE_HAS_HUGE_LEAVES, so that
> users can simply use it. If the config is not set, damos_pa_filter_match()
> will just say nothing is hugepage, and hence will work sanely, in my opinion.
>
I was going along the path of not even exposing it to the userspace in sysfs,
so that the user might not get confused about hugepages. But I agree the alternate
is good a well, with just 0 being returned. I will send v3 without it.
>> 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;
>> + break;
>> +#endif
>
> This part looks good to me.
>
>> default:
>> break;
>> }
>> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
>> index 98f93ae9f59e..3eb3ec464591 100644
>> --- a/mm/damon/sysfs-schemes.c
>> +++ b/mm/damon/sysfs-schemes.c
>> @@ -329,6 +329,9 @@ static const char * const damon_sysfs_scheme_filter_type_strs[] = {
>> "anon",
>> "memcg",
>> "young",
>> +#if defined(CONFIG_PGTABLE_HAS_HUGE_LEAVES)
>> + "hugepage",
>> +#endif
>
> Again, I'd prefer not enclosing this part.
>
>> "addr",
>> "target",
>> };
>> --
>> 2.43.5
>
>
> Thanks,
> SJ
