damon.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Alex Rusuf <yorha.op@gmail.com>
To: damon@lists.linux.dev
Cc: sj@kernel.org
Subject: [RFC PATCH v1 6/7] mm/damon/core: multi-context support
Date: Wed, 15 May 2024 18:24:56 +0300	[thread overview]
Message-ID: <20240515152457.603724-7-yorha.op@gmail.com> (raw)
In-Reply-To: <20240515152457.603724-1-yorha.op@gmail.com>

This patch actually implements support for
multi-context design for kdamond daemon.

In pseudo code previous versions worked like
the following:

    while (!kdamond_should_stop()) {

	/* prepare accesses for only 1 context */
	prepare_accesses(damon_context);

	sleep(sample_interval);

	/* check accesses for only 1 context */
	check_accesses(damon_context);

	...
    }

With this patch kdamond workflow will look
like the following:

    while (!kdamond_shoule_stop()) {

	/* prepare accesses for all contexts in kdamond */
	damon_for_each_context(ctx, kdamond)
	    prepare_accesses(ctx);

	sleep(sample_interval);

	/* check_accesses for all contexts in kdamond */
	damon_for_each_context(ctx, kdamond)
	    check_accesses(ctx);

	...
    }

Another point to note is watermarks. Previous versions
checked watermarks on each iteration for current context
and if matric's value wan't acceptable kdamond waited
for watermark's sleep interval.

Now there's no need to wait for each context, we can
just skip context if watermark's metric isn't ready,
but if there's no contexts that can run we
check for each context's watermark metric and
sleep for the lowest interval of all contexts.

Signed-off-by: Alex Rusuf <yorha.op@gmail.com>
---
 include/linux/damon.h     |   8 +-
 mm/damon/core.c           | 255 +++++++++++++++++++++++---------------
 mm/damon/modules-common.c |   1 -
 mm/damon/sysfs.c          |  48 ++++---
 4 files changed, 194 insertions(+), 118 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index ec291a2eb..972ecf509 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -572,7 +572,6 @@ struct kdamond_struct {
 	struct mutex lock;
 	struct task_struct *self;
 	struct list_head contexts;
-	size_t nr_ctxs;
 
 /* private: */
 	/* for waiting until the execution of the kdamond_fn is started */
@@ -626,6 +625,7 @@ struct damon_ctx {
 	 */
 	unsigned long next_ops_update_sis;
 	unsigned long sz_limit;
+	bool valid;
 
 /* public: */
 	struct kdamond_struct *kdamond;
@@ -673,6 +673,12 @@ static inline struct damon_ctx *damon_first_ctx(struct kdamond_struct *kdamond)
 	return list_first_entry(&kdamond->contexts, struct damon_ctx, list);
 }
 
+static inline bool damon_is_last_ctx(struct damon_ctx *ctx,
+				     struct kdamond_struct *kdamond)
+{
+	return list_is_last(&ctx->list, &kdamond->contexts);
+}
+
 #define damon_for_each_region(r, t) \
 	list_for_each_entry(r, &t->regions_list, list)
 
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 822e54f29..dc92a8703 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -500,6 +500,8 @@ struct damon_ctx *damon_new_ctx(void)
 	ctx->attrs.min_nr_regions = 10;
 	ctx->attrs.max_nr_regions = 1000;
 
+	ctx->valid = true;
+
 	INIT_LIST_HEAD(&ctx->adaptive_targets);
 	INIT_LIST_HEAD(&ctx->schemes);
 	INIT_LIST_HEAD(&ctx->list);
@@ -510,7 +512,6 @@ struct damon_ctx *damon_new_ctx(void)
 void damon_add_ctx(struct kdamond_struct *kdamond, struct damon_ctx *ctx)
 {
 	list_add_tail(&ctx->list, &kdamond->contexts);
-	++kdamond->nr_ctxs;
 }
 
 struct kdamond_struct *damon_new_kdamond(void)
@@ -730,6 +731,20 @@ bool damon_kdamond_running(struct kdamond_struct *kdamond)
 	return running;
 }
 
+/**
+ * kdamond_nr_ctxs() - Return number of contexts for this kdamond.
+ */
+static int kdamond_nr_ctxs(struct kdamond_struct *kdamond)
+{
+	struct list_head *pos;
+	int nr_ctxs = 0;
+
+	list_for_each(pos, &kdamond->contexts)
+		++nr_ctxs;
+
+	return nr_ctxs;
+}
+
 /* Returns the size upper limit for each monitoring region */
 static unsigned long damon_region_sz_limit(struct damon_ctx *ctx)
 {
@@ -784,16 +799,14 @@ static int __damon_start(struct kdamond_struct *kdamond)
 
 /**
  * damon_start() - Starts the monitorings for a given group of contexts.
- * @ctxs:	an array of the pointers for contexts to start monitoring
- * @nr_ctxs:	size of @ctxs
  * @exclusive:	exclusiveness of this contexts group
  *
  * This function starts a group of monitoring threads for a group of monitoring
- * contexts.  One thread per each context is created and run in parallel.  The
- * caller should handle synchronization between the threads by itself.  If
- * @exclusive is true and a group of threads that created by other
+ * contexts. If @exclusive is true and a group of contexts that created by other
  * 'damon_start()' call is currently running, this function does nothing but
- * returns -EBUSY.
+ * returns -EBUSY, if @exclusive is true and a given kdamond wants to run
+ * several contexts, then this function returns -EINVAL. kdamond can run
+ * exclusively only one context.
  *
  * Return: 0 on success, negative error code otherwise.
  */
@@ -802,10 +815,6 @@ int damon_start(struct kdamond_struct *kdamond, bool exclusive)
 	int err = 0;
 
 	BUG_ON(!kdamond);
-	BUG_ON(!kdamon->nr_ctxs);
-
-	if (kdamond->nr_ctxs != 1)
-		return -EINVAL;
 
 	mutex_lock(&damon_lock);
 	if ((exclusive && nr_running_kdamonds) ||
@@ -814,6 +823,11 @@ int damon_start(struct kdamond_struct *kdamond, bool exclusive)
 		return -EBUSY;
 	}
 
+	if (exclusive && kdamond_nr_ctxs(kdamond) > 1) {
+		mutex_unlock(&damon_lock);
+		return -EINVAL;
+	}
+
 	err = __damon_start(kdamond);
 	if (err)
 		return err;
@@ -1499,22 +1513,35 @@ static void kdamond_split_regions(struct damon_ctx *ctx)
  *
  * Returns true if need to stop current monitoring.
  */
-static bool kdamond_need_stop(struct damon_ctx *ctx)
+static bool kdamond_need_stop(void)
 {
-	struct damon_target *t;
-
 	if (kthread_should_stop())
 		return true;
+	return false;
+}
+
+static bool kdamond_valid_ctx(struct damon_ctx *ctx)
+{
+	struct damon_target *t;
 
 	if (!ctx->ops.target_valid)
-		return false;
+		return true;
 
 	damon_for_each_target(t, ctx) {
 		if (ctx->ops.target_valid(t))
-			return false;
+			return true;
 	}
 
-	return true;
+	return false;
+}
+
+static void kdamond_usleep(unsigned long usecs)
+{
+	/* See Documentation/timers/timers-howto.rst for the thresholds */
+	if (usecs > 20 * USEC_PER_MSEC)
+		schedule_timeout_idle(usecs_to_jiffies(usecs));
+	else
+		usleep_idle_range(usecs, usecs + 1);
 }
 
 static unsigned long damos_wmark_metric_value(enum damos_wmark_metric metric)
@@ -1563,41 +1590,22 @@ static unsigned long damos_wmark_wait_us(struct damos *scheme)
 	return 0;
 }
 
-static void kdamond_usleep(unsigned long usecs)
-{
-	/* See Documentation/timers/timers-howto.rst for the thresholds */
-	if (usecs > 20 * USEC_PER_MSEC)
-		schedule_timeout_idle(usecs_to_jiffies(usecs));
-	else
-		usleep_idle_range(usecs, usecs + 1);
-}
-
-/* Returns negative error code if it's not activated but should return */
-static int kdamond_wait_activation(struct damon_ctx *ctx)
+/* context must be valid */
+static unsigned long kdamond_wmark_wait_time(struct damon_ctx *ctx)
 {
 	struct damos *s;
 	unsigned long wait_time;
 	unsigned long min_wait_time = 0;
 	bool init_wait_time = false;
 
-	while (!kdamond_need_stop(ctx)) {
-		damon_for_each_scheme(s, ctx) {
-			wait_time = damos_wmark_wait_us(s);
-			if (!init_wait_time || wait_time < min_wait_time) {
-				init_wait_time = true;
-				min_wait_time = wait_time;
-			}
+	damon_for_each_scheme(s, ctx) {
+		wait_time = damos_wmark_wait_us(s);
+		if (!init_wait_time || wait_time < min_wait_time) {
+			init_wait_time = true;
+			min_wait_time = wait_time;
 		}
-		if (!min_wait_time)
-			return 0;
-
-		kdamond_usleep(min_wait_time);
-
-		if (ctx->callback.after_wmarks_check &&
-				ctx->callback.after_wmarks_check(ctx))
-			break;
 	}
-	return -EBUSY;
+	return min_wait_time;
 }
 
 static void kdamond_init_intervals_sis(struct damon_ctx *ctx)
@@ -1666,14 +1674,42 @@ static void kdamond_finish_ctxs(struct kdamond_struct *kdamond)
 		kdamond_finish_ctx(c);
 }
 
+static bool kdamond_prepare_access_checks_ctx(struct damon_ctx *ctx,
+					      unsigned long *sample_interval,
+					      unsigned long *min_wait_time)
+{
+	unsigned long wait_time = 0;
+
+	if (!ctx->valid || !kdamond_valid_ctx(ctx))
+		goto invalidate_ctx;
+
+	wait_time = kdamond_wmark_wait_time(ctx);
+	if (wait_time) {
+		if (!*min_wait_time || wait_time < *min_wait_time)
+			*min_wait_time = wait_time;
+		return false;
+	}
+
+	if (ctx->ops.prepare_access_checks)
+		ctx->ops.prepare_access_checks(ctx);
+	if (ctx->callback.after_sampling &&
+			ctx->callback.after_sampling(ctx))
+		goto invalidate_ctx;
+	*sample_interval = ctx->attrs.sample_interval;
+	ctx->prepared = true;
+	return true;
+invalidate_ctx:
+	ctx->valid = false;
+	return false;
+}
+
 /*
  * The monitoring daemon that runs as a kernel thread
  */
 static int kdamond_fn(void *data)
 {
+	struct damon_ctx *ctx;
 	struct kdamond_struct *kdamond = data;
-	struct damon_ctx *ctx = damon_first_ctx(kdamond);
-	unsigned int max_nr_accesses = 0;
 
 	pr_debug("kdamond (%d) starts\n", current->pid);
 
@@ -1681,69 +1717,84 @@ static int kdamond_fn(void *data)
 	if (!kdamond_init_ctxs(kdamond))
 		goto done;
 
-	while (!kdamond_need_stop(ctx)) {
-		/*
-		 * ctx->attrs and ctx->next_{aggregation,ops_update}_sis could
-		 * be changed from after_wmarks_check() or after_aggregation()
-		 * callbacks.  Read the values here, and use those for this
-		 * iteration.  That is, damon_set_attrs() updated new values
-		 * are respected from next iteration.
-		 */
-		unsigned long next_aggregation_sis = ctx->next_aggregation_sis;
-		unsigned long next_ops_update_sis = ctx->next_ops_update_sis;
-		unsigned long sample_interval = ctx->attrs.sample_interval;
-		unsigned long sz_limit = ctx->sz_limit;
-
-		if (kdamond_wait_activation(ctx))
-			break;
+	while (!kdamond_need_stop()) {
+		unsigned long nr_valid_ctxs = 0;
+		unsigned long min_wait_time = 0;
+		unsigned long sample_interval = 0;
 
-		if (ctx->ops.prepare_access_checks)
-			ctx->ops.prepare_access_checks(ctx);
-		if (ctx->callback.after_sampling &&
-				ctx->callback.after_sampling(ctx))
-			break;
+		damon_for_each_context(ctx, kdamond) {
+			if (kdamond_prepare_access_checks_ctx(ctx, &sample_interval,
+							      &min_wait_time))
+				nr_valid_ctxs++;
+		}
 
+		if (!nr_valid_ctxs) {
+			if (!min_wait_time)
+				break;
+			kdamond_usleep(min_wait_time);
+			continue;
+		}
 		kdamond_usleep(sample_interval);
-		ctx->passed_sample_intervals++;
 
-		if (ctx->ops.check_accesses)
-			max_nr_accesses = ctx->ops.check_accesses(ctx);
+		damon_for_each_context(ctx, kdamond) {
+			/*
+			 * ctx->attrs and ctx->next_{aggregation,ops_update}_sis could
+			 * be changed from after_wmarks_check() or after_aggregation()
+			 * callbacks.  Read the values here, and use those for this
+			 * iteration.  That is, damon_set_attrs() updated new values
+			 * are respected from next iteration.
+			 */
+			unsigned int max_nr_accesses = 0;
+			unsigned long next_aggregation_sis = ctx->next_aggregation_sis;
+			unsigned long next_ops_update_sis = ctx->next_ops_update_sis;
+			unsigned long sz_limit = ctx->sz_limit;
+			unsigned long sample_interval = ctx->attrs.sample_interval ?
+							ctx->attrs.sample_interval : 1;
+
+			if (!ctx->valid)
+				goto next_ctx;
+
+			ctx->passed_sample_intervals++;
+
+			if (ctx->ops.check_accesses)
+				max_nr_accesses = ctx->ops.check_accesses(ctx);
+
+			if (ctx->passed_sample_intervals == next_aggregation_sis) {
+				kdamond_merge_regions(ctx,
+						max_nr_accesses / 10,
+						sz_limit);
+				if (ctx->callback.after_aggregation &&
+						ctx->callback.after_aggregation(ctx))
+					goto next_ctx;
+			}
 
-		if (ctx->passed_sample_intervals == next_aggregation_sis) {
-			kdamond_merge_regions(ctx,
-					max_nr_accesses / 10,
-					sz_limit);
-			if (ctx->callback.after_aggregation &&
-					ctx->callback.after_aggregation(ctx))
-				break;
-		}
+			/*
+			 * do kdamond_apply_schemes() after kdamond_merge_regions() if
+			 * possible, to reduce overhead
+			 */
+			if (!list_empty(&ctx->schemes))
+				kdamond_apply_schemes(ctx);
 
-		/*
-		 * do kdamond_apply_schemes() after kdamond_merge_regions() if
-		 * possible, to reduce overhead
-		 */
-		if (!list_empty(&ctx->schemes))
-			kdamond_apply_schemes(ctx);
-
-		sample_interval = ctx->attrs.sample_interval ?
-			ctx->attrs.sample_interval : 1;
-		if (ctx->passed_sample_intervals == next_aggregation_sis) {
-			ctx->next_aggregation_sis = next_aggregation_sis +
-				ctx->attrs.aggr_interval / sample_interval;
-
-			kdamond_reset_aggregated(ctx);
-			kdamond_split_regions(ctx);
-			if (ctx->ops.reset_aggregated)
-				ctx->ops.reset_aggregated(ctx);
-		}
+			if (ctx->passed_sample_intervals == next_aggregation_sis) {
+				ctx->next_aggregation_sis = next_aggregation_sis +
+					ctx->attrs.aggr_interval / sample_interval;
+
+				kdamond_reset_aggregated(ctx, ctx_id);
+				kdamond_split_regions(ctx);
+				if (ctx->ops.reset_aggregated)
+					ctx->ops.reset_aggregated(ctx);
+			}
 
-		if (ctx->passed_sample_intervals == next_ops_update_sis) {
-			ctx->next_ops_update_sis = next_ops_update_sis +
-				ctx->attrs.ops_update_interval /
-				sample_interval;
-			if (ctx->ops.update)
-				ctx->ops.update(ctx);
-			ctx->sz_limit = damon_region_sz_limit(ctx);
+			if (ctx->passed_sample_intervals == next_ops_update_sis) {
+				ctx->next_ops_update_sis = next_ops_update_sis +
+					ctx->attrs.ops_update_interval /
+					sample_interval;
+				if (ctx->ops.update)
+					ctx->ops.update(ctx);
+				ctx->sz_limit = damon_region_sz_limit(ctx);
+			}
+next_ctx:
+			++ctx_id;
 		}
 	}
 done:
diff --git a/mm/damon/modules-common.c b/mm/damon/modules-common.c
index a70b4fc7a..0fa53bc43 100644
--- a/mm/damon/modules-common.c
+++ b/mm/damon/modules-common.c
@@ -53,7 +53,6 @@ int damon_modules_new_paddr_kdamond(struct kdamond_struct **kdamondp)
 		damon_destroy_kdamond(kdamond);
 		return err;
 	}
-	kdamond->nr_ctxs = 1;
 
 	*kdamondp = kdamond;
 	return 0;
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index db2d48361..b473a1e6d 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -5,6 +5,8 @@
  * Copyright (c) 2022 SeongJae Park <sj@kernel.org>
  */
 
+#define pr_fmt(fmt) "damon-sysfs: " fmt
+
 #include <linux/pid.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
@@ -897,8 +899,7 @@ static ssize_t nr_contexts_store(struct kobject *kobj,
 	err = kstrtoint(buf, 0, &nr);
 	if (err)
 		return err;
-	/* TODO: support multiple contexts per kdamond */
-	if (nr < 0 || 1 < nr)
+	if (nr < 0)
 		return -EINVAL;
 
 	contexts = container_of(kobj, struct damon_sysfs_contexts, kobj);
@@ -1381,23 +1382,48 @@ static int damon_sysfs_apply_inputs(struct damon_ctx *ctx,
  */
 static int damon_sysfs_commit_input(struct damon_sysfs_kdamond *sys_kdamond)
 {
+	unsigned long ctx_id = 0;
 	struct damon_ctx *c;
-	struct damon_sysfs_context *sysfs_ctx;
+	struct damon_sysfs_context **sysfs_ctxs;
 	int err;
 
 	if (!damon_sysfs_kdamond_running(sys_kdamond))
 		return -EINVAL;
-	/* TODO: Support multiple contexts per kdamond */
-	if (sys_kdamond->contexts->nr != 1)
-		return -EINVAL;
 
-	sysfs_ctx = sys_kdamond->contexts->contexts_arr[0];
+	sysfs_ctxs = sys_kdamond->contexts->contexts_arr;
 	damon_for_each_context(c, sys_kdamond->kdamond) {
+		struct damon_sysfs_context *sysfs_ctx = *sysfs_ctxs;
+		struct damon_sysfs_intervals *sys_intervals =
+			sysfs_ctx->attrs->intervals;;
+
+		if (sys_kdamond->contexts->nr > 1 &&
+				sys_intervals->sample_us != c->attrs.sample_interval) {
+			pr_err("context_id=%lu: "
+				"multiple contexts must have equal sample_interval\n",
+					ctx_id);
+			/*
+			 * since multiple contexts expect equal
+			 * sample_intervals, try to fix it here
+			 */
+			sys_intervals->sample_us = c->attrs.sample_interval;
+		}
+
 		err = damon_sysfs_apply_inputs(c, sysfs_ctx);
 		if (err)
 			return err;
-		++sysfs_ctx;
+		++sysfs_ctxs;
+
+		/* sysfs_ctx may be NIL, so check if it is the last */
+		if (sys_kdamond->contexts->nr > 1 && sysfs_ctxs &&
+				!damon_is_last_ctx(c, sys_kdamond->kdamond)) {
+			sysfs_ctx = *sysfs_ctxs;
+			sys_intervals = sysfs_ctx->attrs->intervals;
+			/* We somehow failed in fixing sample_interval above */
+			BUG_ON(sys_intervals->sample_us != c->attrs.sample_interval);
+		}
+		++ctx_id;
 	}
+
 	return 0;
 }
 
@@ -1410,9 +1436,6 @@ static int damon_sysfs_commit_schemes_quota_goals(
 
 	if (!damon_sysfs_kdamond_running(sysfs_kdamond))
 		return -EINVAL;
-	/* TODO: Support multiple contexts per kdamond */
-	if (sysfs_kdamond->contexts->nr != 1)
-		return -EINVAL;
 
 	sysfs_ctxs = sysfs_kdamond->contexts->contexts_arr;
 	damon_for_each_context(c, sysfs_kdamond->kdamond) {
@@ -1613,9 +1636,6 @@ static int damon_sysfs_turn_damon_on(struct damon_sysfs_kdamond *sys_kdamond)
 		return -EBUSY;
 	if (damon_sysfs_cmd_request.kdamond == sys_kdamond)
 		return -EBUSY;
-	/* TODO: support multiple contexts per kdamond */
-	if (sys_kdamond->contexts->nr != 1)
-		return -EINVAL;
 
 	if (sys_kdamond->kdamond)
 		damon_destroy_kdamond(sys_kdamond->kdamond);
-- 
2.42.0


  parent reply	other threads:[~2024-05-15 15:25 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-15 15:24 [RFC PATCH v1 0/7] DAMON multiple contexts support Alex Rusuf
2024-05-15 15:24 ` [RFC PATCH v1 1/7] mm/damon/core: kdamond_struct abstraction layer Alex Rusuf
2024-05-15 15:24 ` [RFC PATCH v1 2/7] mm/damon/core: list-based contexts organization Alex Rusuf
2024-05-15 15:24 ` [RFC PATCH v1 3/7] mm/damon/lru_sort: kdamond_struct abstraction layer Alex Rusuf
2024-05-15 15:24 ` [RFC PATCH v1 4/7] mm/damon/reclaim: kdamon_struct " Alex Rusuf
2024-05-15 15:24 ` [RFC PATCH v1 5/7] mm/damon/core: rename nr_running_ctxs -> nr_running_kdamonds Alex Rusuf
2024-05-15 15:24 ` Alex Rusuf [this message]
2024-05-15 15:24 ` [RFC PATCH v1 7/7] mm/damon/core: multi-context awarness for trace events Alex Rusuf
2024-05-16 22:17 ` [RFC PATCH v1 0/7] DAMON multiple contexts support SeongJae Park
2024-05-17  8:51   ` Alex Rusuf
2024-05-17 22:59     ` SeongJae Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240515152457.603724-7-yorha.op@gmail.com \
    --to=yorha.op@gmail.com \
    --cc=damon@lists.linux.dev \
    --cc=sj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).