Re: [RFC PATCH v2] samples/damon: support automatic node address detection

[SeongJae Park <sj@kernel.org>]

Hello Yunjeong,

On Thu,  3 Jul 2025 16:44:22 +0900 Yunjeong Mun <yunjeong.mun@sk.com> wrote:

> This patch adds a new knob `detect_node_addresses`, which determines
> whether the physical address range is set manually using the existing
> knobs or automatically by the mtier module. When `detect_node_addresses`
> set to 'Y', mtier automatically converts node0 and node1 to their
> physical addresses. If set to 'N', it uses the existing
> 'node#_start_addr' and 'node#_end_addr' to define regions as before.

Thank you for this patch!

> 
> Suggested-by: Honggyu Kim <honggyu.kim@sk.com>
> Signed-off-by: Yunjeong Mun <yunjeong.mun@sk.com>

Reviewed-by: SeongJae Park <sj@kernel.org>

> ---

From next time, please consider adding a summary of what changes have made from
the previous version here, like suggested[1] on the documentation.

>  samples/damon/mtier.c | 42 +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 39 insertions(+), 3 deletions(-)
> 
> diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
> index f3220d6e6739..3570ebe10fab 100644
> --- a/samples/damon/mtier.c
> +++ b/samples/damon/mtier.c
> @@ -42,8 +42,34 @@ static bool enable __read_mostly;
>  module_param_cb(enable, &enable_param_ops, &enable, 0600);
>  MODULE_PARM_DESC(enable, "Enable of disable DAMON_SAMPLE_MTIER");
>  
> +static bool detect_node_addresses __read_mostly;
> +module_param(detect_node_addresses, bool, 0600);
> +
>  static struct damon_ctx *ctxs[2];
>  
> +struct region_range {
> +	phys_addr_t start;
> +	phys_addr_t end;
> +};
> +
> +static int nid_to_phys(int target_node, struct region_range *range)
> +{
> +
> +	if (!node_online(target_node)) {
> +		pr_err("NUMA node %d is not online\n", target_node);
> +		return -EINVAL;
> +	}
> +
> +	/* TODO: Do we need to support more accurate region range?  */

I understand you are saying we might need to remove address ranges in the node
that DAMON will anyway unable to check accesses, e.g., reserved memory.  Since
those are uusally only a small portion and this is a sample code, I think we
don't really need to do that, so I think you can drop this TODO comment from
your next version, if you are gonna make it.

> +	unsigned long start_pfn = node_start_pfn(target_node);
> +	unsigned long end_pfn   = node_end_pfn(target_node);
> +
> +	range->start = PFN_PHYS(start_pfn);
> +	range->end  = PFN_PHYS(end_pfn);
> +
> +	return 0;
> +}
[...]

[1] https://docs.kernel.org/process/submitting-patches.html#commentary


Thanks,
SJ