Re: [PATCH v4 4/6] mm/damon: introduce DAMOS filter type hugepage

[SeongJae Park <sj@kernel.org>]

On Mon,  3 Feb 2025 22:55:31 +0000 Usama Arif <usamaarif642@gmail.com> wrote:

> This is to gather statistics to check if memory regions of specific
> access tempratures are backed by hugepages of a size in a specific
> range

nit.  A period is missed?

> 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>
> ---
>  include/linux/damon.h    | 2 ++
>  mm/damon/paddr.c         | 7 +++++++
>  mm/damon/sysfs-schemes.c | 1 +
>  3 files changed, 10 insertions(+)
> 
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 6f30ceeff215..5ba6c2114e3f 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -336,6 +336,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.

What about "DAMOS_FILTER_TYPE_HUGEPAGE_SIZE: hugepages of a given size range."?

>   * @DAMOS_FILTER_TYPE_ADDR:	Address range.
>   * @DAMOS_FILTER_TYPE_TARGET:	Data Access Monitoring target.
>   * @NR_DAMOS_FILTER_TYPES:	Number of filter types.
> @@ -355,6 +356,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 3f59a3fdc391..34fe1eb664cc 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -227,6 +227,7 @@ static bool damos_pa_filter_match(struct damos_filter *filter,
>  {
>  	bool matched = false;
>  	struct mem_cgroup *memcg;
> +	size_t folio_sz;
>  
>  	switch (filter->type) {
>  	case DAMOS_FILTER_TYPE_ANON:
> @@ -246,6 +247,12 @@ static bool damos_pa_filter_match(struct damos_filter *filter,
>  		if (matched)
>  			damon_folio_mkold(folio);
>  		break;
> +#if defined(CONFIG_PGTABLE_HAS_HUGE_LEAVES)

I think we don't really need this macro?

> +	case DAMOS_FILTER_TYPE_HUGEPAGE:
> +		folio_sz = folio_size(folio);
> +		matched = filter->folio_size.min <= folio_sz && folio_sz <= filter->folio_size.max;

We should also return 'false' if the folio is not a large folio (folio_sz ==
PAGE_SIZE), if we agreed to my suggestion on the previous version of this patch
series?

I'd also prefer calling filter->folio_siz as sz_range or size_range, so that we
can reuse it for future filter types of size range.

> +		break;
> +#endif
>  	default:
>  		break;
>  	}
> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> index bc7ca43ca9c4..76aee3ab277e 100644
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c
> @@ -330,6 +330,7 @@ static const char * const damon_sysfs_scheme_filter_type_strs[] = {
>  	"anon",
>  	"memcg",
>  	"young",
> +	"hugepage",

hugepage_size?

>  	"addr",
>  	"target",
>  };
> -- 
> 2.43.5


Thanks,
SJ