Re: (sashiko review) [PATCH] mm/damon: add synchronous commit for commit_inputs

[SeongJae Park <sj@kernel.org>]

On Sat, 28 Mar 2026 18:44:50 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
[...]
> > > +static const struct kernel_param_ops commit_inputs_param_ops = {
> > 
> > The previous module_param(..., bool) macro used param_ops_bool, which
> > includes the KERNEL_PARAM_OPS_FL_NOARG flag. This allows parameters to be
> > provided as a standalone flag without an explicit value.
> > 
> > By omitting this flag here, does providing the parameter without an explicit
> > value now produce a parsing error?
> 
> To be honest, I was not aware of the implications of omitting the NOARG
> flag. It was not a deliberate choice on my part.
> 
> SeongJae, what are your thoughts on this? SHould we stick with the
> explicit 'commit_inputs=Y' requirement for the new synchronous behavior,
> or should we restore the NOARG compatibility?

Should be a no big deal.  But it would better to keep the compatibility unless
it makes things too difficult.  And it seems simple to do.  Could you please do
that?

> 
> > > +	.set = damon_lru_sort_commit_inputs_store,
> > > +	.get = param_get_bool,
> > > +};
> > 
> > The commit_inputs_store function parses the input into a local variable
> > commit_inputs_request but never writes to the global commit_inputs variable.
> > 
> > Because param_get_bool reads the unmodified global commit_inputs variable,
> > will sysfs reads of commit_inputs unconditionally return 'N'?
> 
> This is actually a design choice we discussed [2]. Since 'commit_inputs'
> is now a synchronous trigger (or a "button") rather than a persistent
> state, it doesn't necessarily need to store the 'Y' value.

I agree to all Liew's opinions.

> 
> > [ ... ]
> > 
> > (Note: The same regressions appear to exist in mm/damon/reclaim.c)
> 
> Thank you for your review. :>

Thank you for reviewing the review.

> 
> [1] https://lore.kernel.org/20260327233319.3528-1-sj@kernel.org (Lastest
> but may not the last fix)
> [2] https://lore.kernel.org/20260323150544.81042-1-sj@kernel.org


Thanks,
SJ

[...]