linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: gutierrez.asier@huawei-partners.com
Cc: SeongJae Park <sj@kernel.org>,
	artem.kuzin@huawei.com, stepanov.anatoly@huawei.com,
	wangkefeng.wang@huawei.com, yanquanmin1@huawei.com,
	zuoze1@huawei.com, damon@lists.linux.dev,
	akpm@linux-foundation.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1 1/4] mm/damon: Generic context creation for modules
Date: Mon,  2 Feb 2026 17:16:32 -0800	[thread overview]
Message-ID: <20260203011633.67532-1-sj@kernel.org> (raw)
In-Reply-To: <20260202145650.1795854-2-gutierrez.asier@huawei-partners.com>

Let's use 'mm/damon/modules-common:' as the commit subject prefix.

On Mon, 2 Feb 2026 14:56:46 +0000 <gutierrez.asier@huawei-partners.com> wrote:

> From: Asier Gutierrez <gutierrez.asier@huawei-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@huawei-partners.com>
> Co-developed-by: Anatoly Stepanov <stepanov.anatoly@huawei.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(&param_ctx, &param_target);
> +	err = damon_modules_new_ctx_target(&param_ctx, &param_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(&param_ctx, &param_target);
> +	err = damon_modules_new_ctx_target(&param_ctx, &param_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


  reply	other threads:[~2026-02-03  1:16 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-02 14:56 [RFC PATCH v1 0/4] mm/damon: Support hot application detections gutierrez.asier
2026-02-02 14:56 ` [RFC PATCH v1 1/4] mm/damon: Generic context creation for modules gutierrez.asier
2026-02-03  1:16   ` SeongJae Park [this message]
2026-02-03 13:04     ` Gutierrez Asier
2026-02-02 14:56 ` [RFC PATCH v1 2/4] mm/damon: Support for synchrounous huge pages collapse gutierrez.asier
2026-02-03  1:23   ` SeongJae Park
2026-02-03 14:04     ` Gutierrez Asier
2026-02-02 14:56 ` [RFC PATCH v1 3/4] mm/damon: New module with hot application detection gutierrez.asier
2026-02-03  5:04   ` SeongJae Park
2026-02-03 14:21     ` Gutierrez Asier
2026-02-02 14:56 ` [RFC PATCH v1 4/4] documentation/mm/damon: Documentation for the dynamic_hugepages module gutierrez.asier
2026-02-03  5:34   ` SeongJae Park
2026-02-03  1:10 ` [RFC PATCH v1 0/4] mm/damon: Support hot application detections SeongJae Park
2026-02-03 13:03   ` Gutierrez Asier
2026-02-04  7:31     ` SeongJae Park
2026-02-03 14:25   ` Gutierrez Asier
2026-02-04  7:17     ` SeongJae Park
2026-02-04 13:07       ` Gutierrez Asier
2026-02-04 15:43         ` SeongJae Park
2026-02-11  6:59 ` SeongJae Park
2026-02-11 11:29   ` Gutierrez Asier
2026-02-11 15:09     ` SeongJae Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260203011633.67532-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=artem.kuzin@huawei.com \
    --cc=damon@lists.linux.dev \
    --cc=gutierrez.asier@huawei-partners.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=stepanov.anatoly@huawei.com \
    --cc=wangkefeng.wang@huawei.com \
    --cc=yanquanmin1@huawei.com \
    --cc=zuoze1@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox