This patchset contains trivial cleansups for DAMON code.
SeongJae Park (6):
Docs/admin-guide/damon/reclaim: remove a paragraph that been obsolete
due to online tuning support
mm/damon/{dbgfs,sysfs}: move target_has_pid() from dbgfs to damon.h
mm/damon/reclaim: deduplicate 'commit_inputs' handling
mm/damon/sysfs: deduplicate inputs applying
mm/damon/reclaim: make 'enabled' checking timer simpler
mm/damon/reclaim: add 'damon_reclaim_' prefix to 'enabled_store()'
.../admin-guide/mm/damon/reclaim.rst | 6 --
include/linux/damon.h | 6 ++
mm/damon/dbgfs.c | 15 ++---
mm/damon/reclaim.c | 40 +++++------
mm/damon/sysfs.c | 67 ++++++++-----------
5 files changed, 55 insertions(+), 79 deletions(-)
--
2.25.1
DAMON sysfs interface's DAMON context building and its online parameter
update have duplicated code. This commit removes the duplicate.
Signed-off-by: SeongJae Park <[email protected]>
---
mm/damon/sysfs.c | 59 ++++++++++++++++++++----------------------------
1 file changed, 24 insertions(+), 35 deletions(-)
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 8810e6abdb06..c35809c6087c 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -2357,6 +2357,23 @@ static inline bool damon_sysfs_kdamond_running(
damon_sysfs_ctx_running(kdamond->damon_ctx);
}
+static int damon_sysfs_apply_inputs(struct damon_ctx *ctx,
+ struct damon_sysfs_context *sys_ctx)
+{
+ int err;
+
+ err = damon_select_ops(ctx, sys_ctx->ops_id);
+ if (err)
+ return err;
+ err = damon_sysfs_set_attrs(ctx, sys_ctx->attrs);
+ if (err)
+ return err;
+ err = damon_sysfs_set_targets(ctx, sys_ctx->targets);
+ if (err)
+ return err;
+ return damon_sysfs_set_schemes(ctx, sys_ctx->schemes);
+}
+
/*
* damon_sysfs_commit_input() - Commit user inputs to a running kdamond.
* @kdamond: The kobject wrapper for the associated kdamond.
@@ -2365,31 +2382,14 @@ static inline bool damon_sysfs_kdamond_running(
*/
static int damon_sysfs_commit_input(struct damon_sysfs_kdamond *kdamond)
{
- struct damon_ctx *ctx = kdamond->damon_ctx;
- struct damon_sysfs_context *sys_ctx;
- int err = 0;
-
if (!damon_sysfs_kdamond_running(kdamond))
return -EINVAL;
/* TODO: Support multiple contexts per kdamond */
if (kdamond->contexts->nr != 1)
return -EINVAL;
- sys_ctx = kdamond->contexts->contexts_arr[0];
-
- err = damon_select_ops(ctx, sys_ctx->ops_id);
- if (err)
- return err;
- err = damon_sysfs_set_attrs(ctx, sys_ctx->attrs);
- if (err)
- return err;
- err = damon_sysfs_set_targets(ctx, sys_ctx->targets);
- if (err)
- return err;
- err = damon_sysfs_set_schemes(ctx, sys_ctx->schemes);
- if (err)
- return err;
- return err;
+ return damon_sysfs_apply_inputs(kdamond->damon_ctx,
+ kdamond->contexts->contexts_arr[0]);
}
/*
@@ -2436,27 +2436,16 @@ static struct damon_ctx *damon_sysfs_build_ctx(
if (!ctx)
return ERR_PTR(-ENOMEM);
- err = damon_select_ops(ctx, sys_ctx->ops_id);
- if (err)
- goto out;
- err = damon_sysfs_set_attrs(ctx, sys_ctx->attrs);
- if (err)
- goto out;
- err = damon_sysfs_set_targets(ctx, sys_ctx->targets);
- if (err)
- goto out;
- err = damon_sysfs_set_schemes(ctx, sys_ctx->schemes);
- if (err)
- goto out;
+ err = damon_sysfs_apply_inputs(ctx, sys_ctx);
+ if (err) {
+ damon_destroy_ctx(ctx);
+ return ERR_PTR(err);
+ }
ctx->callback.after_wmarks_check = damon_sysfs_cmd_request_callback;
ctx->callback.after_aggregation = damon_sysfs_cmd_request_callback;
ctx->callback.before_terminate = damon_sysfs_before_terminate;
return ctx;
-
-out:
- damon_destroy_ctx(ctx);
- return ERR_PTR(err);
}
static int damon_sysfs_turn_damon_on(struct damon_sysfs_kdamond *kdamond)
--
2.25.1
This commit adds 'damon_reclaim_' prefix to 'enabled_store()', so that
we can distinguish it easily from the stack trace using 'faddr2line.sh'
like tools.
Signed-off-by: SeongJae Park <[email protected]>
---
mm/damon/reclaim.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 38da28803d75..e69b807fefe4 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -371,7 +371,7 @@ static DECLARE_DELAYED_WORK(damon_reclaim_timer, damon_reclaim_timer_fn);
static bool damon_reclaim_initialized;
-static int enabled_store(const char *val,
+static int damon_reclaim_enabled_store(const char *val,
const struct kernel_param *kp)
{
int rc = param_set_bool(val, kp);
@@ -388,7 +388,7 @@ static int enabled_store(const char *val,
}
static const struct kernel_param_ops enabled_param_ops = {
- .set = enabled_store,
+ .set = damon_reclaim_enabled_store,
.get = param_get_bool,
};
--
2.25.1
DAMON_RECLAIM's 'enabled' parameter store callback ('enabled_store()')
schedules the parameter check timer ('damon_reclaim_timer') if the
parameter is set as 'Y'. Then, the timer schedules itself to check if
user has set the parameter as 'N'. It's unnecessarily complex.
This commit makes it simpler by making the parameter store callback to
schedule the timer regardless of the parameter value and disabling the
timer's self scheduling.
Signed-off-by: SeongJae Park <[email protected]>
---
mm/damon/reclaim.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index c2ed962db23f..38da28803d75 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -353,7 +353,6 @@ static int damon_reclaim_turn(bool on)
return 0;
}
-#define ENABLE_CHECK_INTERVAL_MS 1000
static struct delayed_work damon_reclaim_timer;
static void damon_reclaim_timer_fn(struct work_struct *work)
{
@@ -367,10 +366,6 @@ static void damon_reclaim_timer_fn(struct work_struct *work)
else
enabled = last_enabled;
}
-
- if (enabled)
- schedule_delayed_work(&damon_reclaim_timer,
- msecs_to_jiffies(ENABLE_CHECK_INTERVAL_MS));
}
static DECLARE_DELAYED_WORK(damon_reclaim_timer, damon_reclaim_timer_fn);
@@ -388,9 +383,7 @@ static int enabled_store(const char *val,
if (!damon_reclaim_initialized)
return rc;
- if (enabled)
- schedule_delayed_work(&damon_reclaim_timer, 0);
-
+ schedule_delayed_work(&damon_reclaim_timer, 0);
return 0;
}
--
2.25.1
The function for knowing if given monitoring context's targets will have
pid or not is defined and used in dbgfs only. However, the logic is
also needed for sysfs. This commit moves the code to damon.h and makes
both dbgfs and sysfs to use it.
Signed-off-by: SeongJae Park <[email protected]>
---
include/linux/damon.h | 6 ++++++
mm/damon/dbgfs.c | 15 +++++----------
mm/damon/sysfs.c | 8 +++-----
3 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 2765c7d99beb..b9aae19fab3e 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -525,6 +525,12 @@ bool damon_is_registered_ops(enum damon_ops_id id);
int damon_register_ops(struct damon_operations *ops);
int damon_select_ops(struct damon_ctx *ctx, enum damon_ops_id id);
+static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
+{
+ return ctx->ops.id == DAMON_OPS_VADDR || ctx->ops.id == DAMON_OPS_FVADDR;
+}
+
+
int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive);
int damon_stop(struct damon_ctx **ctxs, int nr_ctxs);
diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c
index a0dab8b5e45f..5ae810927309 100644
--- a/mm/damon/dbgfs.c
+++ b/mm/damon/dbgfs.c
@@ -275,11 +275,6 @@ static ssize_t dbgfs_schemes_write(struct file *file, const char __user *buf,
return ret;
}
-static inline bool target_has_pid(const struct damon_ctx *ctx)
-{
- return ctx->ops.id == DAMON_OPS_VADDR;
-}
-
static ssize_t sprint_target_ids(struct damon_ctx *ctx, char *buf, ssize_t len)
{
struct damon_target *t;
@@ -288,7 +283,7 @@ static ssize_t sprint_target_ids(struct damon_ctx *ctx, char *buf, ssize_t len)
int rc;
damon_for_each_target(t, ctx) {
- if (target_has_pid(ctx))
+ if (damon_target_has_pid(ctx))
/* Show pid numbers to debugfs users */
id = pid_vnr(t->pid);
else
@@ -415,7 +410,7 @@ static int dbgfs_set_targets(struct damon_ctx *ctx, ssize_t nr_targets,
struct damon_target *t, *next;
damon_for_each_target_safe(t, next, ctx) {
- if (target_has_pid(ctx))
+ if (damon_target_has_pid(ctx))
put_pid(t->pid);
damon_destroy_target(t);
}
@@ -425,11 +420,11 @@ static int dbgfs_set_targets(struct damon_ctx *ctx, ssize_t nr_targets,
if (!t) {
damon_for_each_target_safe(t, next, ctx)
damon_destroy_target(t);
- if (target_has_pid(ctx))
+ if (damon_target_has_pid(ctx))
dbgfs_put_pids(pids, nr_targets);
return -ENOMEM;
}
- if (target_has_pid(ctx))
+ if (damon_target_has_pid(ctx))
t->pid = pids[i];
damon_add_target(ctx, t);
}
@@ -722,7 +717,7 @@ static void dbgfs_before_terminate(struct damon_ctx *ctx)
{
struct damon_target *t, *next;
- if (!target_has_pid(ctx))
+ if (!damon_target_has_pid(ctx))
return;
mutex_lock(&ctx->kdamond_lock);
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 09f9e8ca3d1f..8810e6abdb06 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -2136,8 +2136,7 @@ static void damon_sysfs_destroy_targets(struct damon_ctx *ctx)
struct damon_target *t, *next;
damon_for_each_target_safe(t, next, ctx) {
- if (ctx->ops.id == DAMON_OPS_VADDR ||
- ctx->ops.id == DAMON_OPS_FVADDR)
+ if (damon_target_has_pid(ctx))
put_pid(t->pid);
damon_destroy_target(t);
}
@@ -2181,8 +2180,7 @@ static int damon_sysfs_add_target(struct damon_sysfs_target *sys_target,
if (!t)
return -ENOMEM;
- if (ctx->ops.id == DAMON_OPS_VADDR ||
- ctx->ops.id == DAMON_OPS_FVADDR) {
+ if (damon_target_has_pid(ctx)) {
t->pid = find_get_pid(sys_target->pid);
if (!t->pid)
goto destroy_targets_out;
@@ -2210,7 +2208,7 @@ static struct damon_target *damon_sysfs_existing_target(
struct pid *pid;
struct damon_target *t;
- if (ctx->ops.id == DAMON_OPS_PADDR) {
+ if (!damon_target_has_pid(ctx)) {
/* Up to only one target for paddr could exist */
damon_for_each_target(t, ctx)
return t;
--
2.25.1