diff mbox series

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

Message ID 20250116144436.3947318-2-usamaarif642@gmail.com (mailing list archive)
State New
Headers show
Series [v1,1/2] mm/damon: have damon_get_folio return folio even for tail pages | expand

Commit Message

Usama Arif Jan. 16, 2025, 2:44 p.m. UTC
This is to gather statistics to check if hotest memory regions
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>
---
 include/linux/damon.h    | 2 ++
 mm/damon/paddr.c         | 3 +++
 mm/damon/sysfs-schemes.c | 1 +
 3 files changed, 6 insertions(+)

Comments

Usama Arif Jan. 16, 2025, 3:15 p.m. UTC | #1
On 16/01/2025 14:44, Usama Arif wrote:
> This is to gather statistics to check if hotest memory regions
> 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>
> ---
>  include/linux/damon.h    | 2 ++
>  mm/damon/paddr.c         | 3 +++
>  mm/damon/sysfs-schemes.c | 1 +
>  3 files changed, 6 insertions(+)

The corresponding DAMO PR is at https://github.com/damonitor/damo/pull/19
SeongJae Park Jan. 16, 2025, 7:05 p.m. UTC | #2
On Thu, 16 Jan 2025 14:44:36 +0000 Usama Arif <usamaarif642@gmail.com> wrote:

> This is to gather statistics to check if hotest memory regions

s/hotest/hottest/?

> are backed by hugepages.

This would also be useful for not only hottest memory regions but general
memory regions of specific access temperature.  I like this.

> This includes both THPs and hugetlbfs.
> This filter can help to observe and prove the effectivenes of
> different schemes for shrinking/collapsing hugepages.

Nice extension of DAMON's page level properties based monitoring!

> 
> Signed-off-by: Usama Arif <usamaarif642@gmail.com>
> ---
>  include/linux/damon.h    | 2 ++
>  mm/damon/paddr.c         | 3 +++
>  mm/damon/sysfs-schemes.c | 1 +
>  3 files changed, 6 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..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) ?

>  	default:
>  		break;
>  	}
> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> index 98f93ae9f59e..de9265f7ccde 100644
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c
> @@ -329,6 +329,7 @@ static const char * const damon_sysfs_scheme_filter_type_strs[] = {
>  	"anon",
>  	"memcg",
>  	"young",
> +	"hugepage",
>  	"addr",
>  	"target",
>  };

Other parts look good to me :)


Thanks,
SJ

[...]
SeongJae Park Jan. 16, 2025, 7:07 p.m. UTC | #3
On Thu, 16 Jan 2025 15:15:21 +0000 Usama Arif <usamaarif642@gmail.com> wrote:

> 
> 
> On 16/01/2025 14:44, Usama Arif wrote:
> > This is to gather statistics to check if hotest memory regions
> > 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>
> > ---
> >  include/linux/damon.h    | 2 ++
> >  mm/damon/paddr.c         | 3 +++
> >  mm/damon/sysfs-schemes.c | 1 +
> >  3 files changed, 6 insertions(+)
> 
> The corresponding DAMO PR is at https://github.com/damonitor/damo/pull/19

Thank you for taking care of the user-space tool together!  I will do review on
the PR.


Thanks,
SJ
Usama Arif Jan. 17, 2025, 11:02 a.m. UTC | #4
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?
SeongJae Park Jan. 17, 2025, 5:08 p.m. UTC | #5
On Fri, 17 Jan 2025 11:02:05 +0000 Usama Arif <usamaarif642@gmail.com> wrote:

> 
> 
> 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?

Yes, that also looks good to me :)


Thanks,
SJ
kernel test robot Jan. 18, 2025, 8:09 a.m. UTC | #6
Hi Usama,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Usama-Arif/mm-damon-introduce-DAMOS-filter-type-hugepage/20250116-224546
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20250116144436.3947318-2-usamaarif642%40gmail.com
patch subject: [v1 2/2] mm/damon: introduce DAMOS filter type hugepage
config: i386-randconfig-014-20250118 (https://download.01.org/0day-ci/archive/20250118/202501181509.wyig2s2H-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250118/202501181509.wyig2s2H-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501181509.wyig2s2H-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from mm/damon/paddr.c:12:
   In file included from include/linux/pagemap.h:8:
   In file included from include/linux/mm.h:2224:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   In file included from mm/damon/paddr.c:17:
   include/linux/mm_inline.h:47:41: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
      47 |         __mod_lruvec_state(lruvec, NR_LRU_BASE + lru, nr_pages);
         |                                    ~~~~~~~~~~~ ^ ~~~
   include/linux/mm_inline.h:49:22: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
      49 |                                 NR_ZONE_LRU_BASE + lru, nr_pages);
         |                                 ~~~~~~~~~~~~~~~~ ^ ~~~
   5 warnings generated.
>> ld.lld: error: call to __compiletime_assert_374 marked "dontcall-error": BUILD_BUG failed
kernel test robot Jan. 18, 2025, 10:34 a.m. UTC | #7
Hi Usama,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Usama-Arif/mm-damon-introduce-DAMOS-filter-type-hugepage/20250116-224546
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20250116144436.3947318-2-usamaarif642%40gmail.com
patch subject: [v1 2/2] mm/damon: introduce DAMOS filter type hugepage
config: x86_64-randconfig-077-20250118 (https://download.01.org/0day-ci/archive/20250118/202501181819.RVctFodO-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250118/202501181819.RVctFodO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501181819.RVctFodO-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from mm/damon/paddr.c:17:
   include/linux/mm_inline.h:47:41: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
      47 |         __mod_lruvec_state(lruvec, NR_LRU_BASE + lru, nr_pages);
         |                                    ~~~~~~~~~~~ ^ ~~~
   include/linux/mm_inline.h:49:22: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
      49 |                                 NR_ZONE_LRU_BASE + lru, nr_pages);
         |                                 ~~~~~~~~~~~~~~~~ ^ ~~~
>> mm/damon/paddr.c:226:34: error: call to '__compiletime_assert_603' declared with 'error' attribute: BUILD_BUG failed
     226 |                 matched = folio_size(folio) == HPAGE_PMD_SIZE;
         |                                                ^
   include/linux/huge_mm.h:111:34: note: expanded from macro 'HPAGE_PMD_SIZE'
     111 | #define HPAGE_PMD_SIZE  ((1UL) << HPAGE_PMD_SHIFT)
         |                                   ^
   include/linux/huge_mm.h:104:28: note: expanded from macro 'HPAGE_PMD_SHIFT'
     104 | #define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
         |                            ^
   include/linux/build_bug.h:59:21: note: expanded from macro 'BUILD_BUG'
      59 | #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
         |                     ^
   note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:530:2: note: expanded from macro '_compiletime_assert'
     530 |         __compiletime_assert(condition, msg, prefix, suffix)
         |         ^
   include/linux/compiler_types.h:523:4: note: expanded from macro '__compiletime_assert'
     523 |                         prefix ## suffix();                             \
         |                         ^
   <scratch space>:50:1: note: expanded from here
      50 | __compiletime_assert_603
         | ^
   2 warnings and 1 error generated.


vim +226 mm/damon/paddr.c

   200	
   201	static bool damos_pa_filter_match(struct damos_filter *filter,
   202			struct folio *folio)
   203	{
   204		bool matched = false;
   205		struct mem_cgroup *memcg;
   206	
   207		switch (filter->type) {
   208		case DAMOS_FILTER_TYPE_ANON:
   209			matched = folio_test_anon(folio);
   210			break;
   211		case DAMOS_FILTER_TYPE_MEMCG:
   212			rcu_read_lock();
   213			memcg = folio_memcg_check(folio);
   214			if (!memcg)
   215				matched = false;
   216			else
   217				matched = filter->memcg_id == mem_cgroup_id(memcg);
   218			rcu_read_unlock();
   219			break;
   220		case DAMOS_FILTER_TYPE_YOUNG:
   221			matched = damon_folio_young(folio);
   222			if (matched)
   223				damon_folio_mkold(folio);
   224			break;
   225		case DAMOS_FILTER_TYPE_HUGEPAGE:
 > 226			matched = folio_size(folio) == HPAGE_PMD_SIZE;
   227			break;
   228		default:
   229			break;
   230		}
   231	
   232		return matched == filter->matching;
   233	}
   234
diff mbox series

Patch

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..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;
 	default:
 		break;
 	}
diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 98f93ae9f59e..de9265f7ccde 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -329,6 +329,7 @@  static const char * const damon_sysfs_scheme_filter_type_strs[] = {
 	"anon",
 	"memcg",
 	"young",
+	"hugepage",
 	"addr",
 	"target",
 };