2023-11-28 07:35:00

by cuiyangpei

[permalink] [raw]
Subject: [PATCH 1/2] mm/damon/sysfs: Implement recording feature

The user space users can control DAMON and get the monitoring results
via implements 'recording' feature in 'damon-sysfs'. The feature
can be used via 'record' and 'state' file in the '<sysfs>/kernel/mm/
damon/admin/kdamonds/N/' directory.

The file allows users to record monitored access patterns in a text
file. Firstly, users set the size of the buffer and the path of the
result file by writing to the ``record`` file. Then the recorded
results are first written in an in-memory buffer and flushed the
recorded results to a file in batch by writing 'record' to the
``state`` file.

For example, below commands set the buffer to be 4 KiB and the result
to be saved in ``/damon.txt``. ::

# cd <sysfs>/kernel/mm/damon/admin/kdamonds/N
# echo "4096 /damon.txt" > record
# echo "record" > state

Signed-off-by: cuiyangpei <[email protected]>
---
.../ABI/testing/sysfs-kernel-mm-damon | 20 +-
include/linux/damon.h | 11 +
mm/damon/sysfs.c | 282 ++++++++++++++++++
3 files changed, 307 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-damon b/Documentation/ABI/testing/sysfs-kernel-mm-damon
index b35649a46a2f..819534dcfb6c 100644
--- a/Documentation/ABI/testing/sysfs-kernel-mm-damon
+++ b/Documentation/ABI/testing/sysfs-kernel-mm-damon
@@ -25,15 +25,23 @@ Description: Writing 'on' or 'off' to this file makes the kdamond starts or
stops, respectively. Reading the file returns the keywords
based on the current status. Writing 'commit' to this file
makes the kdamond reads the user inputs in the sysfs files
- except 'state' 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
+ except 'state' again. Writing 'record' to this file makes the
+ kdamond saves the monitoring results to file specified by the
+ /record file. 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.

+What: /sys/kernel/mm/damon/admin/kdamonds/<K>/record
+Date: Nov 2023
+Contact: Ping Xiong <[email protected]>
+Description: Writing a string '4096 /damon.txt' to this file makes the
+ user to record monitored access patterns in a text file.
+
What: /sys/kernel/mm/damon/admin/kdamonds/<K>/pid
Date: Mar 2022
Contact: SeongJae Park <[email protected]>
diff --git a/include/linux/damon.h b/include/linux/damon.h
index ab2f17d9926b..6495513cc6de 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -19,6 +19,17 @@
/* Max priority score for DAMON-based operation schemes */
#define DAMOS_MAX_SCORE (99)

+#define MIN_RECORD_BUFFER_LEN 1024
+#define MAX_RECORD_BUFFER_LEN (4 * 1024 * 1024)
+#define MAX_RFILE_PATH_LEN 256
+
+struct sysfs_recorder {
+ unsigned char *rbuf;
+ unsigned int rbuf_len;
+ unsigned int rbuf_offset;
+ char *rfile_path;
+};
+
/* Get a random number in [l, r) */
static inline unsigned long damon_rand(unsigned long l, unsigned long r)
{
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index e27846708b5a..7a7d41e609e3 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -994,6 +994,8 @@ enum damon_sysfs_cmd {
DAMON_SYSFS_CMD_OFF,
/* @DAMON_SYSFS_CMD_COMMIT: Update kdamond inputs. */
DAMON_SYSFS_CMD_COMMIT,
+ /* @DAMON_SYSFS_CMD_RECORD: Save the monitoring results to file. */
+ DAMON_SYSFS_CMD_RECORD,
/*
* @DAMON_SYSFS_CMD_UPDATE_SCHEMES_STATS: Update scheme stats sysfs
* files.
@@ -1025,6 +1027,7 @@ static const char * const damon_sysfs_cmd_strs[] = {
"on",
"off",
"commit",
+ "record",
"update_schemes_stats",
"update_schemes_tried_bytes",
"update_schemes_tried_regions",
@@ -1349,6 +1352,160 @@ static int damon_sysfs_commit_input(struct damon_sysfs_kdamond *kdamond)
kdamond->contexts->contexts_arr[0]);
}

+/*
+ * Flush the content in the result buffer to the result file
+ */
+static int sysfs_flush_rbuffer(struct sysfs_recorder *rec)
+{
+ ssize_t sz;
+ loff_t pos = 0;
+ struct file *rfile;
+
+ if (!rec->rbuf_offset)
+ return 0;
+
+ rfile = filp_open(rec->rfile_path,
+ O_CREAT | O_RDWR | O_APPEND | O_LARGEFILE, 0644);
+ if (IS_ERR(rfile)) {
+ pr_err("Cannot open the result file %s\n",
+ rec->rfile_path);
+ return PTR_ERR(rfile);
+ }
+
+ while (rec->rbuf_offset) {
+ sz = kernel_write(rfile, rec->rbuf, rec->rbuf_offset, &pos);
+ if (sz < 0) {
+ filp_close(rfile, NULL);
+ return sz;
+ }
+ rec->rbuf_offset -= sz;
+ }
+ filp_close(rfile, NULL);
+
+ return 0;
+}
+
+/*
+ * Write a data into the result buffer
+ */
+static void sysfs_write_rbuf(struct damon_ctx *ctx, char *data, int size)
+{
+ struct sysfs_recorder *rec = ctx->callback.private;
+
+ if (!rec->rbuf_len || !rec->rbuf || !rec->rfile_path)
+ return;
+ if (rec->rbuf_offset + size > rec->rbuf_len)
+ sysfs_flush_rbuffer(ctx->callback.private);
+ if (rec->rbuf_offset + size > rec->rbuf_len) {
+ pr_warn("%s: flush failed, or wrong size given(%u, %d)\n",
+ __func__, rec->rbuf_offset, size);
+ return;
+ }
+
+ memcpy(&rec->rbuf[rec->rbuf_offset], data, size);
+ rec->rbuf_offset += size;
+}
+
+static unsigned int nr_damon_targets(struct damon_ctx *ctx)
+{
+ struct damon_target *t;
+ unsigned int nr_targets = 0;
+ int count = 0;
+
+ damon_for_each_target(t, ctx) {
+ count++;
+ nr_targets++;
+ }
+
+ return nr_targets;
+}
+
+/*
+ * Store the aggregated monitoring results to the result buffer
+ *
+ * The format for the result buffer is as below:
+ *
+ * <time> <number of targets>
+ *
+ * target info: <pid> <number of regions>
+ * region info: <start address> <region size> <nr_accesses>
+ */
+static int damon_flush_aggregation(struct damon_ctx *c)
+{
+ struct damon_target *t;
+ struct timespec64 now;
+ struct task_struct *tsk;
+ int tsk_pid = -1;
+ unsigned int nr = 0;
+ char buf[128];
+ int rc = 0;
+
+ memset(buf, 0, sizeof(buf));
+ ktime_get_coarse_ts64(&now);
+ nr = nr_damon_targets(c);
+ rc = scnprintf(buf, sizeof(buf), "time: %lld.%09ld, nr: %u\n",
+ (long long)now.tv_sec, now.tv_nsec, nr);
+ if (!rc)
+ return -ENOMEM;
+
+ sysfs_write_rbuf(c, buf, rc);
+ memset(buf, 0, sizeof(buf));
+
+ damon_for_each_target(t, c) {
+ struct damon_region *r;
+
+ tsk = get_pid_task(t->pid, PIDTYPE_PID);
+ tsk_pid = tsk->pid;
+ nr = damon_nr_regions(t);
+ rc = scnprintf(buf, sizeof(buf), "pid: %d, nr: %u\n", tsk_pid, nr);
+ if (!rc)
+ return -ENOMEM;
+
+ sysfs_write_rbuf(c, buf, rc);
+ memset(buf, 0, sizeof(buf));
+
+ damon_for_each_region(r, t) {
+
+ rc = scnprintf(buf, sizeof(buf), "%lu, %lu, %d\n",
+ r->ar.start, r->ar.end - r->ar.start, r->nr_accesses);
+ if (!rc)
+ return -ENOMEM;
+
+ sysfs_write_rbuf(c, buf, rc);
+ memset(buf, 0, sizeof(buf));
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * damon_sysfs_record() - Save the monitoring results to file.
+ * @kdamond: The kobject wrapper for the associated kdamond.
+ *
+ * If the sysfs input is wrong, the kdamond will be terminated.
+ */
+static int damon_sysfs_record(struct damon_sysfs_kdamond *kdamond)
+{
+ struct damon_ctx *ctx = kdamond->damon_ctx;
+ struct sysfs_recorder *rec = ctx->callback.private;
+ int err = 0;
+
+ if (!damon_sysfs_kdamond_running(kdamond))
+ return -EINVAL;
+ /* TODO: Support multiple contexts per kdamond */
+ if (kdamond->contexts->nr != 1)
+ return -EINVAL;
+
+ err = damon_flush_aggregation(ctx);
+ if (!err) {
+ if (rec->rbuf_offset)
+ err = sysfs_flush_rbuffer(rec);
+ }
+
+ return err;
+}
+
/*
* damon_sysfs_cmd_request_callback() - DAMON callback for handling requests.
* @c: The DAMON context of the callback.
@@ -1371,6 +1528,9 @@ static int damon_sysfs_cmd_request_callback(struct damon_ctx *c, bool active)
if (!kdamond || kdamond->damon_ctx != c)
goto out;
switch (damon_sysfs_cmd_request.cmd) {
+ case DAMON_SYSFS_CMD_RECORD:
+ err = damon_sysfs_record(kdamond);
+ break;
case DAMON_SYSFS_CMD_UPDATE_SCHEMES_STATS:
err = damon_sysfs_upd_schemes_stats(kdamond);
break;
@@ -1433,6 +1593,65 @@ static int damon_sysfs_after_aggregation(struct damon_ctx *c)
return damon_sysfs_cmd_request_callback(c, true);
}

+/*
+ * sysfs_set_recording() - Set attributes for the recording.
+ * @ctx: target kdamond context
+ * @rbuf_len: length of the result buffer
+ * @rfile_path: path to the monitor result files
+ *
+ * Setting 'rbuf_len' 0 disables recording.
+ *
+ * This function should not be called while the kdamond is running.
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+static int sysfs_set_recording(struct damon_ctx *ctx,
+ unsigned int rbuf_len, char *rfile_path)
+{
+ struct sysfs_recorder *recorder;
+ size_t rfile_path_len;
+
+ if (rbuf_len && (rbuf_len > MAX_RECORD_BUFFER_LEN ||
+ rbuf_len < MIN_RECORD_BUFFER_LEN)) {
+ pr_err("result buffer size (%u) is out of [%d,%d]\n",
+ rbuf_len, MIN_RECORD_BUFFER_LEN,
+ MAX_RECORD_BUFFER_LEN);
+ return -EINVAL;
+ }
+ rfile_path_len = strnlen(rfile_path, MAX_RFILE_PATH_LEN);
+ if (rfile_path_len >= MAX_RFILE_PATH_LEN) {
+ pr_err("too long (>%d) result file path %s\n",
+ MAX_RFILE_PATH_LEN, rfile_path);
+ return -EINVAL;
+ }
+
+ recorder = ctx->callback.private;
+ if (!recorder) {
+ recorder = kzalloc(sizeof(*recorder), GFP_KERNEL);
+ if (!recorder)
+ return -ENOMEM;
+ ctx->callback.private = recorder;
+ }
+
+ recorder->rbuf_len = rbuf_len;
+ kfree(recorder->rbuf);
+ recorder->rbuf = NULL;
+ kfree(recorder->rfile_path);
+ recorder->rfile_path = NULL;
+
+ if (rbuf_len) {
+ recorder->rbuf = kvmalloc(rbuf_len, GFP_KERNEL);
+ if (!recorder->rbuf)
+ return -ENOMEM;
+ }
+ recorder->rfile_path = kmalloc(rfile_path_len + 1, GFP_KERNEL);
+ if (!recorder->rfile_path)
+ return -ENOMEM;
+ strscpy(recorder->rfile_path, rfile_path, rfile_path_len + 1);
+
+ return 0;
+}
+
static struct damon_ctx *damon_sysfs_build_ctx(
struct damon_sysfs_context *sys_ctx)
{
@@ -1442,6 +1661,12 @@ static struct damon_ctx *damon_sysfs_build_ctx(
if (!ctx)
return ERR_PTR(-ENOMEM);

+ err = sysfs_set_recording(ctx, 0, "none");
+ if (err) {
+ damon_destroy_ctx(ctx);
+ return ERR_PTR(err);
+ }
+
err = damon_sysfs_apply_inputs(ctx, sys_ctx);
if (err) {
damon_destroy_ctx(ctx);
@@ -1599,6 +1824,59 @@ static ssize_t pid_show(struct kobject *kobj,
return sysfs_emit(buf, "%d\n", pid);
}

+static ssize_t record_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct damon_sysfs_kdamond *kdamond = container_of(kobj,
+ struct damon_sysfs_kdamond, kobj);
+ struct damon_ctx *ctx;
+ struct sysfs_recorder *rec;
+ int len = 0;
+
+ if (!mutex_trylock(&damon_sysfs_lock))
+ return -EBUSY;
+ ctx = kdamond->damon_ctx;
+ if (!ctx)
+ goto out;
+ rec = ctx->callback.private;
+ len = sysfs_emit(buf, "%u %s\n", rec->rbuf_len, rec->rfile_path);
+
+out:
+ mutex_unlock(&damon_sysfs_lock);
+ return len;
+}
+
+static ssize_t record_store(struct kobject *kobj,
+ struct kobj_attribute *attr, const char *buf, size_t count)
+{
+ struct damon_sysfs_kdamond *kdamond = container_of(kobj,
+ struct damon_sysfs_kdamond, kobj);
+ struct damon_ctx *ctx;
+ unsigned int rbuf_len;
+ char rfile_path[MAX_RFILE_PATH_LEN];
+ ssize_t ret = count;
+ int err;
+
+ if (sscanf(buf, "%10u %128s", &rbuf_len, rfile_path) != 2) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!mutex_trylock(&damon_sysfs_lock))
+ return -EBUSY;
+ ctx = kdamond->damon_ctx;
+ if (!ctx)
+ goto unlock_out;
+
+ err = sysfs_set_recording(ctx, rbuf_len, rfile_path);
+ if (err)
+ ret = err;
+unlock_out:
+ mutex_unlock(&damon_sysfs_lock);
+out:
+ return ret;
+}
+
static void damon_sysfs_kdamond_release(struct kobject *kobj)
{
struct damon_sysfs_kdamond *kdamond = container_of(kobj,
@@ -1615,9 +1893,13 @@ static struct kobj_attribute damon_sysfs_kdamond_state_attr =
static struct kobj_attribute damon_sysfs_kdamond_pid_attr =
__ATTR_RO_MODE(pid, 0400);

+static struct kobj_attribute damon_sysfs_kdamond_record_attr =
+ __ATTR_RW_MODE(record, 0600);
+
static struct attribute *damon_sysfs_kdamond_attrs[] = {
&damon_sysfs_kdamond_state_attr.attr,
&damon_sysfs_kdamond_pid_attr.attr,
+ &damon_sysfs_kdamond_record_attr.attr,
NULL,
};
ATTRIBUTE_GROUPS(damon_sysfs_kdamond);
--
2.43.0


2023-11-28 07:36:42

by cuiyangpei

[permalink] [raw]
Subject: [PATCH 2/2] mm/damon/core: add sysfs nodes to set last_nr_accesses weight

The date access frequency is cleared every time the aggregation
interval is reached. In order to refer to the past time access
frequency, the current access frequency can be calculated by
setting the weight of the last access frequency.

Signed-off-by: cuiyangpei <[email protected]>
---
.../ABI/testing/sysfs-kernel-mm-damon | 6 ++++
include/linux/damon.h | 3 ++
mm/damon/core.c | 29 +++++++++++++++++
mm/damon/sysfs.c | 31 +++++++++++++++++++
4 files changed, 69 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-damon b/Documentation/ABI/testing/sysfs-kernel-mm-damon
index 819534dcfb6c..8367e26bf4da 100644
--- a/Documentation/ABI/testing/sysfs-kernel-mm-damon
+++ b/Documentation/ABI/testing/sysfs-kernel-mm-damon
@@ -74,6 +74,12 @@ Description: Writing a keyword for a monitoring operations set ('vaddr' for
Note that only the operations sets that listed in
'avail_operations' file are valid inputs.

+What: /sys/kernel/mm/damon/admin/kdamonds/<K>/contexts/<C>/monitoring_attrs/last_nr_accesses_weight
+Date: Nov 2023
+Contact: Ping Xiong <[email protected]>
+Description: Writing a value to this file sets last_nr_accesses weight
+ of the DAMON context. Reading this file returns the value.
+
What: /sys/kernel/mm/damon/admin/kdamonds/<K>/contexts/<C>/monitoring_attrs/intervals/sample_us
Date: Mar 2022
Contact: SeongJae Park <[email protected]>
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 6495513cc6de..71e67a9d0996 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -18,6 +18,8 @@
#define DAMON_MIN_REGION PAGE_SIZE
/* Max priority score for DAMON-based operation schemes */
#define DAMOS_MAX_SCORE (99)
+/* set last_nr_accesses weight */
+#define LAST_NR_ACCESSES_WEIGHT 0

#define MIN_RECORD_BUFFER_LEN 1024
#define MAX_RECORD_BUFFER_LEN (4 * 1024 * 1024)
@@ -522,6 +524,7 @@ struct damon_attrs {
unsigned long ops_update_interval;
unsigned long min_nr_regions;
unsigned long max_nr_regions;
+ unsigned int last_nr_accesses_weight;
};

/**
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 630077d95dc6..230afcceea22 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1142,6 +1142,34 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
}
}

+static unsigned int __calculate_nr_accesses(struct damon_ctx *c, struct damon_region *r)
+{
+ unsigned int rem_old, rem_new;
+ unsigned int res;
+ unsigned int weight = c->attrs.last_nr_accesses_weight;
+
+ res = div_u64_rem(r->nr_accesses, 100, &rem_new) * (100 - weight)
+ + div_u64_rem(r->last_nr_accesses, 100, &rem_old) * weight;
+
+ if (rem_new)
+ res += rem_new * (100 - weight) / 100;
+ if (rem_old)
+ res += rem_old * weight / 100;
+
+ return res;
+}
+
+static void kdamon_update_nr_accesses(struct damon_ctx *c)
+{
+ struct damon_target *t;
+ struct damon_region *r;
+
+ damon_for_each_target(t, c) {
+ damon_for_each_region(r, t)
+ r->nr_accesses = __calculate_nr_accesses(c, r);
+ }
+}
+
/*
* Merge two adjacent regions into one region
*/
@@ -1469,6 +1497,7 @@ static int kdamond_fn(void *data)
max_nr_accesses = ctx->ops.check_accesses(ctx);

if (ctx->passed_sample_intervals == next_aggregation_sis) {
+ kdamon_update_nr_accesses(ctx);
kdamond_merge_regions(ctx,
max_nr_accesses / 10,
sz_limit);
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 7a7d41e609e3..2946b0e91ad8 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -544,6 +544,7 @@ struct damon_sysfs_attrs {
struct kobject kobj;
struct damon_sysfs_intervals *intervals;
struct damon_sysfs_ul_range *nr_regions_range;
+ unsigned int last_nr_accesses_weight;
};

static struct damon_sysfs_attrs *damon_sysfs_attrs_alloc(void)
@@ -553,6 +554,7 @@ static struct damon_sysfs_attrs *damon_sysfs_attrs_alloc(void)
if (!attrs)
return NULL;
attrs->kobj = (struct kobject){};
+ attrs->last_nr_accesses_weight = LAST_NR_ACCESSES_WEIGHT;
return attrs;
}

@@ -602,12 +604,40 @@ static void damon_sysfs_attrs_rm_dirs(struct damon_sysfs_attrs *attrs)
kobject_put(&attrs->intervals->kobj);
}

+static ssize_t last_nr_accesses_weight_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct damon_sysfs_attrs *attrs = container_of(kobj,
+ struct damon_sysfs_attrs, kobj);
+
+ return sysfs_emit(buf, "%u\n", attrs->last_nr_accesses_weight);
+}
+
+static ssize_t last_nr_accesses_weight_store(struct kobject *kobj,
+ struct kobj_attribute *attr, const char *buf, size_t count)
+{
+ struct damon_sysfs_attrs *attrs = container_of(kobj,
+ struct damon_sysfs_attrs, kobj);
+ int err = kstrtoint(buf, 0, &attrs->last_nr_accesses_weight);
+
+ if (err)
+ return -EINVAL;
+ if (attrs->last_nr_accesses_weight > 100)
+ return -EINVAL;
+
+ return count;
+}
+
static void damon_sysfs_attrs_release(struct kobject *kobj)
{
kfree(container_of(kobj, struct damon_sysfs_attrs, kobj));
}

+static struct kobj_attribute damon_sysfs_attrs_last_nr_accesses_weight_attr =
+ __ATTR_RW_MODE(last_nr_accesses_weight, 0600);
+
static struct attribute *damon_sysfs_attrs_attrs[] = {
+ &damon_sysfs_attrs_last_nr_accesses_weight_attr.attr,
NULL,
};
ATTRIBUTE_GROUPS(damon_sysfs_attrs);
@@ -1083,6 +1113,7 @@ static int damon_sysfs_set_attrs(struct damon_ctx *ctx,
.ops_update_interval = sys_intervals->update_us,
.min_nr_regions = sys_nr_regions->min,
.max_nr_regions = sys_nr_regions->max,
+ .last_nr_accesses_weight = sys_attrs->last_nr_accesses_weight,
};
return damon_set_attrs(ctx, &attrs);
}
--
2.43.0

2023-11-28 16:12:03

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/2] mm/damon/sysfs: Implement recording feature

Hi cuiyangpei,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-everything]

url: https://github.com/intel-lab-lkp/linux/commits/cuiyangpei/mm-damon-core-add-sysfs-nodes-to-set-last_nr_accesses-weight/20231128-194153
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20231128073440.11894-1-cuiyangpei%40xiaomi.com
patch subject: [PATCH 1/2] mm/damon/sysfs: Implement recording feature
config: i386-buildonly-randconfig-002-20231128 (https://download.01.org/0day-ci/archive/20231129/[email protected]/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231129/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> mm/damon/sysfs.c:1415:6: warning: variable 'count' set but not used [-Wunused-but-set-variable]
int count = 0;
^
1 warning generated.


vim +/count +1415 mm/damon/sysfs.c

1410
1411 static unsigned int nr_damon_targets(struct damon_ctx *ctx)
1412 {
1413 struct damon_target *t;
1414 unsigned int nr_targets = 0;
> 1415 int count = 0;
1416
1417 damon_for_each_target(t, ctx) {
1418 count++;
1419 nr_targets++;
1420 }
1421
1422 return nr_targets;
1423 }
1424

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-11-28 16:22:27

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/2] mm/damon/sysfs: Implement recording feature

Hi cuiyangpei,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url: https://github.com/intel-lab-lkp/linux/commits/cuiyangpei/mm-damon-core-add-sysfs-nodes-to-set-last_nr_accesses-weight/20231128-194153
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20231128073440.11894-1-cuiyangpei%40xiaomi.com
patch subject: [PATCH 1/2] mm/damon/sysfs: Implement recording feature
config: i386-buildonly-randconfig-001-20231128 (https://download.01.org/0day-ci/archive/20231129/[email protected]/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231129/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

mm/damon/sysfs.c:1415:6: warning: variable 'count' set but not used [-Wunused-but-set-variable]
int count = 0;
^
In file included from mm/damon/sysfs.c:2139:
>> mm/damon/sysfs-test.h:15:21: error: redefinition of 'nr_damon_targets'
static unsigned int nr_damon_targets(struct damon_ctx *ctx)
^
mm/damon/sysfs.c:1411:21: note: previous definition is here
static unsigned int nr_damon_targets(struct damon_ctx *ctx)
^
1 warning and 1 error generated.


vim +/nr_damon_targets +15 mm/damon/sysfs-test.h

b8ee5575f763c2 SeongJae Park 2023-10-22 14
b8ee5575f763c2 SeongJae Park 2023-10-22 @15 static unsigned int nr_damon_targets(struct damon_ctx *ctx)
b8ee5575f763c2 SeongJae Park 2023-10-22 16 {
b8ee5575f763c2 SeongJae Park 2023-10-22 17 struct damon_target *t;
b8ee5575f763c2 SeongJae Park 2023-10-22 18 unsigned int nr_targets = 0;
b8ee5575f763c2 SeongJae Park 2023-10-22 19
b8ee5575f763c2 SeongJae Park 2023-10-22 20 damon_for_each_target(t, ctx)
b8ee5575f763c2 SeongJae Park 2023-10-22 21 nr_targets++;
b8ee5575f763c2 SeongJae Park 2023-10-22 22
b8ee5575f763c2 SeongJae Park 2023-10-22 23 return nr_targets;
b8ee5575f763c2 SeongJae Park 2023-10-22 24 }
b8ee5575f763c2 SeongJae Park 2023-10-22 25

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-11-28 18:58:12

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH 1/2] mm/damon/sysfs: Implement recording feature

Hi Cuiyanpei,


Thank you for this nice patchset.

On Tue, 28 Nov 2023 15:34:39 +0800 cuiyangpei <[email protected]> wrote:

> The user space users can control DAMON and get the monitoring results
> via implements 'recording' feature in 'damon-sysfs'. The feature
> can be used via 'record' and 'state' file in the '<sysfs>/kernel/mm/
> damon/admin/kdamonds/N/' directory.
>
> The file allows users to record monitored access patterns in a text
> file. Firstly, users set the size of the buffer and the path of the
> result file by writing to the ``record`` file. Then the recorded
> results are first written in an in-memory buffer and flushed the
> recorded results to a file in batch by writing 'record' to the
> ``state`` file.
>
> For example, below commands set the buffer to be 4 KiB and the result
> to be saved in ``/damon.txt``. ::
>
> # cd <sysfs>/kernel/mm/damon/admin/kdamonds/N
> # echo "4096 /damon.txt" > record
> # echo "record" > state

This reminds me the record feature of DAMON debugfs interface[1], which still
not merged in the mainline. I deprioritized the patchset to have a better
answer to Andrew's questions on the discussion (nice definition of the binary
format and quatization of the benefit), and later I realized I don't have real
use case that this makes real benefit, so I'm no more aiming to make this
merged into the mainline.

More specifically, I'm now thinking the feature is not really needed since
trace event based recording works, and we found no problem so far. The DAMON
user-space tool (damo)[2] also dropped support of the in-kernel record feature,
but we received no problem report.

Also, I believe DAMOS tried regions like feature could provide some level of
information, since it provides snapshot of the monitoring result, which
contains a time data, namely 'age'.

Could you please further elaborate your aimed use case of this feature and the
advantage compared to other alternatives (tracepoint-based recording or DAMOS
tried regions based snapshot collecting) I mentioned above?

[1] https://lore.kernel.org/linux-mm/[email protected]/
[2] https://github.com/awslabs/damo


Thanks,
SJ

>
> Signed-off-by: cuiyangpei <[email protected]>