Re: [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()

On Thu, 13 Aug 2026 04:02:07 +0000 [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch introduces an inconsistent state in the `damon_ctx` API by initializing `call_controls_obsolete` to true while leaving its identically-designed sibling field `walk_control_obsolete` as false.
> --
> 
> --- Patch [2]: [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx() ---
> commit 94c12d6bfcca8f83b408960a36d0c0a8519d80c7
> Author: SJ Park <[email protected]>
> 
> mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
> 
> This commit addresses an issue where passing an unstarted DAMON context
> to damon_call() can cause an indefinite hang. It fixes this by initializing
> ctx->call_controls_obsolete to true in damon_new_ctx().
> 
> Link: https://lore.kernel.org/[email protected]
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index af7b77710745d..5080cb12220d2 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -864,6 +864,7 @@ struct damon_ctx *damon_new_ctx(void)
> >  	INIT_LIST_HEAD(&ctx->adaptive_targets);
> >  	INIT_LIST_HEAD(&ctx->schemes);
> >  
> > +	ctx->call_controls_obsolete = true;
> 
> [Severity: Medium]
> Should ctx->walk_control_obsolete also be initialized to true here?
> 
> Because walk_control_obsolete is zero-initialized to false during allocation,
> an unstarted context passed to damos_walk() will bypass the obsolete check:
> 
> mm/damon/core.c:damos_walk() {
> 	...
> 	if (ctx->walk_control_obsolete) {
> 		mutex_unlock(&ctx->walk_control_lock);
> 		return -ECANCELED;
> 	}
> 	...
> }
> 
> This appears to leave damos_walk() vulnerable to the exact same indefinite
> wait_for_completion() hang on unstarted contexts that this patch resolves for
> damon_call(). 
> 
> Does it make sense to update walk_control_obsolete alongside
> call_controls_obsolete to ensure the API behaves consistently?

The issue could happen in damon_call() because it has repeat mode.
damos_walk() doesn't have the repeat mode, so I don't think the initialization
is needed right now.  I will consider doing so in future, for consistency or a
case that it turns out the infinite hang is really possible.

> 
> >  	prandom_seed_state(&ctx->rnd_state, get_random_u64());
> >  
> >  	return ctx;
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2


Thanks,
SJ