* [PATCH 1/5] mm/damon/core: Add damos_destroy_dests()
2025-08-05 16:20 [PATCH 0/5] mm/damon/sysfs: Add commands useful for using migration dests Bijan Tabatabai
@ 2025-08-05 16:20 ` Bijan Tabatabai
2025-08-05 16:20 ` [PATCH 2/5] mm/damon/sysfs: Implement a command to only commit scheme dests Bijan Tabatabai
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Bijan Tabatabai @ 2025-08-05 16:20 UTC (permalink / raw)
To: damon, linux-mm, linux-kernel, linux-doc
Cc: sj, Andrew Morton, corbet, Bijan Tabatabai
From: Bijan Tabatabai <bijantabatab@micron.com>
This patch adds damos_destroy_dests(), which is used to free a struct
damos_migrate_dests. Currently, the contents of a struct
damos_migrate_dests in freed inline in damon_destroy_scheme(). Moving
the code to a separate function is useful for when damos_migrate_dest
structs need to also be freed elsewhere.
Signed-off-by: Bijan Tabatabai <bijantabatab@micron.com>
---
include/linux/damon.h | 2 ++
mm/damon/core.c | 9 +++++++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 6b797120d2f2..a851c8bc2e52 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -914,6 +914,8 @@ struct damos_quota_goal *damos_new_quota_goal(
void damos_add_quota_goal(struct damos_quota *q, struct damos_quota_goal *g);
void damos_destroy_quota_goal(struct damos_quota_goal *goal);
+void damos_destroy_dests(struct damos_migrate_dests *dests);
+
struct damos *damon_new_scheme(struct damos_access_pattern *pattern,
enum damos_action action,
unsigned long apply_interval_us,
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 6a2fe1f2c952..07b4fc5a9790 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -359,6 +359,12 @@ void damos_destroy_quota_goal(struct damos_quota_goal *g)
damos_free_quota_goal(g);
}
+void damos_destroy_dests(struct damos_migrate_dests *dests)
+{
+ kfree(dests->node_id_arr);
+ kfree(dests->weight_arr);
+}
+
/* initialize fields of @quota that normally API users wouldn't set */
static struct damos_quota *damos_quota_init(struct damos_quota *quota)
{
@@ -451,8 +457,7 @@ void damon_destroy_scheme(struct damos *s)
damos_for_each_filter_safe(f, next, s)
damos_destroy_filter(f);
- kfree(s->migrate_dests.node_id_arr);
- kfree(s->migrate_dests.weight_arr);
+ damos_destroy_dests(&s->migrate_dests);
damon_del_scheme(s);
damon_free_scheme(s);
}
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/5] mm/damon/sysfs: Implement a command to only commit scheme dests
2025-08-05 16:20 [PATCH 0/5] mm/damon/sysfs: Add commands useful for using migration dests Bijan Tabatabai
2025-08-05 16:20 ` [PATCH 1/5] mm/damon/core: Add damos_destroy_dests() Bijan Tabatabai
@ 2025-08-05 16:20 ` Bijan Tabatabai
2025-08-05 16:20 ` [PATCH 3/5] mm/damon/sysfs: Implement a command to wait until schemes are applied Bijan Tabatabai
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Bijan Tabatabai @ 2025-08-05 16:20 UTC (permalink / raw)
To: damon, linux-mm, linux-kernel, linux-doc
Cc: sj, Andrew Morton, corbet, Bijan Tabatabai
From: Bijan Tabatabai <bijantabatab@micron.com>
To update DAMOS migration dests, users need to write "commit" to the
"state" file of kdamond, which will recommit all of the damon parameters.
This patch implements another state file input command,
"commit_scheme_dests," that only commits the DAMOS migration dests.
This provides two benefits:
1) It is slightly more efficient
2) When the "commit" command is sent to the state file,
ctx->next_{aggregation/ops_update}_sis are reset. If a user sends the
"commit" command frequently (relative to the aggregation and
ops_update periods) to update the migration dests, the aggregation and
ops_update events will be prevented from triggering. Having a
separate command side steps this problem.
Signed-off-by: Bijan Tabatabai <bijantabatab@micron.com>
---
mm/damon/sysfs-common.h | 3 +++
mm/damon/sysfs-schemes.c | 35 ++++++++++++++++++++++++++++++++---
mm/damon/sysfs.c | 27 +++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 3 deletions(-)
diff --git a/mm/damon/sysfs-common.h b/mm/damon/sysfs-common.h
index 2099adee11d0..3189b2bda079 100644
--- a/mm/damon/sysfs-common.h
+++ b/mm/damon/sysfs-common.h
@@ -59,3 +59,6 @@ int damos_sysfs_set_quota_scores(struct damon_sysfs_schemes *sysfs_schemes,
void damos_sysfs_update_effective_quotas(
struct damon_sysfs_schemes *sysfs_schemes,
struct damon_ctx *ctx);
+
+int damos_sysfs_set_schemes_dests(struct damon_sysfs_schemes *sysfs_schemes,
+ struct damon_ctx *ctx);
diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index d355f6c42a98..d30c8b6575c2 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -2445,10 +2445,9 @@ void damos_sysfs_update_effective_quotas(
}
}
-static int damos_sysfs_add_migrate_dest(struct damos *scheme,
+static int damos_sysfs_add_migrate_dest(struct damos_migrate_dests *dests,
struct damos_sysfs_dests *sysfs_dests)
{
- struct damos_migrate_dests *dests = &scheme->migrate_dests;
int i;
dests->node_id_arr = kmalloc_array(sysfs_dests->nr,
@@ -2468,6 +2467,35 @@ static int damos_sysfs_add_migrate_dest(struct damos *scheme,
return 0;
}
+int damos_sysfs_set_schemes_dests(struct damon_sysfs_schemes *sysfs_schemes,
+ struct damon_ctx *ctx)
+{
+ struct damos *scheme;
+ int i = 0;
+
+ damon_for_each_scheme(scheme, ctx) {
+ struct damos_sysfs_dests *sysfs_dests;
+ struct damos_migrate_dests dests = {};
+ int err;
+
+ /* user could have removed the scheme sysfs dir */
+ if (i >= sysfs_schemes->nr)
+ break;
+
+ sysfs_dests = sysfs_schemes->schemes_arr[i]->dests;
+ err = damos_sysfs_add_migrate_dest(&dests, sysfs_dests);
+ if (err) {
+ damos_destroy_dests(&dests);
+ return err;
+ }
+
+ damos_destroy_dests(&scheme->migrate_dests);
+ scheme->migrate_dests = dests;
+ }
+
+ return 0;
+}
+
static struct damos *damon_sysfs_mk_scheme(
struct damon_sysfs_scheme *sysfs_scheme)
{
@@ -2530,7 +2558,8 @@ static struct damos *damon_sysfs_mk_scheme(
damon_destroy_scheme(scheme);
return NULL;
}
- err = damos_sysfs_add_migrate_dest(scheme, sysfs_scheme->dests);
+ err = damos_sysfs_add_migrate_dest(&scheme->migrate_dests,
+ sysfs_scheme->dests);
if (err) {
damon_destroy_scheme(scheme);
return NULL;
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 1af6aff35d84..c2b036abb25b 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1188,6 +1188,11 @@ enum damon_sysfs_cmd {
* to DAMON.
*/
DAMON_SYSFS_CMD_COMMIT_SCHEMES_QUOTA_GOALS,
+ /*
+ * @DAMON_SYSFS_CMD_COMMIT_SCHEMES_DESTS: Commit the scheme dests to
+ * DAMON.
+ */
+ DAMON_SYSFS_CMD_COMMIT_SCHEMES_DESTS,
/*
* @DAMON_SYSFS_CMD_UPDATE_SCHEMES_STATS: Update scheme stats sysfs
* files.
@@ -1230,6 +1235,7 @@ static const char * const damon_sysfs_cmd_strs[] = {
"off",
"commit",
"commit_schemes_quota_goals",
+ "commit_schemes_dests",
"update_schemes_stats",
"update_schemes_tried_bytes",
"update_schemes_tried_regions",
@@ -1478,6 +1484,23 @@ static int damon_sysfs_commit_schemes_quota_goals(void *data)
return damos_sysfs_set_quota_scores(sysfs_ctx->schemes, ctx);
}
+static int damon_sysfs_commit_schemes_dests(void *data)
+{
+ struct damon_sysfs_kdamond *sysfs_kdamond = data;
+ struct damon_ctx *ctx;
+ struct damon_sysfs_context *sysfs_ctx;
+
+ if (!damon_sysfs_kdamond_running(sysfs_kdamond))
+ return -EINVAL;
+ /* TODO: Support multiple contexts per kdamond */
+ if (sysfs_kdamond->contexts->nr != 1)
+ return -EINVAL;
+
+ ctx = sysfs_kdamond->damon_ctx;
+ sysfs_ctx = sysfs_kdamond->contexts->contexts_arr[0];
+ return damos_sysfs_set_schemes_dests(sysfs_ctx->schemes, ctx);
+}
+
/*
* damon_sysfs_upd_schemes_effective_quotas() - Update schemes effective quotas
* sysfs files.
@@ -1644,6 +1667,10 @@ static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
return damon_sysfs_damon_call(
damon_sysfs_commit_schemes_quota_goals,
kdamond);
+ case DAMON_SYSFS_CMD_COMMIT_SCHEMES_DESTS:
+ return damon_sysfs_damon_call(
+ damon_sysfs_commit_schemes_dests,
+ kdamond);
case DAMON_SYSFS_CMD_UPDATE_SCHEMES_STATS:
return damon_sysfs_damon_call(
damon_sysfs_upd_schemes_stats, kdamond);
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/5] mm/damon/sysfs: Implement a command to wait until schemes are applied
2025-08-05 16:20 [PATCH 0/5] mm/damon/sysfs: Add commands useful for using migration dests Bijan Tabatabai
2025-08-05 16:20 ` [PATCH 1/5] mm/damon/core: Add damos_destroy_dests() Bijan Tabatabai
2025-08-05 16:20 ` [PATCH 2/5] mm/damon/sysfs: Implement a command to only commit scheme dests Bijan Tabatabai
@ 2025-08-05 16:20 ` Bijan Tabatabai
2025-08-05 16:20 ` [PATCH 4/5] Docs/ABI/damon: Document new DAMON commands Bijan Tabatabai
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Bijan Tabatabai @ 2025-08-05 16:20 UTC (permalink / raw)
To: damon, linux-mm, linux-kernel, linux-doc
Cc: sj, Andrew Morton, corbet, Bijan Tabatabai
From: Bijan Tabatabai <bijantabatab@micron.com>
Any DAMON command that uses damos_walk() internally, such as
update_schemes_tried_bytes and update_schemes_tried_regions, waits until
each scheme in the DAMON context have finished. This can be useful, for
example, if a user wants to know when new scheme parameters they've
committed have been applied. Another use case could be if a user wants
to record the system state every time a scheme is applied for debug
purposes.
This patch adds a new command, wait_for_schemes_apply, which calls
damos_walk() without a walk function so that all it does is wait until
all schemes have been applied. The same functionality can be achieved
by using update_schemes_tried_bytes, for example, but having a separate
command for this avoid unnecessary work. It also makes user intent
clearer when used in scripts.
Signed-off-by: Bijan Tabatabai <bijantabatab@micron.com>
---
mm/damon/core.c | 3 ++-
mm/damon/sysfs.c | 27 +++++++++++++++++++++++++++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 07b4fc5a9790..56a13d16e4d1 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1731,7 +1731,8 @@ static void damos_walk_call_walk(struct damon_ctx *ctx, struct damon_target *t,
if (!control)
return;
- control->walk_fn(control->data, ctx, t, r, s, sz_filter_passed);
+ if (control->walk_fn)
+ control->walk_fn(control->data, ctx, t, r, s, sz_filter_passed);
}
/*
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index c2b036abb25b..ded3f60e4e22 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1223,6 +1223,11 @@ enum damon_sysfs_cmd {
* intevals.
*/
DAMON_SYSFS_CMD_UPDATE_TUNED_INTERVALS,
+ /*
+ * @DAMON_SYSFS_CMD_WAIT_FOR_SCHEMES_APPLY: Wait for all schemes to be
+ * applied.
+ */
+ DAMON_SYSFS_CMD_WAIT_FOR_SCHEMES_APPLY,
/*
* @NR_DAMON_SYSFS_CMDS: Total number of DAMON sysfs commands.
*/
@@ -1242,6 +1247,7 @@ static const char * const damon_sysfs_cmd_strs[] = {
"clear_schemes_tried_regions",
"update_schemes_effective_quotas",
"update_tuned_intervals",
+ "wait_for_schemes_apply",
};
static ssize_t state_show(struct kobject *kobj, struct kobj_attribute *attr,
@@ -1643,6 +1649,25 @@ static int damon_sysfs_update_schemes_tried_regions(
return damos_walk(ctx, &control);
}
+static int damon_sysfs_wait_for_schemes_apply(
+ struct damon_sysfs_kdamond *sysfs_kdamond)
+{
+ /*
+ * damos_walk returns after the next time all of the schemes have been
+ * applied. We don't need to do any actual work, so walk_fn is NULL.
+ */
+ struct damos_walk_control control = {
+ .walk_fn = NULL,
+ .data = NULL,
+ };
+ struct damon_ctx *ctx = sysfs_kdamond->damon_ctx;
+
+ if (!ctx)
+ return -EINVAL;
+
+ return damos_walk(ctx, &control);
+}
+
/*
* damon_sysfs_handle_cmd() - Handle a command for a specific kdamond.
* @cmd: The command to handle.
@@ -1688,6 +1713,8 @@ static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
case DAMON_SYSFS_CMD_UPDATE_TUNED_INTERVALS:
return damon_sysfs_damon_call(
damon_sysfs_upd_tuned_intervals, kdamond);
+ case DAMON_SYSFS_CMD_WAIT_FOR_SCHEMES_APPLY:
+ return damon_sysfs_wait_for_schemes_apply(kdamond);
default:
return -EINVAL;
}
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/5] Docs/ABI/damon: Document new DAMON commands
2025-08-05 16:20 [PATCH 0/5] mm/damon/sysfs: Add commands useful for using migration dests Bijan Tabatabai
` (2 preceding siblings ...)
2025-08-05 16:20 ` [PATCH 3/5] mm/damon/sysfs: Implement a command to wait until schemes are applied Bijan Tabatabai
@ 2025-08-05 16:20 ` Bijan Tabatabai
2025-08-05 16:20 ` [PATCH 5/5] Docs/admin-guide/mm/damon/usage: " Bijan Tabatabai
2025-08-06 0:40 ` [PATCH 0/5] mm/damon/sysfs: Add commands useful for using migration dests SeongJae Park
5 siblings, 0 replies; 12+ messages in thread
From: Bijan Tabatabai @ 2025-08-05 16:20 UTC (permalink / raw)
To: damon, linux-mm, linux-kernel, linux-doc
Cc: sj, Andrew Morton, corbet, Bijan Tabatabai
From: Bijan Tabatabai <bijantabatab@micron.com>
Document the new commit_schemes_dests and wait_for_schemes_apply DAMON
commands.
Signed-off-by: Bijan Tabatabai <bijantabatab@micron.com>
---
.../ABI/testing/sysfs-kernel-mm-damon | 21 +++++++++++--------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-damon b/Documentation/ABI/testing/sysfs-kernel-mm-damon
index e98974dfac7a..429947de9aba 100644
--- a/Documentation/ABI/testing/sysfs-kernel-mm-damon
+++ b/Documentation/ABI/testing/sysfs-kernel-mm-damon
@@ -27,16 +27,19 @@ Description: Writing 'on' or 'off' to this file makes the kdamond starts or
makes the kdamond reads the user inputs in the sysfs files
except 'state' again. Writing 'commit_schemes_quota_goals' to
this file makes the kdamond reads the quota goal files again.
- Writing 'update_schemes_stats' to the file updates contents of
- schemes stats files of the kdamond. Writing
- 'update_schemes_tried_regions' to the file updates contents of
- 'tried_regions' directory of every scheme directory of this
- kdamond. Writing 'update_schemes_tried_bytes' to the file
- updates only '.../tried_regions/total_bytes' files of this
- kdamond. Writing 'clear_schemes_tried_regions' to the file
- removes contents of the 'tried_regions' directory. Writing
+ Writing 'commit_schemes_dests' to this file makes the kdamond
+ read the dests files again. Writing 'update_schemes_stats' to
+ the file updates contents of schemes stats files of the
+ kdamond. Writing 'update_schemes_tried_regions' to the file
+ updates contents of 'tried_regions' directory of every scheme
+ directory of this kdamond. Writing 'update_schemes_tried_bytes'
+ to the file updates only '.../tried_regions/total_bytes' files
+ of this kdamond. Writing 'clear_schemes_tried_regions' to the
+ file removes contents of the 'tried_regions' directory. Writing
'update_schemes_effective_quotas' to the file updates
- '.../quotas/effective_bytes' files of this kdamond.
+ '.../quotas/effective_bytes' files of this kdamond. Writing
+ 'wait_for_schemes_apply' to the file does nothing but wait until
+ the next time every scheme is applied.
What: /sys/kernel/mm/damon/admin/kdamonds/<K>/pid
Date: Mar 2022
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/5] Docs/admin-guide/mm/damon/usage: Document new DAMON commands
2025-08-05 16:20 [PATCH 0/5] mm/damon/sysfs: Add commands useful for using migration dests Bijan Tabatabai
` (3 preceding siblings ...)
2025-08-05 16:20 ` [PATCH 4/5] Docs/ABI/damon: Document new DAMON commands Bijan Tabatabai
@ 2025-08-05 16:20 ` Bijan Tabatabai
2025-08-06 0:40 ` [PATCH 0/5] mm/damon/sysfs: Add commands useful for using migration dests SeongJae Park
5 siblings, 0 replies; 12+ messages in thread
From: Bijan Tabatabai @ 2025-08-05 16:20 UTC (permalink / raw)
To: damon, linux-mm, linux-kernel, linux-doc
Cc: sj, Andrew Morton, corbet, Bijan Tabatabai
From: Bijan Tabatabai <bijantabatab@micron.com>
Document the new commit_schemes_dests and wait_for_schemes_apply DAMON
commands.
Signed-off-by: Bijan Tabatabai <bijantabatab@micron.com>
---
Documentation/admin-guide/mm/damon/usage.rst | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/admin-guide/mm/damon/usage.rst b/Documentation/admin-guide/mm/damon/usage.rst
index fc5c962353ed..46b6260855d9 100644
--- a/Documentation/admin-guide/mm/damon/usage.rst
+++ b/Documentation/admin-guide/mm/damon/usage.rst
@@ -142,6 +142,8 @@ Users can write below commands for the kdamond to the ``state`` file.
for more details.
- ``commit_schemes_quota_goals``: Read the DAMON-based operation schemes'
:ref:`quota goals <sysfs_schemes_quota_goals>`.
+- ``commit_schemes_dests``: Read the DAMON-based operation schemes'
+ :ref:`migration dests <damon_sysfs_dests>`.
- ``update_schemes_stats``: Update the contents of stats files for each
DAMON-based operation scheme of the kdamond. For details of the stats,
please refer to :ref:`stats section <sysfs_schemes_stats>`.
@@ -158,6 +160,8 @@ Users can write below commands for the kdamond to the ``state`` file.
- ``update_schemes_effective_quotas``: Update the contents of
``effective_bytes`` files for each DAMON-based operation scheme of the
kdamond. For more details, refer to :ref:`quotas directory <sysfs_quotas>`.
+- ``wait_for_schemes_apply``: Wait until the next time every scheme has been
+ applied.
If the state is ``on``, reading ``pid`` shows the pid of the kdamond thread.
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 0/5] mm/damon/sysfs: Add commands useful for using migration dests
2025-08-05 16:20 [PATCH 0/5] mm/damon/sysfs: Add commands useful for using migration dests Bijan Tabatabai
` (4 preceding siblings ...)
2025-08-05 16:20 ` [PATCH 5/5] Docs/admin-guide/mm/damon/usage: " Bijan Tabatabai
@ 2025-08-06 0:40 ` SeongJae Park
2025-08-06 1:27 ` Bijan Tabatabai
5 siblings, 1 reply; 12+ messages in thread
From: SeongJae Park @ 2025-08-06 0:40 UTC (permalink / raw)
To: Bijan Tabatabai
Cc: SeongJae Park, damon, linux-mm, linux-kernel, linux-doc,
Andrew Morton, corbet, Bijan Tabatabai
Hi Bijan,
Subjects of patches usually be converted into lowercase when be merged into mm
tree. I'd suggest using lowercase even in patches stage, if those aim to be
merged into mm tree.
On Tue, 5 Aug 2025 11:20:17 -0500 Bijan Tabatabai <bijan311@gmail.com> wrote:
> From: Bijan Tabatabai <bijantabatab@micron.com>
>
> This patchset adds two DAMON commands, commit_schemes_dests and
> wait_for_schemes_apply, that I have found useful for using a migrate_hot
> scheme with migration dests.
Thank you for this patchset! I believe we shouldn't be afraid at adding
features, but should also be carefult at making good solutions for real
problems. So, my main response to this patchset is, hopefully unsurprisingly,
I'd like to better understand the requirements and the problem you are
encountering.
>
> The commit_schemes_dests command, similar to the existing
> commit_schemes_quota_goals, is used to commit only the dests fields of
> schemes. This has a couple of benefits:
> 1) It is more efficient than recommitting all the DAMON data.
> 2) Doing a full commit resets the aggregation and ops_update intervals. If
> a user sends the full commit command frequently (relatively to those
> intervals) the aggregation and ops_update events will be prevented from
> triggering. Having a separate commit command side steps this problem.
I agree the commit command of DAMON sysfs interface is inefficient, and could
make the infinite intervals delay problem. But, I didn't expect there could be
use cases that use commit feature frequently enough to make the inefficiency
and the intervals delay be real problems. Could you please let me know more
details about your use case and how severe problem DAMON is causing?
Depending on the real problem, I'm wondering if optimizing commit command can
be a solution. For example, skipping the update of next_aggregation_sis and
next_ops_update_sis when the intervals are not changed might be able to solve
the intervals delay problem.
>
> The wait_for_schemes_apply command causes the calling thread to wait until
> all schemes have been applied. It does this by calling damos_walk() with a
> NULL walk_fn. This can be useful, for example, if a user wants to know when
> new scheme parameters they've committed have been applied. Another use case
> could be if a user wants to record the system state every time a scheme is
> applied for debuging purposes.
>
> The functionality of wait_for_schemes_apply can be achieved with the
> existing update_schemes_tried_bytes and update_schemes_tried_regions
> commands. However, having a separate command avoids extra work and makes
> user intent clearer when used in scripts.
I agree extra works are always better to be avoided. But is the overhead large
enough to be a real problem for your use case? I also agree it could make the
user script cleaner, but adding a kernel feature only for user scripts
readabilities sounds like too much, since the user script could have its own
abstract layers for its readability.
Also, even if the new command is implemented, since the DAMOS schemes continue
running, the system status will keep changing. If you cannot do the recording
of the system state in a restricted time, the recorded information might not be
that reliable. So I'm not sure if you really need this strict waiting in this
way.
Could you please share more details about what you want to do with the new
command, and how much problem you are seeing? I'm particularly curious what
system state you want to record, and why you need to wait the exact time
interval.
Depending on the real use case and the problem, I think we might be able to
reuse the DAMOS scheme stats, or implement more strict and reliable time
control features, say, making kdamond or schemes pause and resume as users ask?
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/5] mm/damon/sysfs: Add commands useful for using migration dests
2025-08-06 0:40 ` [PATCH 0/5] mm/damon/sysfs: Add commands useful for using migration dests SeongJae Park
@ 2025-08-06 1:27 ` Bijan Tabatabai
2025-08-06 4:15 ` SeongJae Park
0 siblings, 1 reply; 12+ messages in thread
From: Bijan Tabatabai @ 2025-08-06 1:27 UTC (permalink / raw)
To: SeongJae Park
Cc: damon, linux-mm, linux-kernel, linux-doc, Andrew Morton, corbet,
Bijan Tabatabai
Hi SJ,
On Tue, Aug 5, 2025 at 7:40 PM SeongJae Park <sj@kernel.org> wrote:
>
> Hi Bijan,
>
> Subjects of patches usually be converted into lowercase when be merged into mm
> tree. I'd suggest using lowercase even in patches stage, if those aim to be
> merged into mm tree.
Noted, thanks.
[...]
> > The commit_schemes_dests command, similar to the existing
> > commit_schemes_quota_goals, is used to commit only the dests fields of
> > schemes. This has a couple of benefits:
> > 1) It is more efficient than recommitting all the DAMON data.
> > 2) Doing a full commit resets the aggregation and ops_update intervals. If
> > a user sends the full commit command frequently (relatively to those
> > intervals) the aggregation and ops_update events will be prevented from
> > triggering. Having a separate commit command side steps this problem.
>
> I agree the commit command of DAMON sysfs interface is inefficient, and could
> make the infinite intervals delay problem. But, I didn't expect there could be
> use cases that use commit feature frequently enough to make the inefficiency
> and the intervals delay be real problems. Could you please let me know more
> details about your use case and how severe problem DAMON is causing?
In my use case, I am trying to optimize the interleave ratio of
applications to maximize their performance without prior knowledge of
their behavior. To do this, we take the steps of updating the ratio,
observing how the system reacts to the change in ratio, and update the
ratio again accordingly. Because we want to approach the ideal
interleave ratio quickly, we update the weights frequently, motivating
the commit_schemes_dests. Similarly, we want to observe how the system
reacts to the change only after the change has been applied,
motivating wait_for_schemes_apply.
The consequences are not very severe. The problem can be worked around
by either updating less frequently, at the cost of converging slower,
or decreasing the maximum aggregation period, which from what I
understand may affect the access monitoring behavior.
> Depending on the real problem, I'm wondering if optimizing commit command can
> be a solution. For example, skipping the update of next_aggregation_sis and
> next_ops_update_sis when the intervals are not changed might be able to solve
> the intervals delay problem.
This would work for my use case. Another option might be to have a
more general commit_schemes command, which may be useful to other use
cases. I'll defer to your judgement on which would be better.
> >
> > The wait_for_schemes_apply command causes the calling thread to wait until
> > all schemes have been applied. It does this by calling damos_walk() with a
> > NULL walk_fn. This can be useful, for example, if a user wants to know when
> > new scheme parameters they've committed have been applied. Another use case
> > could be if a user wants to record the system state every time a scheme is
> > applied for debuging purposes.
> >
> > The functionality of wait_for_schemes_apply can be achieved with the
> > existing update_schemes_tried_bytes and update_schemes_tried_regions
> > commands. However, having a separate command avoids extra work and makes
> > user intent clearer when used in scripts.
>
> I agree extra works are always better to be avoided. But is the overhead large
> enough to be a real problem for your use case? I also agree it could make the
> user script cleaner, but adding a kernel feature only for user scripts
> readabilities sounds like too much, since the user script could have its own
> abstract layers for its readability.
Totally fair. I will drop wait_for_apply_schemes in any future versions.
> Also, even if the new command is implemented, since the DAMOS schemes continue
> running, the system status will keep changing. If you cannot do the recording
> of the system state in a restricted time, the recorded information might not be
> that reliable. So I'm not sure if you really need this strict waiting in this
> way.
Fair. That was not something I was personally using the command for,
just another possible use case I thought of. Regardless of the
usefulness of that, the existing commands using damos_walk would work
well enough.
> Could you please share more details about what you want to do with the new
> command, and how much problem you are seeing? I'm particularly curious what
> system state you want to record, and why you need to wait the exact time
> interval.
I mentioned this above, but I am using this to wait for new migration
weights to be applied before monitoring how the change affects
applications, but again, this can be done with existing commands.
Thanks for your response,
Bijan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/5] mm/damon/sysfs: Add commands useful for using migration dests
2025-08-06 1:27 ` Bijan Tabatabai
@ 2025-08-06 4:15 ` SeongJae Park
0 siblings, 0 replies; 12+ messages in thread
From: SeongJae Park @ 2025-08-06 4:15 UTC (permalink / raw)
To: Bijan Tabatabai
Cc: SeongJae Park, damon, linux-mm, linux-kernel, linux-doc,
Andrew Morton, corbet, Bijan Tabatabai
On Tue, 5 Aug 2025 20:27:52 -0500 Bijan Tabatabai <bijan311@gmail.com> wrote:
> Hi SJ,
>
>
> On Tue, Aug 5, 2025 at 7:40 PM SeongJae Park <sj@kernel.org> wrote:
> >
> > Hi Bijan,
> >
> > Subjects of patches usually be converted into lowercase when be merged into mm
> > tree. I'd suggest using lowercase even in patches stage, if those aim to be
> > merged into mm tree.
>
> Noted, thanks.
>
> [...]
> > > The commit_schemes_dests command, similar to the existing
> > > commit_schemes_quota_goals, is used to commit only the dests fields of
> > > schemes. This has a couple of benefits:
> > > 1) It is more efficient than recommitting all the DAMON data.
> > > 2) Doing a full commit resets the aggregation and ops_update intervals. If
> > > a user sends the full commit command frequently (relatively to those
> > > intervals) the aggregation and ops_update events will be prevented from
> > > triggering. Having a separate commit command side steps this problem.
> >
> > I agree the commit command of DAMON sysfs interface is inefficient, and could
> > make the infinite intervals delay problem. But, I didn't expect there could be
> > use cases that use commit feature frequently enough to make the inefficiency
> > and the intervals delay be real problems. Could you please let me know more
> > details about your use case and how severe problem DAMON is causing?
>
> In my use case, I am trying to optimize the interleave ratio of
> applications to maximize their performance without prior knowledge of
> their behavior. To do this, we take the steps of updating the ratio,
> observing how the system reacts to the change in ratio, and update the
> ratio again accordingly. Because we want to approach the ideal
> interleave ratio quickly, we update the weights frequently, motivating
> the commit_schemes_dests. Similarly, we want to observe how the system
> reacts to the change only after the change has been applied,
> motivating wait_for_schemes_apply.
Thank you for sharing these details!
It sounds like you are using DAMOS without any quota, and the target workload
has static memory mapping. Hence all migrations for the new weights can be
completed after one DAMOS schemes apply interval, and no more migration will
happen until new weights are given. And that's why you want
wait_for_schemes_apply, since when the command is finished is when all new
weights based interleaving is done. Am I understanding correctly?
>
> The consequences are not very severe. The problem can be worked around
> by either updating less frequently, at the cost of converging slower,
> or decreasing the maximum aggregation period, which from what I
> understand may affect the access monitoring behavior.
Sounds suboptimal work arounds for you...
>
> > Depending on the real problem, I'm wondering if optimizing commit command can
> > be a solution. For example, skipping the update of next_aggregation_sis and
> > next_ops_update_sis when the intervals are not changed might be able to solve
> > the intervals delay problem.
>
> This would work for my use case.
Great to hear this, and I agree. The commit operation internally uses
damon_call(), which takes up to one sampling interval. Also the migration
operation that you will wait for would also take no small time, depending on
the amount of pages to migrate. Compared to those, I think the commit speed
increase due to committing unnecessary paramters may relatively short.
deal.
Meanwhile, I was concerning the continuous next_{aggregation,ops_update_}sis
delay could be a real problem. And this option should solve the real problem.
> Another option might be to have a
> more general commit_schemes command, which may be useful to other use
> cases. I'll defer to your judgement on which would be better.
If my above theory is not wrong, I'd suggest making the commit operation
optimization. If it turns out to be not enough for your or others' use cases,
we can further consider commit_schemes.
>
> > >
> > > The wait_for_schemes_apply command causes the calling thread to wait until
> > > all schemes have been applied. It does this by calling damos_walk() with a
> > > NULL walk_fn. This can be useful, for example, if a user wants to know when
> > > new scheme parameters they've committed have been applied. Another use case
> > > could be if a user wants to record the system state every time a scheme is
> > > applied for debuging purposes.
> > >
> > > The functionality of wait_for_schemes_apply can be achieved with the
> > > existing update_schemes_tried_bytes and update_schemes_tried_regions
> > > commands. However, having a separate command avoids extra work and makes
> > > user intent clearer when used in scripts.
> >
> > I agree extra works are always better to be avoided. But is the overhead large
> > enough to be a real problem for your use case? I also agree it could make the
> > user script cleaner, but adding a kernel feature only for user scripts
> > readabilities sounds like too much, since the user script could have its own
> > abstract layers for its readability.
>
> Totally fair. I will drop wait_for_apply_schemes in any future versions.
>
> > Also, even if the new command is implemented, since the DAMOS schemes continue
> > running, the system status will keep changing. If you cannot do the recording
> > of the system state in a restricted time, the recorded information might not be
> > that reliable. So I'm not sure if you really need this strict waiting in this
> > way.
>
> Fair. That was not something I was personally using the command for,
> just another possible use case I thought of. Regardless of the
> usefulness of that, the existing commands using damos_walk would work
> well enough.
I'm glad to hear we found a way to go. As you may know,
update_schemes_tried_bytes would be more efficient, so I would suggest that
more than update_schemes_tried_regions.
If the wait is not strictly need to be accurate, maybe monitoring the DAMOS
scheme stats in auto-update mode[1] until any change is made could also be a
way. The stat update for a scheme will be done only after the scheme is
applied to all applicable regions for a round.
>
> > Could you please share more details about what you want to do with the new
> > command, and how much problem you are seeing? I'm particularly curious what
> > system state you want to record, and why you need to wait the exact time
> > interval.
>
> I mentioned this above, but I am using this to wait for new migration
> weights to be applied before monitoring how the change affects
> applications, but again, this can be done with existing commands.
Thank you again for kindly sharing your use case and participate in this
constructive discussion :)
[1] https://lore.kernel.org/20250717055448.56976-1-sj@kernel.org
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 12+ messages in thread