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

[SeongJae Park <sj@kernel.org>]

On Fri,  4 Jul 2025 16:05:59 +0900 Yunjeong Mun <yunjeong.mun@sk.com> wrote:

> Hello Seongjae,
> 
> On Thu,  3 Jul 2025 09:52:37 -0700 SeongJae Park <sj@kernel.org> wrote:
> > 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.
> 
> Ok, I'll add it next time, thanks:)
> One concern I have about this patch is the requirement to set
> 'detect_node_addresses=Y' before setting 'enable=Y'. Not following
> this order causes an error, which makes it difficult for users to use
> the module.

That's same to existing address parameters, and similar to existing DAMON user
interfaces.  Parameters are applied when starting DAMON.  Parameters can be
updated while DAMON is running, but it requires explicit "commit" action for
updating those at once.

DAMON sample modules don't support the runtime commit feature, though.  I think
it is not a bad tradeoff for simplicity of the code, given the purpose of
sample modules.

> So, how about removing 'detect_node_address'? Instead, we
> could convert node0,1 to physical address automatically by default, and use
> existing 'node#_*_addr' values only when those files are explicitly set.

This would make old usage broken.  Since this is a sample module, I think that
could be justified if there is a very good reason.  But I don't think we have a
very good reason here.  So I suggest not to do that.


Thanks,
SJ

[...]