Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   SeongJae Park <sj38.park@gmail.com>
To:     sj38.park@gmail.com
Cc:     jgowans@amazon.com, akpm@linux-foundation.org,
        SeongJae Park <sjpark@amazon.de>, Jonathan.Cameron@Huawei.com,
        acme@kernel.org, alexander.shishkin@linux.intel.com,
        amit@kernel.org, benh@kernel.crashing.org,
        brendanhiggins@google.com, corbet@lwn.net, david@redhat.com,
        dwmw@amazon.com, elver@google.com, fan.du@intel.com,
        foersleo@amazon.de, greg@kroah.com, gthelen@google.com,
        guoju.fgj@alibaba-inc.com, mgorman@suse.de, minchan@kernel.org,
        mingo@redhat.com, namhyung@kernel.org, peterz@infradead.org,
        riel@surriel.com, rientjes@google.com, rostedt@goodmis.org,
        rppt@kernel.org, shakeelb@google.com, shuah@kernel.org,
        snu@zelle79.org, vbabka@suse.cz, vdavydov.dev@gmail.com,
        zgf574564920@gmail.com, linux-damon@amazon.com, linux-mm@kvack.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 10/13] mm/damon/schemes: Activate schemes based on a watermarks mechanism
Date:   Mon,  7 Jun 2021 08:40:15 +0000
Message-Id: <20210607084015.1620-1-sjpark@amazon.de>
In-Reply-To: <20210531133816.12689-11-sj38.park@gmail.com>
Precedence: bulk

From: SeongJae Park <sjpark@amazon.de>

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

> From: SeongJae Park <sjpark@amazon.de>
> 
> 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@amazon.de>
> ---
>  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@amazon.com) 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

[...]