Re: (sashiko review) [RFC PATCH v4 01/10] mm/damon/core: introduce damon_ctx->paused

[SeongJae Park <sj@kernel.org>]

Forwarding Sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/20260322155728.81434-2-sj@kernel.org
# start of sashiko.dev inline review
commit 2a3be2c8b6c327ef72163cd54e6121ef72b9c932
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: introduce damon_ctx->paused

DAMON supports only start and stop of the execution. When it is
stopped, its internal data that it self-trained goes away. It will be
useful if the execution can be paused and resumed with the previous
self-trained data.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index db6c67e52d2b..0ab2cfa848e6 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -3003,6 +3004,14 @@ static int kdamond_fn(void *data)
>  		kdamond_call(ctx, false);
>  		if (ctx->maybe_corrupted)
>  			break;
> +		while (ctx->pause) {
> +			damos_walk_cancel(ctx);

Is it safe to repeatedly call damos_walk_cancel() here?

In damos_walk(), the walk_control_lock is dropped after setting
ctx->walk_control to check damon_is_running(), which opens a race window.

Concurrently, kdamond_fn() calls damos_walk_cancel(), which reads
ctx->walk_control, drops the lock, and completes the old request.

If a new caller invokes damos_walk() and successfully registers a new
walk_control, could damos_walk_cancel() resume, reacquire the lock,
and unconditionally set ctx->walk_control = NULL, dropping the new
request?

This might leave the new caller permanently deadlocked in
wait_for_completion().

> +			kdamond_usleep(ctx->attrs.sample_interval);

Could this result in a busy loop if sample_interval is 0?

The DAMON API allows attrs.sample_interval to be 0 (for example, when
tuning is disabled via goal.aggrs == 0).

If sample_interval is 0, this invokes usleep_range_idle(0, 1), which
sleeps for at most 1 microsecond using an hrtimer.

Since the normally expensive monitoring operations are bypassed while
paused, would this cause the thread to loop almost instantly and create
an interrupt storm?

> +			/* allow caller unset pause via damon_call() */
> +			kdamond_call(ctx, false);
> +			if (kdamond_need_stop(ctx) || ctx->maybe_corrupted)
> +				goto done;
> +		}
>  		if (!list_empty(&ctx->schemes))
>  			kdamond_apply_schemes(ctx);
>  		else


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

Sent using hkml (https://github.com/sjp38/hackermail)