Re: [RFC PATCH 10/13] mm/damon/schemes: Activate schemes based on a watermarks mechanism

[SeongJae Park]

From: SeongJae Park <sjpark@xxxxxxxxx>

On Mon, 31 May 2021 13:38:13 +0000 sj38.park@xxxxxxxxx wrote:

> From: SeongJae Park <sjpark@xxxxxxxxx>
> 
> DAMON-based operation schemes need to be manually turned on and off.  In
> some use cases, however, the condition for turning a scheme on and off
> would depend on the system's situation.  For example, schemes for
> proactive pages reclamation would need to be turned on when some memory
> pressure is detected, and turned off when the system has enough free
> memory.
> 
> For easier control of schemes activation based on the system situation,
> this commit introduces a watermarks-based mechanism.  The client can
> describe the watermark metric (e.g., amount of free memory in the
> system), watermark check interval, and three watermarks, namely high,
> mid, and low.  If the scheme is deactivated, it only gets the metric and
> compare that to the three watermarks for every check interval.  If the
> metric is higher than the high watermark, the scheme is deactivated.  If
> the metric is between the mid watermark and the low watermark, the
> scheme is activated.  If the metric is lower than the low watermark, the
> scheme is deactivated again.  This is to allow users fall back to
> traditional page-granularity mechanisms.
> 
> Signed-off-by: SeongJae Park <sjpark@xxxxxxxxx>
> ---
>  include/linux/damon.h | 52 +++++++++++++++++++++++++-
>  mm/damon/core.c       | 87 ++++++++++++++++++++++++++++++++++++++++++-
>  mm/damon/dbgfs.c      |  5 ++-
>  3 files changed, 141 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 565f49d8ba44..2edd84e98056 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -127,6 +127,45 @@ struct damos_speed_limit {
>  	unsigned int min_score;
>  };
[...]
>  static void set_kdamond_stop(struct damon_ctx *ctx)
>  {
>  	mutex_lock(&ctx->kdamond_lock);
> @@ -904,6 +982,13 @@ static int kdamond_fn(void *data)
>  	sz_limit = damon_region_sz_limit(ctx);
>  
>  	while (!kdamond_need_stop(ctx)) {
> +		unsigned long wmark_wait_us = kdamond_wmark_wait_us(ctx);
> +
> +		if (wmark_wait_us) {
> +			usleep_range(wmark_wait_us, wmark_wait_us + 1);
> +			continue;
> +		}

James Gowans (jgowans@xxxxxxxxxx) found this will make kdamond sleeps in
TASK_UNINTERRUPTIBLE state.  So, when DAMON is deactivated due to the
watermarks rule, the sysadmin assumes it would do nothing and DAMON really do
nothing.  But, because it's sleeping in TASK_UNINTERRUPTIBLE state, which is
usually interpreted as waiting for I/O, '/proc/loadavg' like monitors will
report I/O loads, so that the sysadmin get confused.

In the next version of this RFC patchset, I will make this to use
'schedule_timeout_interruptible()' instead, if 'wmark_wait_us' is larger than
100ms.  I will continue using 'usleep_range()' for small sleep time, to keep
the precision high.


Thanks,
SeongJae Park

[...]