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

Gutierrez Asier
2026-03-11T13:10:21Z

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