1. Patchew
  2. linux
  3. [RFC PATCH v2 0/4] mm/damon: Support hot application detections
  4. View patch

[RFC PATCH v2 1/4] mm/damon: Generic context creation for modules

gutierrez.asier@huawei-partners.com posted 4 patches 3 weeks, 1 day ago
[RFC PATCH v2 1/4] mm/damon: Generic context creation for modules
Posted by gutierrez.asier@huawei-partners.com 3 weeks, 1 day ago 
From: Asier Gutierrez <gutierrez.asier@huawei-partners.com>

It is more elegant to have a generic version of new context creation
which receives the mode as a parameter.

Signed-off-by: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
Co-developed-by: Anatoly Stepanov <stepanov.anatoly@huawei.com>
---
 mm/damon/lru_sort.c       | 5 +++--
 mm/damon/modules-common.c | 6 +++---
 mm/damon/modules-common.h | 4 ++--
 mm/damon/reclaim.c        | 5 +++--
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 7bc5c0b2aea3..143ee0b21da6 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -287,7 +287,8 @@ static int damon_lru_sort_apply_parameters(void)
 	unsigned int hot_thres, cold_thres;
 	int err;
 
-	err = damon_modules_new_paddr_ctx_target(&param_ctx, &param_target);
+	err = damon_modules_new_ctx_target(&param_ctx, &param_target,
+				DAMON_OPS_PADDR);
 	if (err)
 		return err;
 
@@ -479,7 +480,7 @@ static int __init damon_lru_sort_init(void)
 		err = -ENOMEM;
 		goto out;
 	}
-	err = damon_modules_new_paddr_ctx_target(&ctx, &target);
+	err = damon_modules_new_ctx_target(&ctx, &target, DAMON_OPS_PADDR);
 	if (err)
 		goto out;
 
diff --git a/mm/damon/modules-common.c b/mm/damon/modules-common.c
index 86d58f8c4f63..ae50b2fa3a86 100644
--- a/mm/damon/modules-common.c
+++ b/mm/damon/modules-common.c
@@ -14,8 +14,8 @@
  * @ctxp:	Pointer to save the point to the newly created context
  * @targetp:	Pointer to save the point to the newly created target
  */
-int damon_modules_new_paddr_ctx_target(struct damon_ctx **ctxp,
-		struct damon_target **targetp)
+int damon_modules_new_ctx_target(struct damon_ctx **ctxp,
+		struct damon_target **targetp, enum damon_ops_id mode)
 {
 	struct damon_ctx *ctx;
 	struct damon_target *target;
@@ -24,7 +24,7 @@ int damon_modules_new_paddr_ctx_target(struct damon_ctx **ctxp,
 	if (!ctx)
 		return -ENOMEM;
 
-	if (damon_select_ops(ctx, DAMON_OPS_PADDR)) {
+	if (damon_select_ops(ctx, mode)) {
 		damon_destroy_ctx(ctx);
 		return -EINVAL;
 	}
diff --git a/mm/damon/modules-common.h b/mm/damon/modules-common.h
index f103ad556368..379b49c6a617 100644
--- a/mm/damon/modules-common.h
+++ b/mm/damon/modules-common.h
@@ -45,5 +45,5 @@
 	module_param_named(nr_##qt_exceed_name, stat.qt_exceeds, ulong,	\
 			0400);
 
-int damon_modules_new_paddr_ctx_target(struct damon_ctx **ctxp,
-		struct damon_target **targetp);
+int damon_modules_new_ctx_target(struct damon_ctx **ctxp,
+		struct damon_target **targetp, enum damon_ops_id mode);
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 43d76f5bed44..24786a58683a 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -197,7 +197,8 @@ static int damon_reclaim_apply_parameters(void)
 	struct damos_filter *filter;
 	int err;
 
-	err = damon_modules_new_paddr_ctx_target(&param_ctx, &param_target);
+	err = damon_modules_new_ctx_target(&param_ctx, &param_target,
+				DAMON_OPS_PADDR);
 	if (err)
 		return err;
 
@@ -381,7 +382,7 @@ static int __init damon_reclaim_init(void)
 		err = -ENOMEM;
 		goto out;
 	}
-	err = damon_modules_new_paddr_ctx_target(&ctx, &target);
+	err = damon_modules_new_ctx_target(&ctx, &target, DAMON_OPS_PADDR);
 	if (err)
 		goto out;
 
-- 
2.43.0
Re: [RFC PATCH v2 1/4] mm/damon: Generic context creation for modules
Posted by SeongJae Park 3 weeks, 1 day ago 
On Tue, 10 Mar 2026 16:24:17 +0000 <gutierrez.asier@huawei-partners.com> wrote:

> From: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
> 
> It is more elegant to have a generic version of new context creation
> which receives the mode as a parameter.
> 
> Signed-off-by: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
> Co-developed-by: Anatoly Stepanov <stepanov.anatoly@huawei.com>
> ---
>  mm/damon/lru_sort.c       | 5 +++--
>  mm/damon/modules-common.c | 6 +++---
>  mm/damon/modules-common.h | 4 ++--
>  mm/damon/reclaim.c        | 5 +++--
>  4 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> index 7bc5c0b2aea3..143ee0b21da6 100644
> --- a/mm/damon/lru_sort.c
> +++ b/mm/damon/lru_sort.c
> @@ -287,7 +287,8 @@ static int damon_lru_sort_apply_parameters(void)
>  	unsigned int hot_thres, cold_thres;
>  	int err;
>  
> -	err = damon_modules_new_paddr_ctx_target(&param_ctx, &param_target);
> +	err = damon_modules_new_ctx_target(&param_ctx, &param_target,
> +				DAMON_OPS_PADDR);

I like the name of the function is becoming shorter.  But I'm not happy with
the fact the resulting calling code becomes longer.

I understand you are doing this extension because you want a version of the
function for vaddr.  What about introducing another dedicated function, say,
damon_modules_new_vaddr_ctx_target() ?

And you can avoid duplicates between damon_modules_new_{p,v}addr_ctx_target()
by implementing internal function, say, damon_modules_new_ctx_target() that
receives the damon_ops_id.  And damon_modules_new_{P,v}addr_ctx_target() will
just wrappers of damon_modules_new_ctx_target().

>  	if (err)
>  		return err;
>  
> @@ -479,7 +480,7 @@ static int __init damon_lru_sort_init(void)
>  		err = -ENOMEM;
>  		goto out;
>  	}
> -	err = damon_modules_new_paddr_ctx_target(&ctx, &target);
> +	err = damon_modules_new_ctx_target(&ctx, &target, DAMON_OPS_PADDR);

Ditto.

>  	if (err)
>  		goto out;
>  
> diff --git a/mm/damon/modules-common.c b/mm/damon/modules-common.c
> index 86d58f8c4f63..ae50b2fa3a86 100644
> --- a/mm/damon/modules-common.c
> +++ b/mm/damon/modules-common.c
> @@ -14,8 +14,8 @@
>   * @ctxp:	Pointer to save the point to the newly created context
>   * @targetp:	Pointer to save the point to the newly created target
>   */
> -int damon_modules_new_paddr_ctx_target(struct damon_ctx **ctxp,
> -		struct damon_target **targetp)
> +int damon_modules_new_ctx_target(struct damon_ctx **ctxp,
> +		struct damon_target **targetp, enum damon_ops_id mode)

Nit.  I'd suggest 'ops_id' as the parameter name, instead of 'mode'.

>  {
>  	struct damon_ctx *ctx;
>  	struct damon_target *target;
> @@ -24,7 +24,7 @@ int damon_modules_new_paddr_ctx_target(struct damon_ctx **ctxp,
>  	if (!ctx)
>  		return -ENOMEM;
>  
> -	if (damon_select_ops(ctx, DAMON_OPS_PADDR)) {
> +	if (damon_select_ops(ctx, mode)) {
>  		damon_destroy_ctx(ctx);
>  		return -EINVAL;
>  	}
> diff --git a/mm/damon/modules-common.h b/mm/damon/modules-common.h
> index f103ad556368..379b49c6a617 100644
> --- a/mm/damon/modules-common.h
> +++ b/mm/damon/modules-common.h
> @@ -45,5 +45,5 @@
>  	module_param_named(nr_##qt_exceed_name, stat.qt_exceeds, ulong,	\
>  			0400);
>  
> -int damon_modules_new_paddr_ctx_target(struct damon_ctx **ctxp,
> -		struct damon_target **targetp);
> +int damon_modules_new_ctx_target(struct damon_ctx **ctxp,
> +		struct damon_target **targetp, enum damon_ops_id mode);
> diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> index 43d76f5bed44..24786a58683a 100644
> --- a/mm/damon/reclaim.c
> +++ b/mm/damon/reclaim.c
> @@ -197,7 +197,8 @@ static int damon_reclaim_apply_parameters(void)
>  	struct damos_filter *filter;
>  	int err;
>  
> -	err = damon_modules_new_paddr_ctx_target(&param_ctx, &param_target);
> +	err = damon_modules_new_ctx_target(&param_ctx, &param_target,
> +				DAMON_OPS_PADDR);
>  	if (err)
>  		return err;
>  
> @@ -381,7 +382,7 @@ static int __init damon_reclaim_init(void)
>  		err = -ENOMEM;
>  		goto out;
>  	}
> -	err = damon_modules_new_paddr_ctx_target(&ctx, &target);
> +	err = damon_modules_new_ctx_target(&ctx, &target, DAMON_OPS_PADDR);
>  	if (err)
>  		goto out;
>  
> -- 
> 2.43.0


Thanks,
SJ
Re: [RFC PATCH v2 1/4] mm/damon: Generic context creation for modules
Posted by Gutierrez Asier 3 weeks ago 

On 3/11/2026 3:57 AM, SeongJae Park wrote:
> On Tue, 10 Mar 2026 16:24:17 +0000 <gutierrez.asier@huawei-partners.com> wrote:
> 
>> From: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
>>
>> It is more elegant to have a generic version of new context creation
>> which receives the mode as a parameter.
>>
>> Signed-off-by: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
>> Co-developed-by: Anatoly Stepanov <stepanov.anatoly@huawei.com>
>> ---
>>  mm/damon/lru_sort.c       | 5 +++--
>>  mm/damon/modules-common.c | 6 +++---
>>  mm/damon/modules-common.h | 4 ++--
>>  mm/damon/reclaim.c        | 5 +++--
>>  4 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
>> index 7bc5c0b2aea3..143ee0b21da6 100644
>> --- a/mm/damon/lru_sort.c
>> +++ b/mm/damon/lru_sort.c
>> @@ -287,7 +287,8 @@ static int damon_lru_sort_apply_parameters(void)
>>  	unsigned int hot_thres, cold_thres;
>>  	int err;
>>  
>> -	err = damon_modules_new_paddr_ctx_target(&param_ctx, &param_target);
>> +	err = damon_modules_new_ctx_target(&param_ctx, &param_target,
>> +				DAMON_OPS_PADDR);
> 
> I like the name of the function is becoming shorter.  But I'm not happy with
> the fact the resulting calling code becomes longer.
> 
> I understand you are doing this extension because you want a version of the
> function for vaddr.  What about introducing another dedicated function, say,
> damon_modules_new_vaddr_ctx_target() ?
> 
> And you can avoid duplicates between damon_modules_new_{p,v}addr_ctx_target()
> by implementing internal function, say, damon_modules_new_ctx_target() that
> receives the damon_ops_id.  And damon_modules_new_{P,v}addr_ctx_target() will
> just wrappers of damon_modules_new_ctx_target().

Sure, I will do it.

Since this is some generic code not related to the module I am working on,
maybe I can move this to a different patch set that we can upstream. What do
you think?

> 
>>  	if (err)
>>  		return err;
>>  
>> @@ -479,7 +480,7 @@ static int __init damon_lru_sort_init(void)
>>  		err = -ENOMEM;
>>  		goto out;
>>  	}
>> -	err = damon_modules_new_paddr_ctx_target(&ctx, &target);
>> +	err = damon_modules_new_ctx_target(&ctx, &target, DAMON_OPS_PADDR);
> 
> Ditto.
> 
>>  	if (err)
>>  		goto out;
>>  
>> diff --git a/mm/damon/modules-common.c b/mm/damon/modules-common.c
>> index 86d58f8c4f63..ae50b2fa3a86 100644
>> --- a/mm/damon/modules-common.c
>> +++ b/mm/damon/modules-common.c
>> @@ -14,8 +14,8 @@
>>   * @ctxp:	Pointer to save the point to the newly created context
>>   * @targetp:	Pointer to save the point to the newly created target
>>   */
>> -int damon_modules_new_paddr_ctx_target(struct damon_ctx **ctxp,
>> -		struct damon_target **targetp)
>> +int damon_modules_new_ctx_target(struct damon_ctx **ctxp,
>> +		struct damon_target **targetp, enum damon_ops_id mode)
> 
> Nit.  I'd suggest 'ops_id' as the parameter name, instead of 'mode'.
> 
>>  {
>>  	struct damon_ctx *ctx;
>>  	struct damon_target *target;
>> @@ -24,7 +24,7 @@ int damon_modules_new_paddr_ctx_target(struct damon_ctx **ctxp,
>>  	if (!ctx)
>>  		return -ENOMEM;
>>  
>> -	if (damon_select_ops(ctx, DAMON_OPS_PADDR)) {
>> +	if (damon_select_ops(ctx, mode)) {
>>  		damon_destroy_ctx(ctx);
>>  		return -EINVAL;
>>  	}
>> diff --git a/mm/damon/modules-common.h b/mm/damon/modules-common.h
>> index f103ad556368..379b49c6a617 100644
>> --- a/mm/damon/modules-common.h
>> +++ b/mm/damon/modules-common.h
>> @@ -45,5 +45,5 @@
>>  	module_param_named(nr_##qt_exceed_name, stat.qt_exceeds, ulong,	\
>>  			0400);
>>  
>> -int damon_modules_new_paddr_ctx_target(struct damon_ctx **ctxp,
>> -		struct damon_target **targetp);
>> +int damon_modules_new_ctx_target(struct damon_ctx **ctxp,
>> +		struct damon_target **targetp, enum damon_ops_id mode);
>> diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
>> index 43d76f5bed44..24786a58683a 100644
>> --- a/mm/damon/reclaim.c
>> +++ b/mm/damon/reclaim.c
>> @@ -197,7 +197,8 @@ static int damon_reclaim_apply_parameters(void)
>>  	struct damos_filter *filter;
>>  	int err;
>>  
>> -	err = damon_modules_new_paddr_ctx_target(&param_ctx, &param_target);
>> +	err = damon_modules_new_ctx_target(&param_ctx, &param_target,
>> +				DAMON_OPS_PADDR);
>>  	if (err)
>>  		return err;
>>  
>> @@ -381,7 +382,7 @@ static int __init damon_reclaim_init(void)
>>  		err = -ENOMEM;
>>  		goto out;
>>  	}
>> -	err = damon_modules_new_paddr_ctx_target(&ctx, &target);
>> +	err = damon_modules_new_ctx_target(&ctx, &target, DAMON_OPS_PADDR);
>>  	if (err)
>>  		goto out;
>>  
>> -- 
>> 2.43.0
> 
> 
> Thanks,
> SJ
> 

-- 
Asier Gutierrez
Huawei
Re: [RFC PATCH v2 1/4] mm/damon: Generic context creation for modules
Posted by SeongJae Park 3 weeks ago 
On Wed, 11 Mar 2026 16:10:19 +0300 Gutierrez Asier <gutierrez.asier@huawei-partners.com> wrote:

> 
> 
> On 3/11/2026 3:57 AM, SeongJae Park wrote:
> > On Tue, 10 Mar 2026 16:24:17 +0000 <gutierrez.asier@huawei-partners.com> wrote:
> > 
> >> From: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
> >>
> >> It is more elegant to have a generic version of new context creation
> >> which receives the mode as a parameter.
> >>
> >> Signed-off-by: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
> >> Co-developed-by: Anatoly Stepanov <stepanov.anatoly@huawei.com>
> >> ---
> >>  mm/damon/lru_sort.c       | 5 +++--
> >>  mm/damon/modules-common.c | 6 +++---
> >>  mm/damon/modules-common.h | 4 ++--
> >>  mm/damon/reclaim.c        | 5 +++--
> >>  4 files changed, 11 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> >> index 7bc5c0b2aea3..143ee0b21da6 100644
> >> --- a/mm/damon/lru_sort.c
> >> +++ b/mm/damon/lru_sort.c
> >> @@ -287,7 +287,8 @@ static int damon_lru_sort_apply_parameters(void)
> >>  	unsigned int hot_thres, cold_thres;
> >>  	int err;
> >>  
> >> -	err = damon_modules_new_paddr_ctx_target(&param_ctx, &param_target);
> >> +	err = damon_modules_new_ctx_target(&param_ctx, &param_target,
> >> +				DAMON_OPS_PADDR);
> > 
> > I like the name of the function is becoming shorter.  But I'm not happy with
> > the fact the resulting calling code becomes longer.
> > 
> > I understand you are doing this extension because you want a version of the
> > function for vaddr.  What about introducing another dedicated function, say,
> > damon_modules_new_vaddr_ctx_target() ?
> > 
> > And you can avoid duplicates between damon_modules_new_{p,v}addr_ctx_target()
> > by implementing internal function, say, damon_modules_new_ctx_target() that
> > receives the damon_ops_id.  And damon_modules_new_{P,v}addr_ctx_target() will
> > just wrappers of damon_modules_new_ctx_target().
> 
> Sure, I will do it.

Thanks for accepting my suggestion.

> 
> Since this is some generic code not related to the module I am working on,
> maybe I can move this to a different patch set that we can upstream. What do
> you think?

Please feel free to do so :)


Thanks,
SJ

[...]

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.