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?

> 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?


# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/20260327062558.66392-1-aethernet65535@gmail.com
#
# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260327062558.66392-1-aethernet65535@gmail.com
#
# [1] https://github.com/sjp38/hackermail