[RFC PATCH 1/5] mm/damon/core: add damon_target->obsolete for pin-point removal

SeongJae Park posted 5 patches 3 months, 3 weeks ago
There is a newer version of this series
[RFC PATCH 1/5] mm/damon/core: add damon_target->obsolete for pin-point removal
Posted by SeongJae Park 3 months, 3 weeks ago
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 <sj@kernel.org>
---
 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
Re: [RFC PATCH 1/5] mm/damon/core: add damon_target->obsolete for pin-point removal
Posted by Bijan Tabatabai 3 months, 3 weeks ago
Thanks for working on this SJ!

On Thu, Oct 16, 2025 at 4:47 PM SeongJae Park <sj@kernel.org> wrote:
>
> 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 <sj@kernel.org>
> ---
>  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.
>   */
Nit: Twice in the above comment you've written "obsolte" instead of "obsolete."

With that fixed
Reviewed-by: Bijan Tabatabai <bijan311@gmail.com>
for the entire series.

Thanks,
Bijan

>  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
Re: [RFC PATCH 1/5] mm/damon/core: add damon_target->obsolete for pin-point removal
Posted by SeongJae Park 3 months, 3 weeks ago
On Fri, 17 Oct 2025 09:52:16 -0500 Bijan Tabatabai <bijan311@gmail.com> wrote:

> Thanks for working on this SJ!

My pleasure!

> 
> On Thu, Oct 16, 2025 at 4:47 PM SeongJae Park <sj@kernel.org> wrote:
> >
> > 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 <sj@kernel.org>
> > ---
> >  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.
> >   */
> Nit: Twice in the above comment you've written "obsolte" instead of "obsolete."

Thank you for catching this, I will definitely fix those on the next spin.

> 
> With that fixed
> Reviewed-by: Bijan Tabatabai <bijan311@gmail.com>
> for the entire series.

Thank you!


Thanks,
SJ

[...]