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

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

> Forwarding Sashiko review for doing discussions via mails.
> 
> # review url: https://sashiko.dev/#/patchset/20260327062558.66392-1-aethernet65535@gmail.com
> # start of sashiko.dev inline review
> commit 405545126e8bb145d7fb1fbb4f36016bc1f6c42c
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> Date:   Fri Mar 27 06:25:58 2026 +0000
> 
> mm/damon: add synchronous commit for commit_inputs
> 
> This commit modifies the commit_inputs_store() callback to use damon_call()
> for synchronously committing parameters in the kdamond thread's context.
> This change aims to ensure validation errors are returned immediately to
> userspace.
> 
> Link: https://lore.kernel.org/20260318153731.97470-1-aethernet65535@gmail.com
> 
> > diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> > index 554559d72976..453033f3cb3e 100644
> > --- a/mm/damon/lru_sort.c
> > +++ b/mm/damon/lru_sort.c
> 
> [ ... ]
> 
> > @@ -349,18 +348,47 @@ static int damon_lru_sort_apply_parameters(void)
> >  	return err;
> >  }
> >  
> > -static int damon_lru_sort_handle_commit_inputs(void)
> > +static int damon_lru_sort_commit_inputs_fn(void *arg)
> > +{
> > +	return damon_lru_sort_apply_parameters();
> > +}
> > +
> > +static int damon_lru_sort_commit_inputs_store(const char *val,
> > +		const struct kernel_param *kp)
> > +{
> > +	bool commit_inputs_request;
> >  	int err;
> > +	struct damon_call_control control = {
> > +		.fn = damon_lru_sort_commit_inputs_fn,
> > +		.repeat = false,
> > +	};
> 
> [ ... ]
> 
> > +	err = damon_call(ctx, &control);
> > +
> > +	return control.return_code;
> >  }
> 
> If damon_call() fails, for example returning -EINVAL when kdamond is not
> running, err is set but the error code is never returned or checked.
> 
> Will this cause the sysfs write to incorrectly succeed by returning the
> default control.return_code of 0?
> 
> Should it return err if err is non-zero, or control.return_code otherwise?

Thank you for reminder. I will add
'return err ? err : control.return_code' back in next patch.

> > diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> > index 86da14778658..03e3300ce607 100644
> > --- a/mm/damon/reclaim.c
> > +++ b/mm/damon/reclaim.c
> 
> [ ... ]
> 
> > +static int damon_reclaim_commit_inputs_store(const char *val,
> > +		const struct kernel_param *kp)
> > +{
> > +	bool commit_inputs_request;
> >  	int err;
> > +	struct damon_call_control control = {
> > +		.fn = damon_reclaim_commit_inputs_fn,
> > +		.repeat = false,
> > +	};
> 
> [ ... ]
> 
> > +	err = damon_call(ctx, &control);
> > +
> > +	return control.return_code;
> >  }
> 
> Does this function have the same issue where the return value of
> damon_call() is ignored?

Yes, it have the same issue. Thank you for reminder. :>

Best regards,
Rui Yan