* [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function
@ 2024-06-18 18:17 SeongJae Park
2024-06-18 18:17 ` [PATCH 01/12] mm/damon/core: implement DAMOS quota goals " SeongJae Park
` (11 more replies)
0 siblings, 12 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:17 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-kernel, linux-mm
DAMON context struct (damon_ctx) contains user requests (parameters),
internal status, and operation results. For flexible usages, DAMON API
users are encouraged to manually manipulate the struct. That works well
for simple use cases. However, it has turned out that it is not that
simple at least for online parameters udpate. It is easy to forget
properly maintaining internal status and operation results. Also, such
manual manipulation for online tuning is implemented multiple times on
DAMON API users including DAMON sysfs interface, DAMON_RECLAIM and
DAMON_LRU_SORT. As a result, we have multiple sources of bugs for same
problem. Actually we found and fixed a few bugs from online parameter
updating of DAMON API users.
Implement a function for online DAMON parameters update in core layer,
and replace DAMON API users' manual manipulation code for the use case.
The core layer function could still have bugs, but this change reduces
the source of bugs for the problem to one place.
SeongJae Park (12):
mm/damon/core: implement DAMOS quota goals online commit function
mm/damon/core: implement DAMON context commit function
mm/damon/sysfs: use damon_commit_ctx()
mm/damon/sysfs-schemes: use damos_commit_quota_goals()
mm/damon/sysfs: remove unnecessary online tuning handling code
mm/damon/sysfs: rename damon_sysfs_set_targets() to ...add_targets()
mm/damon/sysfs-schemes: remove unnecessary online tuning handling code
mm/damon/sysfs-schemes: rename
*_set_{schemes,scheme_filters,quota_score,schemes}()
mm/damon/reclaim: use damon_commit_ctx()
mm/damon/reclaim: remove unnecessary code for online tuning
mm/damon/lru_sort: use damon_commit_ctx()
mm/damon/lru_sort: remove unnecessary online tuning handling code
include/linux/damon.h | 2 +
mm/damon/core.c | 333 +++++++++++++++++++++++++++++++++++++++
mm/damon/lru_sort.c | 53 +++----
mm/damon/reclaim.c | 62 +++-----
mm/damon/sysfs-common.h | 2 +-
mm/damon/sysfs-schemes.c | 94 +++--------
mm/damon/sysfs-test.h | 10 +-
mm/damon/sysfs.c | 81 +++-------
8 files changed, 425 insertions(+), 212 deletions(-)
base-commit: a7b6f23b7fa3f5d1f3ae64034a4aff12fb8c1df0
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 01/12] mm/damon/core: implement DAMOS quota goals online commit function
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
@ 2024-06-18 18:17 ` SeongJae Park
2024-06-18 18:17 ` [PATCH 02/12] mm/damon/core: implement DAMON context " SeongJae Park
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:17 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-mm, linux-kernel
Implement functions for supporting online DAMOS quota goals parameters
update. The function receives two DAMOS quota structs. One is the
struct that currently being used by a kdamond and therefore to be
updated. The other one contains the parameters to be applied to the
first one. The function applies the new parameters to the destination
struct while keeping/updating the internal status. The function should
be called from parameters-update safe place, like DAMON callbacks.
Signed-off-by: SeongJae Park <sj@kernel.org>
---
include/linux/damon.h | 1 +
mm/damon/core.c | 59 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 3d62d98d6359..ce12c9f1b4e4 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -742,6 +742,7 @@ struct damos *damon_new_scheme(struct damos_access_pattern *pattern,
int target_nid);
void damon_add_scheme(struct damon_ctx *ctx, struct damos *s);
void damon_destroy_scheme(struct damos *s);
+int damos_commit_quota_goals(struct damos_quota *dst, struct damos_quota *src);
struct damon_target *damon_new_target(void);
void damon_add_target(struct damon_ctx *ctx, struct damon_target *t);
diff --git a/mm/damon/core.c b/mm/damon/core.c
index c0ec5be4f56e..b538a31fbd83 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -666,6 +666,65 @@ void damon_set_schemes(struct damon_ctx *ctx, struct damos **schemes,
damon_add_scheme(ctx, schemes[i]);
}
+static struct damos_quota_goal *damos_nth_quota_goal(
+ int n, struct damos_quota *q)
+{
+ struct damos_quota_goal *goal;
+ int i = 0;
+
+ damos_for_each_quota_goal(goal, q) {
+ if (i++ == n)
+ return goal;
+ }
+ return NULL;
+}
+
+static void damos_commit_quota_goal(
+ struct damos_quota_goal *dst, struct damos_quota_goal *src)
+{
+ dst->metric = src->metric;
+ dst->target_value = src->target_value;
+ if (dst->metric == DAMOS_QUOTA_USER_INPUT)
+ dst->current_value = src->current_value;
+ /* keep last_psi_total as is, since it will be updated in next cycle */
+}
+
+/**
+ * damos_commit_quota_goals() - Commit DAMOS quota goals to another quota.
+ * @dst: The commit destination DAMOS quota.
+ * @src: The commit source DAMOS quota.
+ *
+ * Copies user-specified parameters for quota goals from @src to @dst. Users
+ * should use this function for quota goals-level parameters update of running
+ * DAMON contexts, instead of manual in-place updates.
+ *
+ * This function should be called from parameters-update safe context, like
+ * DAMON callbacks.
+ */
+int damos_commit_quota_goals(struct damos_quota *dst, struct damos_quota *src)
+{
+ struct damos_quota_goal *dst_goal, *next, *src_goal, *new_goal;
+ int i = 0, j = 0;
+
+ damos_for_each_quota_goal_safe(dst_goal, next, dst) {
+ src_goal = damos_nth_quota_goal(i++, src);
+ if (src_goal)
+ damos_commit_quota_goal(dst_goal, src_goal);
+ else
+ damos_destroy_quota_goal(dst_goal);
+ }
+ damos_for_each_quota_goal_safe(src_goal, next, src) {
+ if (j++ < i)
+ continue;
+ new_goal = damos_new_quota_goal(
+ src_goal->metric, src_goal->target_value);
+ if (!new_goal)
+ return -ENOMEM;
+ damos_add_quota_goal(dst, new_goal);
+ }
+ return 0;
+}
+
/**
* damon_nr_running_ctxs() - Return number of currently running contexts.
*/
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 02/12] mm/damon/core: implement DAMON context commit function
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
2024-06-18 18:17 ` [PATCH 01/12] mm/damon/core: implement DAMOS quota goals " SeongJae Park
@ 2024-06-18 18:17 ` SeongJae Park
2024-06-18 18:18 ` [PATCH 03/12] mm/damon/sysfs: use damon_commit_ctx() SeongJae Park
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:17 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-mm, linux-kernel
Implement functions for supporting online DAMON context level parameters
update. The function receives two DAMON context structs. One is the
struct that currently being used by a kdamond and therefore to be
updated. The other one contains the parameters to be applied to the
first one. The function applies the new parameters to the destination
struct while keeping/updating the internal status and operation results.
The function should be called from DAMON context-update-safe place, like
DAMON callbacks.
Signed-off-by: SeongJae Park <sj@kernel.org>
---
include/linux/damon.h | 1 +
mm/damon/core.c | 274 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 275 insertions(+)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index ce12c9f1b4e4..27c546bfc6d4 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -756,6 +756,7 @@ void damon_destroy_ctx(struct damon_ctx *ctx);
int damon_set_attrs(struct damon_ctx *ctx, struct damon_attrs *attrs);
void damon_set_schemes(struct damon_ctx *ctx,
struct damos **schemes, ssize_t nr_schemes);
+int damon_commit_ctx(struct damon_ctx *old_ctx, struct damon_ctx *new_ctx);
int damon_nr_running_ctxs(void);
bool damon_is_registered_ops(enum damon_ops_id id);
int damon_register_ops(struct damon_operations *ops);
diff --git a/mm/damon/core.c b/mm/damon/core.c
index b538a31fbd83..f69250b68bcc 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -725,6 +725,280 @@ int damos_commit_quota_goals(struct damos_quota *dst, struct damos_quota *src)
return 0;
}
+static int damos_commit_quota(struct damos_quota *dst, struct damos_quota *src)
+{
+ int err;
+
+ dst->reset_interval = src->reset_interval;
+ dst->ms = src->ms;
+ dst->sz = src->sz;
+ err = damos_commit_quota_goals(dst, src);
+ if (err)
+ return err;
+ dst->weight_sz = src->weight_sz;
+ dst->weight_nr_accesses = src->weight_nr_accesses;
+ dst->weight_age = src->weight_age;
+ return 0;
+}
+
+static struct damos_filter *damos_nth_filter(int n, struct damos *s)
+{
+ struct damos_filter *filter;
+ int i = 0;
+
+ damos_for_each_filter(filter, s) {
+ if (i++ == n)
+ return filter;
+ }
+ return NULL;
+}
+
+static void damos_commit_filter_arg(
+ struct damos_filter *dst, struct damos_filter *src)
+{
+ switch (dst->type) {
+ case DAMOS_FILTER_TYPE_MEMCG:
+ dst->memcg_id = src->memcg_id;
+ break;
+ case DAMOS_FILTER_TYPE_ADDR:
+ dst->addr_range = src->addr_range;
+ break;
+ case DAMOS_FILTER_TYPE_TARGET:
+ dst->target_idx = src->target_idx;
+ break;
+ default:
+ break;
+ }
+}
+
+static void damos_commit_filter(
+ struct damos_filter *dst, struct damos_filter *src)
+{
+ dst->type = src->type;
+ dst->matching = src->matching;
+ damos_commit_filter_arg(dst, src);
+}
+
+static int damos_commit_filters(struct damos *dst, struct damos *src)
+{
+ struct damos_filter *dst_filter, *next, *src_filter, *new_filter;
+ int i = 0, j = 0;
+
+ damos_for_each_filter_safe(dst_filter, next, dst) {
+ src_filter = damos_nth_filter(i++, src);
+ if (src_filter)
+ damos_commit_filter(dst_filter, src_filter);
+ else
+ damos_destroy_filter(dst_filter);
+ }
+
+ damos_for_each_filter_safe(src_filter, next, src) {
+ if (j++ < i)
+ continue;
+
+ new_filter = damos_new_filter(
+ src_filter->type, src_filter->matching);
+ if (!new_filter)
+ return -ENOMEM;
+ damos_commit_filter_arg(new_filter, src_filter);
+ damos_add_filter(dst, new_filter);
+ }
+ return 0;
+}
+
+static struct damos *damon_nth_scheme(int n, struct damon_ctx *ctx)
+{
+ struct damos *s;
+ int i = 0;
+
+ damon_for_each_scheme(s, ctx) {
+ if (i++ == n)
+ return s;
+ }
+ return NULL;
+}
+
+static int damos_commit(struct damos *dst, struct damos *src)
+{
+ int err;
+
+ dst->pattern = src->pattern;
+ dst->action = src->action;
+ dst->apply_interval_us = src->apply_interval_us;
+
+ err = damos_commit_quota(&dst->quota, &src->quota);
+ if (err)
+ return err;
+
+ dst->wmarks = src->wmarks;
+
+ err = damos_commit_filters(dst, src);
+ return err;
+}
+
+static int damon_commit_schemes(struct damon_ctx *dst, struct damon_ctx *src)
+{
+ struct damos *dst_scheme, *next, *src_scheme, *new_scheme;
+ int i = 0, j = 0, err;
+
+ damon_for_each_scheme_safe(dst_scheme, next, dst) {
+ src_scheme = damon_nth_scheme(i++, src);
+ if (src_scheme) {
+ err = damos_commit(dst_scheme, src_scheme);
+ if (err)
+ return err;
+ } else {
+ damon_destroy_scheme(dst_scheme);
+ }
+ }
+
+ damon_for_each_scheme_safe(src_scheme, next, src) {
+ if (j++ < i)
+ continue;
+ new_scheme = damon_new_scheme(&src_scheme->pattern,
+ src_scheme->action,
+ src_scheme->apply_interval_us,
+ &src_scheme->quota, &src_scheme->wmarks,
+ NUMA_NO_NODE);
+ if (!new_scheme)
+ return -ENOMEM;
+ damon_add_scheme(dst, new_scheme);
+ }
+ return 0;
+}
+
+static struct damon_target *damon_nth_target(int n, struct damon_ctx *ctx)
+{
+ struct damon_target *t;
+ int i = 0;
+
+ damon_for_each_target(t, ctx) {
+ if (i++ == n)
+ return t;
+ }
+ return NULL;
+}
+
+/*
+ * The caller should ensure the regions of @src are
+ * 1. valid (end >= src) and
+ * 2. sorted by starting address.
+ *
+ * If @src has no region, @dst keeps current regions.
+ */
+static int damon_commit_target_regions(
+ struct damon_target *dst, struct damon_target *src)
+{
+ struct damon_region *src_region;
+ struct damon_addr_range *ranges;
+ int i = 0, err;
+
+ damon_for_each_region(src_region, src)
+ i++;
+ if (!i)
+ return 0;
+
+ ranges = kmalloc_array(i, sizeof(*ranges), GFP_KERNEL | __GFP_NOWARN);
+ if (!ranges)
+ return -ENOMEM;
+ i = 0;
+ damon_for_each_region(src_region, src)
+ ranges[i++] = src_region->ar;
+ err = damon_set_regions(dst, ranges, i);
+ kfree(ranges);
+ return err;
+}
+
+static int damon_commit_target(
+ struct damon_target *dst, bool dst_has_pid,
+ struct damon_target *src, bool src_has_pid)
+{
+ int err;
+
+ err = damon_commit_target_regions(dst, src);
+ if (err)
+ return err;
+ if (dst_has_pid)
+ put_pid(dst->pid);
+ if (src_has_pid)
+ get_pid(src->pid);
+ dst->pid = src->pid;
+ return 0;
+}
+
+static int damon_commit_targets(
+ struct damon_ctx *dst, struct damon_ctx *src)
+{
+ struct damon_target *dst_target, *next, *src_target, *new_target;
+ int i = 0, j = 0, err;
+
+ damon_for_each_target_safe(dst_target, next, dst) {
+ src_target = damon_nth_target(i++, src);
+ if (src_target) {
+ err = damon_commit_target(
+ dst_target, damon_target_has_pid(dst),
+ src_target, damon_target_has_pid(src));
+ if (err)
+ return err;
+ } else {
+ if (damon_target_has_pid(dst))
+ put_pid(dst_target->pid);
+ damon_destroy_target(dst_target);
+ }
+ }
+
+ damon_for_each_target_safe(src_target, next, src) {
+ if (j++ < i)
+ continue;
+ new_target = damon_new_target();
+ if (!new_target)
+ return -ENOMEM;
+ err = damon_commit_target(new_target, false,
+ src_target, damon_target_has_pid(src));
+ if (err)
+ return err;
+ }
+ return 0;
+}
+
+/**
+ * damon_commit_ctx() - Commit parameters of a DAMON context to another.
+ * @dst: The commit destination DAMON context.
+ * @src: The commit source DAMON context.
+ *
+ * This function copies user-specified parameters from @src to @dst and update
+ * the internal status and results accordingly. Users should use this function
+ * for context-level parameters update of running context, instead of manual
+ * in-place updates.
+ *
+ * This function should be called from parameters-update safe context, like
+ * DAMON callbacks.
+ */
+int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
+{
+ int err;
+
+ err = damon_commit_schemes(dst, src);
+ if (err)
+ return err;
+ err = damon_commit_targets(dst, src);
+ if (err)
+ return err;
+ /*
+ * schemes and targets should be updated first, since
+ * 1. damon_set_attrs() updates monitoring results of targets and
+ * next_apply_sis of schemes, and
+ * 2. ops update should be done after pid handling is done (target
+ * committing require putting pids).
+ */
+ err = damon_set_attrs(dst, &src->attrs);
+ if (err)
+ return err;
+ dst->ops = src->ops;
+
+ return 0;
+}
+
/**
* damon_nr_running_ctxs() - Return number of currently running contexts.
*/
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 03/12] mm/damon/sysfs: use damon_commit_ctx()
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
2024-06-18 18:17 ` [PATCH 01/12] mm/damon/core: implement DAMOS quota goals " SeongJae Park
2024-06-18 18:17 ` [PATCH 02/12] mm/damon/core: implement DAMON context " SeongJae Park
@ 2024-06-18 18:18 ` SeongJae Park
2024-06-18 18:18 ` [PATCH 04/12] mm/damon/sysfs-schemes: use damos_commit_quota_goals() SeongJae Park
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-mm, linux-kernel
DAMON_SYSFS manually manipulates DAMON context structs for online
parameters update. Since the struct contains not only input parameters
but also internal status and operation results, it is not that simple.
Indeed, we found and fixed a few bugs in the code. Now DAMON core layer
provides a function for the usage, namely damon_commit_ctx(). Replace
the manual manipulation logic with the function. The core layer
function could have its own bugs, but this change removes a source of
bugs.
Signed-off-by: SeongJae Park <sj@kernel.org>
---
mm/damon/sysfs.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 6fee383bc0c5..0f9fe18beb40 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1345,6 +1345,9 @@ static int damon_sysfs_apply_inputs(struct damon_ctx *ctx,
return damon_sysfs_set_schemes(ctx, sys_ctx->schemes);
}
+static struct damon_ctx *damon_sysfs_build_ctx(
+ struct damon_sysfs_context *sys_ctx);
+
/*
* damon_sysfs_commit_input() - Commit user inputs to a running kdamond.
* @kdamond: The kobject wrapper for the associated kdamond.
@@ -1353,14 +1356,22 @@ static int damon_sysfs_apply_inputs(struct damon_ctx *ctx,
*/
static int damon_sysfs_commit_input(struct damon_sysfs_kdamond *kdamond)
{
+ struct damon_ctx *param_ctx;
+ int err;
+
if (!damon_sysfs_kdamond_running(kdamond))
return -EINVAL;
/* TODO: Support multiple contexts per kdamond */
if (kdamond->contexts->nr != 1)
return -EINVAL;
- return damon_sysfs_apply_inputs(kdamond->damon_ctx,
- kdamond->contexts->contexts_arr[0]);
+ param_ctx = damon_sysfs_build_ctx(kdamond->contexts->contexts_arr[0]);
+ if (IS_ERR(param_ctx))
+ return PTR_ERR(param_ctx);
+ err = damon_commit_ctx(kdamond->damon_ctx, param_ctx);
+ damon_sysfs_destroy_targets(param_ctx);
+ damon_destroy_ctx(param_ctx);
+ return err;
}
static int damon_sysfs_commit_schemes_quota_goals(
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 04/12] mm/damon/sysfs-schemes: use damos_commit_quota_goals()
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
` (2 preceding siblings ...)
2024-06-18 18:18 ` [PATCH 03/12] mm/damon/sysfs: use damon_commit_ctx() SeongJae Park
@ 2024-06-18 18:18 ` SeongJae Park
2024-06-18 18:18 ` [PATCH 05/12] mm/damon/sysfs: remove unnecessary online tuning handling code SeongJae Park
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-mm, linux-kernel
DAMON_SYSFS manually manipulates the DAMOS quota structs for online
quotal goals parameter update. Since the struct contains not only input
parameters but also internal status and operation results, it is not
that simple. Now DAMON core layer provides a function for the usage,
namely damon_commit_quota_goals(). Replace the manual manipulation
logic with the function. The core layer function could have its own
bugs, but this change removes a source of bugs.
Signed-off-by: SeongJae Park <sj@kernel.org>
---
mm/damon/sysfs-schemes.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 66fccfa776d7..1bccf2619e11 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -1983,10 +1983,13 @@ int damos_sysfs_set_quota_scores(struct damon_sysfs_schemes *sysfs_schemes,
struct damon_ctx *ctx)
{
struct damos *scheme;
+ struct damos_quota quota = {};
int i = 0;
+ INIT_LIST_HEAD("a.goals);
damon_for_each_scheme(scheme, ctx) {
struct damon_sysfs_scheme *sysfs_scheme;
+ struct damos_quota_goal *g, *g_next;
int err;
/* user could have removed the scheme sysfs dir */
@@ -1995,9 +1998,16 @@ int damos_sysfs_set_quota_scores(struct damon_sysfs_schemes *sysfs_schemes,
sysfs_scheme = sysfs_schemes->schemes_arr[i];
err = damos_sysfs_set_quota_score(sysfs_scheme->quotas->goals,
- &scheme->quota);
+ "a);
+ if (err) {
+ damos_for_each_quota_goal_safe(g, g_next, "a)
+ damos_destroy_quota_goal(g);
+ return err;
+ }
+ err = damos_commit_quota_goals(&scheme->quota, "a);
+ damos_for_each_quota_goal_safe(g, g_next, "a)
+ damos_destroy_quota_goal(g);
if (err)
- /* kdamond will clean up schemes and terminated */
return err;
i++;
}
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 05/12] mm/damon/sysfs: remove unnecessary online tuning handling code
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
` (3 preceding siblings ...)
2024-06-18 18:18 ` [PATCH 04/12] mm/damon/sysfs-schemes: use damos_commit_quota_goals() SeongJae Park
@ 2024-06-18 18:18 ` SeongJae Park
2024-06-18 18:18 ` [PATCH 06/12] mm/damon/sysfs: rename damon_sysfs_set_targets() to ...add_targets() SeongJae Park
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-mm, linux-kernel
damon/sysfs.c contains code for handling of online DAMON parameters
update edge cases. It is no more necessary since damon_commit_ctx()
takes care of the cases. Remove the unnecessary code.
Signed-off-by: SeongJae Park <sj@kernel.org>
---
mm/damon/sysfs-test.h | 2 +-
mm/damon/sysfs.c | 60 ++-----------------------------------------
2 files changed, 3 insertions(+), 59 deletions(-)
diff --git a/mm/damon/sysfs-test.h b/mm/damon/sysfs-test.h
index 73bdce2452c1..43a15156a7c3 100644
--- a/mm/damon/sysfs-test.h
+++ b/mm/damon/sysfs-test.h
@@ -62,7 +62,7 @@ static void damon_sysfs_test_set_targets(struct kunit *test)
sysfs_target->pid = __damon_sysfs_test_get_any_pid(
sysfs_target->pid + 1, 200);
damon_sysfs_set_targets(ctx, sysfs_targets);
- KUNIT_EXPECT_EQ(test, 1u, nr_damon_targets(ctx));
+ KUNIT_EXPECT_EQ(test, 2u, nr_damon_targets(ctx));
damon_destroy_ctx(ctx);
kfree(sysfs_targets->targets_arr);
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 0f9fe18beb40..c729222797b8 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1162,72 +1162,16 @@ static int damon_sysfs_add_target(struct damon_sysfs_target *sys_target,
return err;
}
-static int damon_sysfs_update_target_pid(struct damon_target *target, int pid)
-{
- struct pid *pid_new;
-
- pid_new = find_get_pid(pid);
- if (!pid_new)
- return -EINVAL;
-
- if (pid_new == target->pid) {
- put_pid(pid_new);
- return 0;
- }
-
- put_pid(target->pid);
- target->pid = pid_new;
- return 0;
-}
-
-static int damon_sysfs_update_target(struct damon_target *target,
- struct damon_ctx *ctx,
- struct damon_sysfs_target *sys_target)
-{
- int err = 0;
-
- if (damon_target_has_pid(ctx)) {
- err = damon_sysfs_update_target_pid(target, sys_target->pid);
- if (err)
- return err;
- }
-
- /*
- * Do monitoring target region boundary update only if one or more
- * regions are set by the user. This is for keeping current monitoring
- * target results and range easier, especially for dynamic monitoring
- * target regions update ops like 'vaddr'.
- */
- if (sys_target->regions->nr)
- err = damon_sysfs_set_regions(target, sys_target->regions);
- return err;
-}
-
static int damon_sysfs_set_targets(struct damon_ctx *ctx,
struct damon_sysfs_targets *sysfs_targets)
{
- struct damon_target *t, *next;
- int i = 0, err;
+ int i, err;
/* Multiple physical address space monitoring targets makes no sense */
if (ctx->ops.id == DAMON_OPS_PADDR && sysfs_targets->nr > 1)
return -EINVAL;
- damon_for_each_target_safe(t, next, ctx) {
- if (i < sysfs_targets->nr) {
- err = damon_sysfs_update_target(t, ctx,
- sysfs_targets->targets_arr[i]);
- if (err)
- return err;
- } else {
- if (damon_target_has_pid(ctx))
- put_pid(t->pid);
- damon_destroy_target(t);
- }
- i++;
- }
-
- for (; i < sysfs_targets->nr; i++) {
+ for (i = 0; i < sysfs_targets->nr; i++) {
struct damon_sysfs_target *st = sysfs_targets->targets_arr[i];
err = damon_sysfs_add_target(st, ctx);
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 06/12] mm/damon/sysfs: rename damon_sysfs_set_targets() to ...add_targets()
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
` (4 preceding siblings ...)
2024-06-18 18:18 ` [PATCH 05/12] mm/damon/sysfs: remove unnecessary online tuning handling code SeongJae Park
@ 2024-06-18 18:18 ` SeongJae Park
2024-06-18 18:18 ` [PATCH 07/12] mm/damon/sysfs-schemes: remove unnecessary online tuning handling code SeongJae Park
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-mm, linux-kernel
The function was for updating DAMON structs that may or may not be
partially populated. Hence it was not for only adding items, but also
removing unnecessary items and updating items in-place. A previous
commit has changed the function to assume the structs are not partially
populated, and do only adding items. Make the function name better
explain the behavior.
Signed-off-by: SeongJae Park <sj@kernel.org>
---
mm/damon/sysfs-test.h | 8 ++++----
mm/damon/sysfs.c | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/mm/damon/sysfs-test.h b/mm/damon/sysfs-test.h
index 43a15156a7c3..1c9b596057a7 100644
--- a/mm/damon/sysfs-test.h
+++ b/mm/damon/sysfs-test.h
@@ -38,7 +38,7 @@ static int __damon_sysfs_test_get_any_pid(int min, int max)
return -1;
}
-static void damon_sysfs_test_set_targets(struct kunit *test)
+static void damon_sysfs_test_add_targets(struct kunit *test)
{
struct damon_sysfs_targets *sysfs_targets;
struct damon_sysfs_target *sysfs_target;
@@ -56,12 +56,12 @@ static void damon_sysfs_test_set_targets(struct kunit *test)
ctx = damon_new_ctx();
- damon_sysfs_set_targets(ctx, sysfs_targets);
+ damon_sysfs_add_targets(ctx, sysfs_targets);
KUNIT_EXPECT_EQ(test, 1u, nr_damon_targets(ctx));
sysfs_target->pid = __damon_sysfs_test_get_any_pid(
sysfs_target->pid + 1, 200);
- damon_sysfs_set_targets(ctx, sysfs_targets);
+ damon_sysfs_add_targets(ctx, sysfs_targets);
KUNIT_EXPECT_EQ(test, 2u, nr_damon_targets(ctx));
damon_destroy_ctx(ctx);
@@ -71,7 +71,7 @@ static void damon_sysfs_test_set_targets(struct kunit *test)
}
static struct kunit_case damon_sysfs_test_cases[] = {
- KUNIT_CASE(damon_sysfs_test_set_targets),
+ KUNIT_CASE(damon_sysfs_test_add_targets),
{},
};
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index c729222797b8..f83ea6a166c6 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1162,7 +1162,7 @@ static int damon_sysfs_add_target(struct damon_sysfs_target *sys_target,
return err;
}
-static int damon_sysfs_set_targets(struct damon_ctx *ctx,
+static int damon_sysfs_add_targets(struct damon_ctx *ctx,
struct damon_sysfs_targets *sysfs_targets)
{
int i, err;
@@ -1283,7 +1283,7 @@ static int damon_sysfs_apply_inputs(struct damon_ctx *ctx,
err = damon_sysfs_set_attrs(ctx, sys_ctx->attrs);
if (err)
return err;
- err = damon_sysfs_set_targets(ctx, sys_ctx->targets);
+ err = damon_sysfs_add_targets(ctx, sys_ctx->targets);
if (err)
return err;
return damon_sysfs_set_schemes(ctx, sys_ctx->schemes);
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 07/12] mm/damon/sysfs-schemes: remove unnecessary online tuning handling code
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
` (5 preceding siblings ...)
2024-06-18 18:18 ` [PATCH 06/12] mm/damon/sysfs: rename damon_sysfs_set_targets() to ...add_targets() SeongJae Park
@ 2024-06-18 18:18 ` SeongJae Park
2024-06-18 18:18 ` [PATCH 08/12] mm/damon/sysfs-schemes: rename *_set_{schemes,scheme_filters,quota_score,schemes}() SeongJae Park
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-mm, linux-kernel
damon/sysfs-schemes.c contains code for handling of online DAMON
parameters update edge cases. The logics are no more necessary since
damon_commit_ctx() and damon_commit_quota_goals() takes care of the
cases. Remove the unnecessary code.
Signed-off-by: SeongJae Park <sj@kernel.org>
---
mm/damon/sysfs-schemes.c | 68 ++--------------------------------------
1 file changed, 3 insertions(+), 65 deletions(-)
diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 1bccf2619e11..77c0265dff5c 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -1912,10 +1912,6 @@ static int damon_sysfs_set_scheme_filters(struct damos *scheme,
struct damon_sysfs_scheme_filters *sysfs_filters)
{
int i;
- struct damos_filter *filter, *next;
-
- damos_for_each_filter_safe(filter, next, scheme)
- damos_destroy_filter(filter);
for (i = 0; i < sysfs_filters->nr; i++) {
struct damon_sysfs_scheme_filter *sysfs_filter =
@@ -1955,12 +1951,9 @@ static int damos_sysfs_set_quota_score(
struct damos_sysfs_quota_goals *sysfs_goals,
struct damos_quota *quota)
{
- struct damos_quota_goal *goal, *next;
+ struct damos_quota_goal *goal;
int i;
- damos_for_each_quota_goal_safe(goal, next, quota)
- damos_destroy_quota_goal(goal);
-
for (i = 0; i < sysfs_goals->nr; i++) {
struct damos_sysfs_quota_goal *sysfs_goal =
sysfs_goals->goals_arr[i];
@@ -2091,67 +2084,12 @@ static struct damos *damon_sysfs_mk_scheme(
return scheme;
}
-static void damon_sysfs_update_scheme(struct damos *scheme,
- struct damon_sysfs_scheme *sysfs_scheme)
-{
- struct damon_sysfs_access_pattern *access_pattern =
- sysfs_scheme->access_pattern;
- struct damon_sysfs_quotas *sysfs_quotas = sysfs_scheme->quotas;
- struct damon_sysfs_weights *sysfs_weights = sysfs_quotas->weights;
- struct damon_sysfs_watermarks *sysfs_wmarks = sysfs_scheme->watermarks;
- int err;
-
- scheme->pattern.min_sz_region = access_pattern->sz->min;
- scheme->pattern.max_sz_region = access_pattern->sz->max;
- scheme->pattern.min_nr_accesses = access_pattern->nr_accesses->min;
- scheme->pattern.max_nr_accesses = access_pattern->nr_accesses->max;
- scheme->pattern.min_age_region = access_pattern->age->min;
- scheme->pattern.max_age_region = access_pattern->age->max;
-
- scheme->action = sysfs_scheme->action;
- scheme->apply_interval_us = sysfs_scheme->apply_interval_us;
- scheme->target_nid = sysfs_scheme->target_nid;
-
- scheme->quota.ms = sysfs_quotas->ms;
- scheme->quota.sz = sysfs_quotas->sz;
- scheme->quota.reset_interval = sysfs_quotas->reset_interval_ms;
- scheme->quota.weight_sz = sysfs_weights->sz;
- scheme->quota.weight_nr_accesses = sysfs_weights->nr_accesses;
- scheme->quota.weight_age = sysfs_weights->age;
-
- err = damos_sysfs_set_quota_score(sysfs_quotas->goals, &scheme->quota);
- if (err) {
- damon_destroy_scheme(scheme);
- return;
- }
-
- scheme->wmarks.metric = sysfs_wmarks->metric;
- scheme->wmarks.interval = sysfs_wmarks->interval_us;
- scheme->wmarks.high = sysfs_wmarks->high;
- scheme->wmarks.mid = sysfs_wmarks->mid;
- scheme->wmarks.low = sysfs_wmarks->low;
-
- err = damon_sysfs_set_scheme_filters(scheme, sysfs_scheme->filters);
- if (err)
- damon_destroy_scheme(scheme);
-}
-
int damon_sysfs_set_schemes(struct damon_ctx *ctx,
struct damon_sysfs_schemes *sysfs_schemes)
{
- struct damos *scheme, *next;
- int i = 0;
-
- damon_for_each_scheme_safe(scheme, next, ctx) {
- if (i < sysfs_schemes->nr)
- damon_sysfs_update_scheme(scheme,
- sysfs_schemes->schemes_arr[i]);
- else
- damon_destroy_scheme(scheme);
- i++;
- }
+ int i;
- for (; i < sysfs_schemes->nr; i++) {
+ for (i = 0; i < sysfs_schemes->nr; i++) {
struct damos *scheme, *next;
scheme = damon_sysfs_mk_scheme(sysfs_schemes->schemes_arr[i]);
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 08/12] mm/damon/sysfs-schemes: rename *_set_{schemes,scheme_filters,quota_score,schemes}()
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
` (6 preceding siblings ...)
2024-06-18 18:18 ` [PATCH 07/12] mm/damon/sysfs-schemes: remove unnecessary online tuning handling code SeongJae Park
@ 2024-06-18 18:18 ` SeongJae Park
2024-06-18 18:18 ` [PATCH 09/12] mm/damon/reclaim: use damon_commit_ctx() SeongJae Park
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-mm, linux-kernel
The functions were for updating DAMON structs that may or may not be
partially populated. Hence it was not for only adding items, but also
removing unnecessary items and updating items in-place. A previous
commit has changed the functions to assume the structs are not partially
populated, and do only adding items. Make the names better explain the
behavior.
Signed-off-by: SeongJae Park <sj@kernel.org>
---
mm/damon/sysfs-common.h | 2 +-
mm/damon/sysfs-schemes.c | 12 ++++++------
mm/damon/sysfs.c | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/mm/damon/sysfs-common.h b/mm/damon/sysfs-common.h
index a63f51577cff..9a18f3c535d3 100644
--- a/mm/damon/sysfs-common.h
+++ b/mm/damon/sysfs-common.h
@@ -38,7 +38,7 @@ void damon_sysfs_schemes_rm_dirs(struct damon_sysfs_schemes *schemes);
extern const struct kobj_type damon_sysfs_schemes_ktype;
-int damon_sysfs_set_schemes(struct damon_ctx *ctx,
+int damon_sysfs_add_schemes(struct damon_ctx *ctx,
struct damon_sysfs_schemes *sysfs_schemes);
void damon_sysfs_schemes_update_stats(
diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 77c0265dff5c..b095457380b5 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -1908,7 +1908,7 @@ static int damon_sysfs_memcg_path_to_id(char *memcg_path, unsigned short *id)
return found ? 0 : -EINVAL;
}
-static int damon_sysfs_set_scheme_filters(struct damos *scheme,
+static int damon_sysfs_add_scheme_filters(struct damos *scheme,
struct damon_sysfs_scheme_filters *sysfs_filters)
{
int i;
@@ -1947,7 +1947,7 @@ static int damon_sysfs_set_scheme_filters(struct damos *scheme,
return 0;
}
-static int damos_sysfs_set_quota_score(
+static int damos_sysfs_add_quota_score(
struct damos_sysfs_quota_goals *sysfs_goals,
struct damos_quota *quota)
{
@@ -1990,7 +1990,7 @@ int damos_sysfs_set_quota_scores(struct damon_sysfs_schemes *sysfs_schemes,
break;
sysfs_scheme = sysfs_schemes->schemes_arr[i];
- err = damos_sysfs_set_quota_score(sysfs_scheme->quotas->goals,
+ err = damos_sysfs_add_quota_score(sysfs_scheme->quotas->goals,
"a);
if (err) {
damos_for_each_quota_goal_safe(g, g_next, "a)
@@ -2070,13 +2070,13 @@ static struct damos *damon_sysfs_mk_scheme(
if (!scheme)
return NULL;
- err = damos_sysfs_set_quota_score(sysfs_quotas->goals, &scheme->quota);
+ err = damos_sysfs_add_quota_score(sysfs_quotas->goals, &scheme->quota);
if (err) {
damon_destroy_scheme(scheme);
return NULL;
}
- err = damon_sysfs_set_scheme_filters(scheme, sysfs_filters);
+ err = damon_sysfs_add_scheme_filters(scheme, sysfs_filters);
if (err) {
damon_destroy_scheme(scheme);
return NULL;
@@ -2084,7 +2084,7 @@ static struct damos *damon_sysfs_mk_scheme(
return scheme;
}
-int damon_sysfs_set_schemes(struct damon_ctx *ctx,
+int damon_sysfs_add_schemes(struct damon_ctx *ctx,
struct damon_sysfs_schemes *sysfs_schemes)
{
int i;
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index f83ea6a166c6..cffc755e7775 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1286,7 +1286,7 @@ static int damon_sysfs_apply_inputs(struct damon_ctx *ctx,
err = damon_sysfs_add_targets(ctx, sys_ctx->targets);
if (err)
return err;
- return damon_sysfs_set_schemes(ctx, sys_ctx->schemes);
+ return damon_sysfs_add_schemes(ctx, sys_ctx->schemes);
}
static struct damon_ctx *damon_sysfs_build_ctx(
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 09/12] mm/damon/reclaim: use damon_commit_ctx()
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
` (7 preceding siblings ...)
2024-06-18 18:18 ` [PATCH 08/12] mm/damon/sysfs-schemes: rename *_set_{schemes,scheme_filters,quota_score,schemes}() SeongJae Park
@ 2024-06-18 18:18 ` SeongJae Park
2024-06-18 18:18 ` [PATCH 10/12] mm/damon/reclaim: remove unnecessary code for online tuning SeongJae Park
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-mm, linux-kernel
DAMON_RECLAIM manually manipulates the DAMON context struct for online
parameters update. Since the struct contains not only input parameters
but also internal status and operation results, it is not that simple.
Indeed, we found and fixed a few bugs in the code. Now DAMON core layer
provides a function for the usage, namely damon_commit_ctx(). Replace
the manual manipulation logic with the function. The core layer
function could have its own bugs, but this change removes a source of
bugs.
Signed-off-by: SeongJae Park <sj@kernel.org>
---
mm/damon/reclaim.c | 43 ++++++++++++++++++++++++-------------------
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index a05ccb41749b..be7f04b00d0c 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -195,59 +195,64 @@ static void damon_reclaim_copy_quota_status(struct damos_quota *dst,
static int damon_reclaim_apply_parameters(void)
{
+ struct damon_ctx *param_ctx;
+ struct damon_target *param_target;
struct damos *scheme, *old_scheme;
struct damos_quota_goal *goal;
struct damos_filter *filter;
- int err = 0;
+ int err;
- err = damon_set_attrs(ctx, &damon_reclaim_mon_attrs);
+ err = damon_modules_new_paddr_ctx_target(¶m_ctx, ¶m_target);
if (err)
return err;
- /* Will be freed by next 'damon_set_schemes()' below */
+ err = damon_set_attrs(ctx, &damon_reclaim_mon_attrs);
+ if (err)
+ goto out;
+
+ err = -ENOMEM;
scheme = damon_reclaim_new_scheme();
if (!scheme)
- return -ENOMEM;
+ goto out;
if (!list_empty(&ctx->schemes)) {
damon_for_each_scheme(old_scheme, ctx)
damon_reclaim_copy_quota_status(&scheme->quota,
&old_scheme->quota);
}
+ damon_set_schemes(ctx, &scheme, 1);
if (quota_mem_pressure_us) {
goal = damos_new_quota_goal(DAMOS_QUOTA_SOME_MEM_PSI_US,
quota_mem_pressure_us);
- if (!goal) {
- damon_destroy_scheme(scheme);
- return -ENOMEM;
- }
+ if (!goal)
+ goto out;
damos_add_quota_goal(&scheme->quota, goal);
}
if (quota_autotune_feedback) {
goal = damos_new_quota_goal(DAMOS_QUOTA_USER_INPUT, 10000);
- if (!goal) {
- damon_destroy_scheme(scheme);
- return -ENOMEM;
- }
+ if (!goal)
+ goto out;
goal->current_value = quota_autotune_feedback;
damos_add_quota_goal(&scheme->quota, goal);
}
if (skip_anon) {
filter = damos_new_filter(DAMOS_FILTER_TYPE_ANON, true);
- if (!filter) {
- /* Will be freed by next 'damon_set_schemes()' below */
- damon_destroy_scheme(scheme);
- return -ENOMEM;
- }
+ if (!filter)
+ goto out;
damos_add_filter(scheme, filter);
}
- damon_set_schemes(ctx, &scheme, 1);
- return damon_set_region_biggest_system_ram_default(target,
+ err = damon_set_region_biggest_system_ram_default(param_target,
&monitor_region_start,
&monitor_region_end);
+ if (err)
+ goto out;
+ err = damon_commit_ctx(ctx, param_ctx);
+out:
+ damon_destroy_ctx(param_ctx);
+ return err;
}
static int damon_reclaim_turn(bool on)
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 10/12] mm/damon/reclaim: remove unnecessary code for online tuning
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
` (8 preceding siblings ...)
2024-06-18 18:18 ` [PATCH 09/12] mm/damon/reclaim: use damon_commit_ctx() SeongJae Park
@ 2024-06-18 18:18 ` SeongJae Park
2024-06-18 18:18 ` [PATCH 11/12] mm/damon/lru_sort: use damon_commit_ctx() SeongJae Park
2024-06-18 18:18 ` [PATCH 12/12] mm/damon/lru_sort: remove unnecessary online tuning handling code SeongJae Park
11 siblings, 0 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-mm, linux-kernel
DAMON_RECLAIM contains code for handling of online DAMON parameters
update edge cases. It is no more necessary since damon_commit_ctx()
takes care of the cases. Remove the unnecessary code.
Signed-off-by: SeongJae Park <sj@kernel.org>
---
mm/damon/reclaim.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index be7f04b00d0c..9e0077a9404e 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -181,23 +181,11 @@ static struct damos *damon_reclaim_new_scheme(void)
NUMA_NO_NODE);
}
-static void damon_reclaim_copy_quota_status(struct damos_quota *dst,
- struct damos_quota *src)
-{
- dst->total_charged_sz = src->total_charged_sz;
- dst->total_charged_ns = src->total_charged_ns;
- dst->charged_sz = src->charged_sz;
- dst->charged_from = src->charged_from;
- dst->charge_target_from = src->charge_target_from;
- dst->charge_addr_from = src->charge_addr_from;
- dst->esz_bp = src->esz_bp;
-}
-
static int damon_reclaim_apply_parameters(void)
{
struct damon_ctx *param_ctx;
struct damon_target *param_target;
- struct damos *scheme, *old_scheme;
+ struct damos *scheme;
struct damos_quota_goal *goal;
struct damos_filter *filter;
int err;
@@ -214,11 +202,6 @@ static int damon_reclaim_apply_parameters(void)
scheme = damon_reclaim_new_scheme();
if (!scheme)
goto out;
- if (!list_empty(&ctx->schemes)) {
- damon_for_each_scheme(old_scheme, ctx)
- damon_reclaim_copy_quota_status(&scheme->quota,
- &old_scheme->quota);
- }
damon_set_schemes(ctx, &scheme, 1);
if (quota_mem_pressure_us) {
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 11/12] mm/damon/lru_sort: use damon_commit_ctx()
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
` (9 preceding siblings ...)
2024-06-18 18:18 ` [PATCH 10/12] mm/damon/reclaim: remove unnecessary code for online tuning SeongJae Park
@ 2024-06-18 18:18 ` SeongJae Park
2024-06-18 18:18 ` [PATCH 12/12] mm/damon/lru_sort: remove unnecessary online tuning handling code SeongJae Park
11 siblings, 0 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-mm, linux-kernel
DAMON_LRU_SORT manually manipulates the DAMON context struct for online
parameters update. Since the struct contains not only input parameters
but also internal status and operation results, it is not that simple.
Indeed, we found and fixed a few bugs in the code. Now DAMON core layer
provides a function for the usage, namely damon_commit_ctx(). Replace
the manual manipulation logic with the function. The core layer
function could have its own bugs, but this change removes a source of
bugs.
Signed-off-by: SeongJae Park <sj@kernel.org>
---
mm/damon/lru_sort.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 3775f0f2743d..f83542973946 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -199,15 +199,22 @@ static void damon_lru_sort_copy_quota_status(struct damos_quota *dst,
static int damon_lru_sort_apply_parameters(void)
{
+ struct damon_ctx *param_ctx;
+ struct damon_target *param_target;
struct damos *scheme, *hot_scheme, *cold_scheme;
struct damos *old_hot_scheme = NULL, *old_cold_scheme = NULL;
unsigned int hot_thres, cold_thres;
- int err = 0;
+ int err;
- err = damon_set_attrs(ctx, &damon_lru_sort_mon_attrs);
+ err = damon_modules_new_paddr_ctx_target(¶m_ctx, ¶m_target);
if (err)
return err;
+ err = damon_set_attrs(ctx, &damon_lru_sort_mon_attrs);
+ if (err)
+ goto out;
+
+ err = -ENOMEM;
damon_for_each_scheme(scheme, ctx) {
if (!old_hot_scheme) {
old_hot_scheme = scheme;
@@ -220,7 +227,7 @@ static int damon_lru_sort_apply_parameters(void)
hot_thres_access_freq / 1000;
hot_scheme = damon_lru_sort_new_hot_scheme(hot_thres);
if (!hot_scheme)
- return -ENOMEM;
+ goto out;
if (old_hot_scheme)
damon_lru_sort_copy_quota_status(&hot_scheme->quota,
&old_hot_scheme->quota);
@@ -229,18 +236,24 @@ static int damon_lru_sort_apply_parameters(void)
cold_scheme = damon_lru_sort_new_cold_scheme(cold_thres);
if (!cold_scheme) {
damon_destroy_scheme(hot_scheme);
- return -ENOMEM;
+ goto out;
}
if (old_cold_scheme)
damon_lru_sort_copy_quota_status(&cold_scheme->quota,
&old_cold_scheme->quota);
- damon_set_schemes(ctx, &hot_scheme, 1);
- damon_add_scheme(ctx, cold_scheme);
+ damon_set_schemes(param_ctx, &hot_scheme, 1);
+ damon_add_scheme(param_ctx, cold_scheme);
- return damon_set_region_biggest_system_ram_default(target,
+ err = damon_set_region_biggest_system_ram_default(param_target,
&monitor_region_start,
&monitor_region_end);
+ if (err)
+ goto out;
+ err = damon_commit_ctx(ctx, param_ctx);
+out:
+ damon_destroy_ctx(param_ctx);
+ return err;
}
static int damon_lru_sort_turn(bool on)
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 12/12] mm/damon/lru_sort: remove unnecessary online tuning handling code
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
` (10 preceding siblings ...)
2024-06-18 18:18 ` [PATCH 11/12] mm/damon/lru_sort: use damon_commit_ctx() SeongJae Park
@ 2024-06-18 18:18 ` SeongJae Park
11 siblings, 0 replies; 13+ messages in thread
From: SeongJae Park @ 2024-06-18 18:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: SeongJae Park, damon, linux-mm, linux-kernel
DAMON_LRU_SORT contains code for handling of online DAMON parameters
update edge cases. It is no more necessary since damon_commit_ctx()
takes care of the cases. Remove the unnecessary code.
Signed-off-by: SeongJae Park <sj@kernel.org>
---
mm/damon/lru_sort.c | 28 +---------------------------
1 file changed, 1 insertion(+), 27 deletions(-)
diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index f83542973946..4af8fd4a390b 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -186,23 +186,11 @@ static struct damos *damon_lru_sort_new_cold_scheme(unsigned int cold_thres)
return damon_lru_sort_new_scheme(&pattern, DAMOS_LRU_DEPRIO);
}
-static void damon_lru_sort_copy_quota_status(struct damos_quota *dst,
- struct damos_quota *src)
-{
- dst->total_charged_sz = src->total_charged_sz;
- dst->total_charged_ns = src->total_charged_ns;
- dst->charged_sz = src->charged_sz;
- dst->charged_from = src->charged_from;
- dst->charge_target_from = src->charge_target_from;
- dst->charge_addr_from = src->charge_addr_from;
-}
-
static int damon_lru_sort_apply_parameters(void)
{
struct damon_ctx *param_ctx;
struct damon_target *param_target;
- struct damos *scheme, *hot_scheme, *cold_scheme;
- struct damos *old_hot_scheme = NULL, *old_cold_scheme = NULL;
+ struct damos *hot_scheme, *cold_scheme;
unsigned int hot_thres, cold_thres;
int err;
@@ -215,22 +203,11 @@ static int damon_lru_sort_apply_parameters(void)
goto out;
err = -ENOMEM;
- damon_for_each_scheme(scheme, ctx) {
- if (!old_hot_scheme) {
- old_hot_scheme = scheme;
- continue;
- }
- old_cold_scheme = scheme;
- }
-
hot_thres = damon_max_nr_accesses(&damon_lru_sort_mon_attrs) *
hot_thres_access_freq / 1000;
hot_scheme = damon_lru_sort_new_hot_scheme(hot_thres);
if (!hot_scheme)
goto out;
- if (old_hot_scheme)
- damon_lru_sort_copy_quota_status(&hot_scheme->quota,
- &old_hot_scheme->quota);
cold_thres = cold_min_age / damon_lru_sort_mon_attrs.aggr_interval;
cold_scheme = damon_lru_sort_new_cold_scheme(cold_thres);
@@ -238,9 +215,6 @@ static int damon_lru_sort_apply_parameters(void)
damon_destroy_scheme(hot_scheme);
goto out;
}
- if (old_cold_scheme)
- damon_lru_sort_copy_quota_status(&cold_scheme->quota,
- &old_cold_scheme->quota);
damon_set_schemes(param_ctx, &hot_scheme, 1);
damon_add_scheme(param_ctx, cold_scheme);
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-06-18 18:18 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-18 18:17 [PATCH 00/12] mm/damon: introduce DAMON parameters online commit function SeongJae Park
2024-06-18 18:17 ` [PATCH 01/12] mm/damon/core: implement DAMOS quota goals " SeongJae Park
2024-06-18 18:17 ` [PATCH 02/12] mm/damon/core: implement DAMON context " SeongJae Park
2024-06-18 18:18 ` [PATCH 03/12] mm/damon/sysfs: use damon_commit_ctx() SeongJae Park
2024-06-18 18:18 ` [PATCH 04/12] mm/damon/sysfs-schemes: use damos_commit_quota_goals() SeongJae Park
2024-06-18 18:18 ` [PATCH 05/12] mm/damon/sysfs: remove unnecessary online tuning handling code SeongJae Park
2024-06-18 18:18 ` [PATCH 06/12] mm/damon/sysfs: rename damon_sysfs_set_targets() to ...add_targets() SeongJae Park
2024-06-18 18:18 ` [PATCH 07/12] mm/damon/sysfs-schemes: remove unnecessary online tuning handling code SeongJae Park
2024-06-18 18:18 ` [PATCH 08/12] mm/damon/sysfs-schemes: rename *_set_{schemes,scheme_filters,quota_score,schemes}() SeongJae Park
2024-06-18 18:18 ` [PATCH 09/12] mm/damon/reclaim: use damon_commit_ctx() SeongJae Park
2024-06-18 18:18 ` [PATCH 10/12] mm/damon/reclaim: remove unnecessary code for online tuning SeongJae Park
2024-06-18 18:18 ` [PATCH 11/12] mm/damon/lru_sort: use damon_commit_ctx() SeongJae Park
2024-06-18 18:18 ` [PATCH 12/12] mm/damon/lru_sort: remove unnecessary online tuning handling code SeongJae Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox