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

[Liew Rui Yan <aethernet65535@gmail.com>]

On Sat, 28 Mar 2026 07:09:06 -0700 SeongJae Park <sj@kernel.org> wrote:

> On Sat, 28 Mar 2026 18:44:50 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> >
> > [...]
> > > 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?

Sure, I will restore the NOARG compatibility in the next version.

> > 
> > > > +	.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.

Great! Glad we are aligned on the design.

> [...]
> > [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
> [...]

Best regards,
Rui Yan