[PATCH V4 0/2] mm/damon: Fix some small bugs

[PATCH V4 0/2] mm/damon: Fix some small bugs

[Xin Hao]

V3 -> V4
  -- patch[1] add Reviewed-by: SeongJae Park <sj@kernel.org>
  -- patch[2]
	Call kfree(kbuf) before return,
	Fix test example in commit info.
	 # cat /sys/kernel/debug/damon/target_ids
	 #
	 # echo on > /sys/kernel/debug/damon/monitor_on
	 # damon: kdamond (5390) starts

V2 -> V3
  -- Fix some formatting issues
  -- Modify the return value of 'damon_targets_empty' to bool type

V1 -> V2
  -- As SeongJae Park advised, put 'ctx->adaptive_targets
  check' into 'dbgfs_monitor_on_write()'

Xin Hao (2):
  mm/damon: Remove unnecessary variable initialization
  mm/damon/dbgfs: Add adaptive_targets list check before enable
    monitor_on

 include/linux/damon.h |  1 +
 mm/damon/core.c       |  5 +++++
 mm/damon/dbgfs.c      | 15 ++++++++++++---
 mm/damon/vaddr.c      |  2 +-
 4 files changed, 19 insertions(+), 4 deletions(-)

--
2.31.0

[PATCH V4 1/2] mm/damon: Remove unnecessary variable initialization

[Xin Hao]

In 'damon_va_apply_three_regions', There is no
need to set variable 'i' as 0

Signed-off-by: Xin Hao <xhao@linux.alibaba.com>
Reviewed-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/vaddr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index b8ba44f69db6..90be463efff7 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -305,7 +305,7 @@ static void damon_va_apply_three_regions(struct damon_target *t,
 		struct damon_addr_range bregions[3])
 {
 	struct damon_region *r, *next;
-	unsigned int i = 0;
+	unsigned int i;

 	/* Remove regions which are not in the three big regions now */
 	damon_for_each_region_safe(r, next, t) {
--
2.31.0

[PATCH V4 2/2] mm/damon/dbgfs: Add adaptive_targets list check before enable monitor_on

[Xin Hao]

When the ctx->adaptive_targets list is empty,
I did some test on monitor_on interface like this.

    # cat /sys/kernel/debug/damon/target_ids
    #
    # echo on > /sys/kernel/debug/damon/monitor_on
    # damon: kdamond (5390) starts

Though the ctx->adaptive_targets list is empty, but the
kthread_run still be called, and the kdamond.x thread still
be created, this is meaningless.

So there adds a judgment in 'dbgfs_monitor_on_write',
if the ctx->adaptive_targets list is empty, return -EINVAL.

Signed-off-by: Xin Hao <xhao@linux.alibaba.com>
---
 include/linux/damon.h |  1 +
 mm/damon/core.c       |  5 +++++
 mm/damon/dbgfs.c      | 15 ++++++++++++---
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 715dadd21f7c..4fce5f1f6dad 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -316,6 +316,7 @@ void damon_destroy_scheme(struct damos *s);

 struct damon_target *damon_new_target(unsigned long id);
 void damon_add_target(struct damon_ctx *ctx, struct damon_target *t);
+bool damon_targets_empty(struct damon_ctx *ctx);
 void damon_free_target(struct damon_target *t);
 void damon_destroy_target(struct damon_target *t);
 unsigned int damon_nr_regions(struct damon_target *t);
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 2f6785737902..c3a1374dbe0b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -156,6 +156,11 @@ void damon_add_target(struct damon_ctx *ctx, struct damon_target *t)
 	list_add_tail(&t->list, &ctx->adaptive_targets);
 }

+bool damon_targets_empty(struct damon_ctx *ctx)
+{
+	return list_empty(&ctx->adaptive_targets);
+}
+
 static void damon_del_target(struct damon_target *t)
 {
 	list_del(&t->list);
diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c
index c90988a20fa4..a02cf6bee8e8 100644
--- a/mm/damon/dbgfs.c
+++ b/mm/damon/dbgfs.c
@@ -861,12 +861,21 @@ static ssize_t dbgfs_monitor_on_write(struct file *file,
 		return -EINVAL;
 	}

-	if (!strncmp(kbuf, "on", count))
+	if (!strncmp(kbuf, "on", count)) {
+		int i;
+
+		for (i = 0; i < dbgfs_nr_ctxs; i++) {
+			if (damon_targets_empty(dbgfs_ctxs[i])) {
+				kfree(kbuf);
+				return -EINVAL;
+			}
+		}
 		ret = damon_start(dbgfs_ctxs, dbgfs_nr_ctxs);
-	else if (!strncmp(kbuf, "off", count))
+	} else if (!strncmp(kbuf, "off", count)) {
 		ret = damon_stop(dbgfs_ctxs, dbgfs_nr_ctxs);
-	else
+	} else {
 		ret = -EINVAL;
+	}

 	if (!ret)
 		ret = count;
--
2.31.0

Re: [PATCH V4 2/2] mm/damon/dbgfs: Add adaptive_targets list check before enable monitor_on

[SeongJae Park]

On Wed, 20 Oct 2021 17:03:01 +0800 Xin Hao <xhao@linux.alibaba.com> wrote:

> When the ctx->adaptive_targets list is empty,
> I did some test on monitor_on interface like this.
> 
>     # cat /sys/kernel/debug/damon/target_ids
>     #
>     # echo on > /sys/kernel/debug/damon/monitor_on
>     # damon: kdamond (5390) starts
> 
> Though the ctx->adaptive_targets list is empty, but the
> kthread_run still be called, and the kdamond.x thread still
> be created, this is meaningless.
> 
> So there adds a judgment in 'dbgfs_monitor_on_write',
> if the ctx->adaptive_targets list is empty, return -EINVAL.
> 
> Signed-off-by: Xin Hao <xhao@linux.alibaba.com>

Reviewed-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

> ---
>  include/linux/damon.h |  1 +
>  mm/damon/core.c       |  5 +++++
>  mm/damon/dbgfs.c      | 15 ++++++++++++---
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 715dadd21f7c..4fce5f1f6dad 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -316,6 +316,7 @@ void damon_destroy_scheme(struct damos *s);
> 
>  struct damon_target *damon_new_target(unsigned long id);
>  void damon_add_target(struct damon_ctx *ctx, struct damon_target *t);
> +bool damon_targets_empty(struct damon_ctx *ctx);
>  void damon_free_target(struct damon_target *t);
>  void damon_destroy_target(struct damon_target *t);
>  unsigned int damon_nr_regions(struct damon_target *t);
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 2f6785737902..c3a1374dbe0b 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -156,6 +156,11 @@ void damon_add_target(struct damon_ctx *ctx, struct damon_target *t)
>  	list_add_tail(&t->list, &ctx->adaptive_targets);
>  }
> 
> +bool damon_targets_empty(struct damon_ctx *ctx)
> +{
> +	return list_empty(&ctx->adaptive_targets);
> +}
> +
>  static void damon_del_target(struct damon_target *t)
>  {
>  	list_del(&t->list);
> diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c
> index c90988a20fa4..a02cf6bee8e8 100644
> --- a/mm/damon/dbgfs.c
> +++ b/mm/damon/dbgfs.c
> @@ -861,12 +861,21 @@ static ssize_t dbgfs_monitor_on_write(struct file *file,
>  		return -EINVAL;
>  	}
> 
> -	if (!strncmp(kbuf, "on", count))
> +	if (!strncmp(kbuf, "on", count)) {
> +		int i;
> +
> +		for (i = 0; i < dbgfs_nr_ctxs; i++) {
> +			if (damon_targets_empty(dbgfs_ctxs[i])) {
> +				kfree(kbuf);
> +				return -EINVAL;
> +			}
> +		}
>  		ret = damon_start(dbgfs_ctxs, dbgfs_nr_ctxs);
> -	else if (!strncmp(kbuf, "off", count))
> +	} else if (!strncmp(kbuf, "off", count)) {
>  		ret = damon_stop(dbgfs_ctxs, dbgfs_nr_ctxs);
> -	else
> +	} else {
>  		ret = -EINVAL;
> +	}
> 
>  	if (!ret)
>  		ret = count;
> --
> 2.31.0