[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260203011633.67532-1-sj@kernel.org>
Date: Mon, 2 Feb 2026 17:16:32 -0800
From: SeongJae Park <sj@...nel.org>
To: gutierrez.asier@...wei-partners.com
Cc: SeongJae Park <sj@...nel.org>,
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
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_/ ?
> 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 :)
> 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
Powered by blists - more mailing lists