DAMON's monitoring targets parameters update function, damon_commit_targets(), is not providing a way to remove a target in the middle of existing targets list. Extend the API by adding a field to struct damon_target. If the field of a damon_commit_targets() source target is set, it indicates the matcing target on the existing targets list is obsolete. damon_commit_targets() understands that and remove those from the list, while respecting the index based matching for other non-obsolete targets. Signed-off-by: SeongJae Park --- include/linux/damon.h | 6 ++++++ mm/damon/core.c | 10 +++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/include/linux/damon.h b/include/linux/damon.h index 524dea87cac7..8a7b45b9e40d 100644 --- a/include/linux/damon.h +++ b/include/linux/damon.h @@ -92,17 +92,23 @@ struct damon_region { * @nr_regions: Number of monitoring target regions of this target. * @regions_list: Head of the monitoring target regions of this target. * @list: List head for siblings. + * @obsolete: Whether the commit destination target is obsolete. * * Each monitoring context could have multiple targets. For example, a context * for virtual memory address spaces could have multiple target processes. The * @pid should be set for appropriate &struct damon_operations including the * virtual address spaces monitoring operations. + * + * @obsolte is used only for damon_commit_targets() source targets, to specify + * the matching destination targets are obsolte. Read damon_commit_targets() + * to see how it is handled. */ struct damon_target { struct pid *pid; unsigned int nr_regions; struct list_head regions_list; struct list_head list; + bool obsolete; }; /** diff --git a/mm/damon/core.c b/mm/damon/core.c index 70e66562a1b3..3242a9573db0 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -492,6 +492,7 @@ struct damon_target *damon_new_target(void) t->nr_regions = 0; INIT_LIST_HEAD(&t->regions_list); INIT_LIST_HEAD(&t->list); + t->obsolete = false; return t; } @@ -1213,7 +1214,11 @@ static int damon_commit_targets( damon_for_each_target_safe(dst_target, next, dst) { src_target = damon_nth_target(i++, src); - if (src_target) { + /* + * If src target is obsolete, do not commit the parameters to + * the dst target, and further remove the dst target. + */ + if (src_target && !src_target->obsolete) { err = damon_commit_target( dst_target, damon_target_has_pid(dst), src_target, damon_target_has_pid(src), @@ -1236,6 +1241,9 @@ static int damon_commit_targets( damon_for_each_target_safe(src_target, next, src) { if (j++ < i) continue; + /* target to remove has no matching dst */ + if (src_target->obsolete) + return -EINVAL; new_target = damon_new_target(); if (!new_target) return -ENOMEM; -- 2.47.3 DAMON sysfs interface tests if given online parameters update request is valid, by committing those using the DAMON kernel API, to a test-purpose destination context. The test-purpose destination context is constructed using damon_new_ctx(), so it has no target, no scheme. If a source target has obsolete field set, the test-purpose commit will fail because damon_commit_targets() fails when there is a source obsolete target that cannot find its matching destination target. DAMON sysfs interface is not letting users set the field for now, so there is no problem. However, a following commit will support that. Also there could be similar future changes that making commit fails based on current context structure. Make the test purpose commit destination context similar to the current running one, by committing the running one to the test purpose context, before doing the real test-purpose commit. Signed-off-by: SeongJae Park --- mm/damon/sysfs.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c index dfdf69bdbbd3..6848437b86af 100644 --- a/mm/damon/sysfs.c +++ b/mm/damon/sysfs.c @@ -1680,6 +1680,26 @@ static int damon_sysfs_apply_inputs(struct damon_ctx *ctx, static struct damon_ctx *damon_sysfs_build_ctx( struct damon_sysfs_context *sys_ctx); +/* + * Return a new damon_ctx for testing new parameters to commit. + */ +static struct damon_ctx *damon_sysfs_new_test_ctx( + struct damon_ctx *running_ctx) +{ + struct damon_ctx *test_ctx; + int err; + + test_ctx = damon_new_ctx(); + if (!test_ctx) + return NULL; + err = damon_commit_ctx(test_ctx, running_ctx); + if (err) { + damon_destroy_ctx(test_ctx); + return NULL; + } + return test_ctx; +} + /* * damon_sysfs_commit_input() - Commit user inputs to a running kdamond. * @kdamond: The kobject wrapper for the associated kdamond. @@ -1701,7 +1721,7 @@ static int damon_sysfs_commit_input(void *data) param_ctx = damon_sysfs_build_ctx(kdamond->contexts->contexts_arr[0]); if (IS_ERR(param_ctx)) return PTR_ERR(param_ctx); - test_ctx = damon_new_ctx(); + test_ctx = damon_sysfs_new_test_ctx(kdamond->damon_ctx); if (!test_ctx) return -ENOMEM; err = damon_commit_ctx(test_ctx, param_ctx); -- 2.47.3 There is no good way to remove DAMON targets in the middle of existing targets list. It restricts efficient and flexible DAMON use cases. Improve the usability by implementing a new DAMON sysfs interface file, namely obsolete_target, under each target directory. It is connected to the obsolete field of parameters commit source targets, so allows removing arbitrary targets in the middle of existing targets list. Note that the sysfs files are not automatically updated. For example, let's suppose there are three targets in the running context, and a user removes the third target using this feature. If the user writes 'commit' to the kdamond 'state' file again, DAMON sysfs interface will again try to remove the third target. But because there is no matching target in the running context, the commit will fail. It is the user's responsibility to understand resulting DAMON internal targets list change, and construct sysfs files (using nr_targets and other sysfs files) to correctly represent it. Also note that this is arguably an improvement rather than a fix of a broken things. Reported-by: Bijan Tabatabai Closes: https://github.com/damonitor/damo/issues/36 Signed-off-by: SeongJae Park --- mm/damon/sysfs.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c index 6848437b86af..26bc658ad873 100644 --- a/mm/damon/sysfs.c +++ b/mm/damon/sysfs.c @@ -212,6 +212,7 @@ struct damon_sysfs_target { struct kobject kobj; struct damon_sysfs_regions *regions; int pid; + bool obsolete; }; static struct damon_sysfs_target *damon_sysfs_target_alloc(void) @@ -263,6 +264,29 @@ static ssize_t pid_target_store(struct kobject *kobj, return count; } +static ssize_t obsolete_target_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) +{ + struct damon_sysfs_target *target = container_of(kobj, + struct damon_sysfs_target, kobj); + + return sysfs_emit(buf, "%c\n", target->obsolete ? 'Y' : 'N'); +} + +static ssize_t obsolete_target_store(struct kobject *kobj, + struct kobj_attribute *attr, const char *buf, size_t count) +{ + struct damon_sysfs_target *target = container_of(kobj, + struct damon_sysfs_target, kobj); + bool obsolete; + int err = kstrtobool(buf, &obsolete); + + if (err) + return err; + target->obsolete = obsolete; + return count; +} + static void damon_sysfs_target_release(struct kobject *kobj) { kfree(container_of(kobj, struct damon_sysfs_target, kobj)); @@ -271,8 +295,12 @@ static void damon_sysfs_target_release(struct kobject *kobj) static struct kobj_attribute damon_sysfs_target_pid_attr = __ATTR_RW_MODE(pid_target, 0600); +static struct kobj_attribute damon_sysfs_target_obsolete_attr = + __ATTR_RW_MODE(obsolete_target, 0600); + static struct attribute *damon_sysfs_target_attrs[] = { &damon_sysfs_target_pid_attr.attr, + &damon_sysfs_target_obsolete_attr.attr, NULL, }; ATTRIBUTE_GROUPS(damon_sysfs_target); @@ -1574,6 +1602,7 @@ static int damon_sysfs_add_target(struct damon_sysfs_target *sys_target, /* caller will destroy targets */ return -EINVAL; } + t->obsolete = sys_target->obsolete; return damon_sysfs_set_regions(t, sys_target->regions, ctx->min_sz_region); } -- 2.47.3 Document the newly added obsolete_target DAMON sysfs file. Signed-off-by: SeongJae Park --- Documentation/admin-guide/mm/damon/usage.rst | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Documentation/admin-guide/mm/damon/usage.rst b/Documentation/admin-guide/mm/damon/usage.rst index 98958975604d..d8a3d6e740b3 100644 --- a/Documentation/admin-guide/mm/damon/usage.rst +++ b/Documentation/admin-guide/mm/damon/usage.rst @@ -67,7 +67,7 @@ comma (","). │ │ │ │ │ │ │ intervals_goal/access_bp,aggrs,min_sample_us,max_sample_us │ │ │ │ │ │ nr_regions/min,max │ │ │ │ │ :ref:`targets `/nr_targets - │ │ │ │ │ │ :ref:`0 `/pid_target + │ │ │ │ │ │ :ref:`0 `/pid_target,obsolete_target │ │ │ │ │ │ │ :ref:`regions `/nr_regions │ │ │ │ │ │ │ │ :ref:`0 `/start,end │ │ │ │ │ │ │ │ ... @@ -264,13 +264,20 @@ to ``N-1``. Each directory represents each monitoring target. targets// ------------ -In each target directory, one file (``pid_target``) and one directory -(``regions``) exist. +In each target directory, two files (``pid_target`` and ``obsolete_target``) +and one directory (``regions``) exist. If you wrote ``vaddr`` to the ``contexts//operations``, each target should be a process. You can specify the process to DAMON by writing the pid of the process to the ``pid_target`` file. +Users can selectively remove targets in the middle of the targets array by +writing non-zero value to ``obsolete_target`` file and committing it (writing +``commit`` to ``state`` file). DAMON will remove the matching targets from its +internal targets array. Users are responsible to construct target directories +again, so that those correctly represent the changed internal targets array. + + .. _sysfs_regions: targets//regions -- 2.47.3 Update DAMON ABI document for the newly added obsolete_target DAMON sysfs file. Signed-off-by: SeongJae Park --- Documentation/ABI/testing/sysfs-kernel-mm-damon | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-damon b/Documentation/ABI/testing/sysfs-kernel-mm-damon index dce6c2cda4e8..4fb8b7a6d625 100644 --- a/Documentation/ABI/testing/sysfs-kernel-mm-damon +++ b/Documentation/ABI/testing/sysfs-kernel-mm-damon @@ -164,6 +164,13 @@ Description: Writing to and reading from this file sets and gets the pid of the target process if the context is for virtual address spaces monitoring, respectively. +What: /sys/kernel/mm/damon/admin/kdamonds//contexts//targets//obsolete_target +Date: Oct 2025 +Contact: SeongJae Park +Description: Writing to and reading from this file sets and gets the + obsoleteness of the matching parameters commit destination + target. + What: /sys/kernel/mm/damon/admin/kdamonds//contexts//targets//regions/nr_regions Date: Mar 2022 Contact: SeongJae Park -- 2.47.3