[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <38d2f33e-03c1-4b33-8e0e-1a38ebbf3305@huawei-partners.com>
Date: Tue, 3 Feb 2026 16:04:19 +0300
From: Gutierrez Asier <gutierrez.asier@...wei-partners.com>
To: SeongJae Park <sj@...nel.org>
CC: <artem.kuzin@...wei.com>, <stepanov.anatoly@...wei.com>,
<wangkefeng.wang@...wei.com>, <yanquanmin1@...wei.com>, <zuoze1@...wei.com>,
<damon@...ts.linux.dev>, <akpm@...ux-foundation.org>, <linux-mm@...ck.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v1 1/4] mm/damon: Generic context creation for modules
On 2/3/2026 4:16 AM, SeongJae Park wrote:
> Let's use 'mm/damon/modules-common:' as the commit subject prefix.
>
> On Mon, 2 Feb 2026 14:56:46 +0000 <gutierrez.asier@...wei-partners.com> wrote:
>
>> From: Asier Gutierrez <gutierrez.asier@...wei-partners.com>
>>
>> Damon_modules_new_paddr_ctx_target. This works only for physical contexts.
>
> Let's be case-sensitive. s/Damon_/damon_/ ?
OK, will do it.
>
>> In case of virtual addresses, we should duplicate the code.
>>
>> It is more elegant to have a generic version of new context creation which
>> receives the mode as a parameter.
>
> Makes sense to me.
>
>>
>> Signed-off-by: Asier Gutierrez <gutierrez.asier@...wei-partners.com>
>> Co-developed-by: Anatoly Stepanov <stepanov.anatoly@...wei.com>
>> ---
>> mm/damon/lru_sort.c | 6 ++++--
>> mm/damon/modules-common.c | 7 ++++---
>> mm/damon/modules-common.h | 5 +++--
>> mm/damon/reclaim.c | 5 +++--
>> 4 files changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
>> index 49b4bc294f4e..ac34b02dace8 100644
>> --- a/mm/damon/lru_sort.c
>> +++ b/mm/damon/lru_sort.c
>> @@ -201,7 +201,8 @@ static int damon_lru_sort_apply_parameters(void)
>> unsigned int hot_thres, cold_thres;
>> int err;
>>
>> - err = damon_modules_new_paddr_ctx_target(¶m_ctx, ¶m_target);
>> + err = damon_modules_new_ctx_target(¶m_ctx, ¶m_target,
>> + DAMON_OPS_PADDR);
>> if (err)
>> return err;
>>
>> @@ -375,7 +376,8 @@ 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);
>
> You could put the above line on the line before that without violating the 80
> columns limit :)
Sure, will correct it.
>
>> if (err)
>> goto out;
>>
>> diff --git a/mm/damon/modules-common.c b/mm/damon/modules-common.c
>> index 86d58f8c4f63..5ba24e0ad9a1 100644
>> --- a/mm/damon/modules-common.c
>> +++ b/mm/damon/modules-common.c
>> @@ -14,8 +14,9 @@
>> * @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)
>
> You should also update the kernel-doc comments for the new parameter.
>
>> {
>> struct damon_ctx *ctx;
>> struct damon_target *target;
>> @@ -24,7 +25,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..87d8058d7d85 100644
>> --- a/mm/damon/modules-common.h
>> +++ b/mm/damon/modules-common.h
>> @@ -45,5 +45,6 @@
>> 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 36a582e09eae..b64fb810e096 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(¶m_ctx, ¶m_target);
>> + err = damon_modules_new_ctx_target(¶m_ctx, ¶m_target,
>> + DAMON_OPS_PADDR);
>> if (err)
>> return err;
>>
>> @@ -379,7 +380,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
>
> Overall, looks good for RFC.
>
>
> Thanks,
> SJ
>
--
Asier Gutierrez
Huawei
Powered by blists - more mailing lists