On 2021/10/19 下午11:42, SeongJae Park wrote:
Hello Xin,
On Tue, 19 Oct 2021 22:26:04 +0800 Xin Hao <xhao@xxxxxxxxxxxxxxxxx> wrote:
When the ctx->adaptive_targets list is empty,
i did some test on monitor_on interface like this.
s/i/I/
thanks
echo on > /sys/kernel/debug/damon/monitor_on
[ 851.988307] damon: kdamond (5390) starts
Could you please add four spaces in the beginning of this example terminal
outputs? Also, I think confirming targets are empty like below before writing
'on' could make this example better.
# cat /sys/kernel/debug/damon/target_ids
#
or, like below.
# echo > /sys/kernel/debug/damon/target_ids
Oh! thank you so much, i will fix it.
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@xxxxxxxxxxxxxxxxx>
---
include/linux/damon.h | 1 +
mm/damon/core.c | 5 +++++
mm/damon/dbgfs.c | 10 ++++++++--
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 715dadd21f7c..06acf73fe9db 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);
+int damon_target_empty(struct damon_ctx *ctx);
Seems 'bool' would be better for the return type?
ok!
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..c651b49249e5 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);
}
+int damon_target_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 38188347d8ab..bac42dd358d1 100644
--- a/mm/damon/dbgfs.c
+++ b/mm/damon/dbgfs.c
@@ -865,9 +865,15 @@ 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_target_empty(dbgfs_ctxs[i]))
+ return -EINVAL;
+ }
err = damon_start(dbgfs_ctxs, dbgfs_nr_ctxs);
- else if (!strncmp(kbuf, "off", count))
+ } else if (!strncmp(kbuf, "off", count))
err = damon_stop(dbgfs_ctxs, dbgfs_nr_ctxs);
else
err = -EINVAL;
Please use braces for all above branches:
Ok，i will correct it.
https://docs.kernel.org/process/coding-style.html#placing-braces-and-spaces
Thanks,
SJ
--
2.31.0
--
Best Regards!
Xin Hao
