Re: [PATCH] samples/damon/mtier: fail early if address range parameters are invalid

[Zenghui Yu]

Hi SeongJae,

On 6/8/26 10:32 PM, SeongJae Park wrote:
> On Mon, 8 Jun 2026 19:15:34 +0800 Zenghui Yu <yuzenghui@xxxxxxxxxx> wrote:
> 
> > The comment on top of `struct damon_region` clearly says that
> >
> >     For any use case, @ar should be non-zero positive size.
> >
> > which is now verified in damon_verify_new_region() if the kernel is built
> > with DAMON_DEBUG_SANITY.
> >
> > The WARN_ONCE() can be triggered if the mtier sample module is enabled
> > before node{0,1}_{start,end}_addr have been properly initialized, which is
> > obviously not good.
> >
> >  ------------[ cut here ]------------
> >  start 0 >= end 0
> >  WARNING: mm/damon/core.c:116 at damon_new_region+0xf0/0x118, CPU#39: bash/34144
> >  Call trace:
> >   damon_new_region+0xf0/0x118 (P)
> >   damon_sample_mtier_build_ctx+0xd4/0x368
> >   damon_sample_mtier_start+0x1c/0x90
> >   damon_sample_mtier_enable_store+0x98/0xb0
> >   param_attr_store+0xb4/0x128
> >   module_attr_store+0x2c/0x50
> >   sysfs_kf_write+0x58/0x90
> >   kernfs_fop_write_iter+0x16c/0x238
> >   vfs_write+0x2c0/0x370
> >   ksys_write+0x74/0x118
> >   __arm64_sys_write+0x24/0x38
> >   invoke_syscall+0xa8/0x118
> >   el0_svc_common.constprop.0+0x48/0xf0
> >   do_el0_svc+0x24/0x38
> >   el0_svc+0x54/0x370
> >   el0t_64_sync_handler+0xa0/0xe8
> >   el0t_64_sync+0x1ac/0x1b0
> >  ---[ end trace 0000000000000000 ]---
> >
> > Fix it by checking the validity of parameters right before
> > damon_new_region() and fail early if they're invalid.
> 
> Good catch, thank you for this patch.
> 
> >
> > Signed-off-by: Zenghui Yu <yuzenghui@xxxxxxxxxx>
> > ---
> >  samples/damon/mtier.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
> > index 775838a23d93..4a5d3fb12e1b 100644
> > --- a/samples/damon/mtier.c
> > +++ b/samples/damon/mtier.c
> > @@ -118,6 +118,9 @@ static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote)
> >  	} else {
> >  		addr.start = promote ? node1_start_addr : node0_start_addr;
> >  		addr.end = promote ? node1_end_addr : node0_end_addr;
> > +
> > +		if (addr.start >= addr.end)
> > +			goto free_out;
> >  	}
> 
> Sashiko found [1] same issue can happen if detect_node_addresses is true, and
> nodes 0 and 1 are both memoryless.  It shouldn't be a blocker of this patch,
> but fixing it together can be very simple by moving this address check to the
> out of the above if block, right here.  Zenghui, could you please update this
> patch to do that?

Yup, it's worth fixing. I will address the detect_node_addresses issue
in v2.

> 
> Also, seems this patch is based on an old tree.  Could you please use
> mm-new [2] as the base of your DAMON patches from next time?

Ah, I'm not familiar with the development process of DAMON and I created
this patch against mainline kernel. I'll re-test the whole thing on top
of mm-new. Thanks for the reminder!

Zenghu