Re: [PATCH v6 06/14] mm/damon: Implement access pattern recording

From: Jonathan Cameron
Date: Tue Mar 10 2020 - 05:01:39 EST


On Mon, 24 Feb 2020 13:30:39 +0100
SeongJae Park <sjpark@xxxxxxxxxx> wrote:

> From: SeongJae Park <sjpark@xxxxxxxxx>
>
> This commit implements the recording feature of DAMON. If this feature
> is enabled, DAMON writes the monitored access patterns in its binary
> format into a file which specified by the user. This is already able to
> be implemented by each user using the callbacks. However, as the
> recording is expected to be used widely, this commit implements the
> feature in the DAMON, for more convenience and efficiency.
>
> Signed-off-by: SeongJae Park <sjpark@xxxxxxxxx>

I guess this work whilst you are still developing, but I'm not convinced
writing to a file should be a standard feature...

> ---
> mm/damon.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 123 insertions(+), 3 deletions(-)
>
> diff --git a/mm/damon.c b/mm/damon.c
> index 554720778e8a..a7edb2dfa700 100644
> --- a/mm/damon.c
> +++ b/mm/damon.c
> @@ -76,6 +76,11 @@ struct damon_ctx {
> struct timespec64 last_aggregation;
> struct timespec64 last_regions_update;
>
> + unsigned char *rbuf;
> + unsigned int rbuf_len;
> + unsigned int rbuf_offset;
> + char *rfile_path;
> +
> struct task_struct *kdamond;
> bool kdamond_stop;
> spinlock_t kdamond_lock;
> @@ -89,6 +94,8 @@ struct damon_ctx {
> void (*aggregate_cb)(struct damon_ctx *context);
> };
>
> +#define MAX_RFILE_PATH_LEN 256
> +
> /* Get a random number in [l, r) */
> #define damon_rand(ctx, l, r) (l + prandom_u32_state(&ctx->rndseed) % (r - l))
>
> @@ -550,16 +557,81 @@ static bool kdamond_aggregate_interval_passed(struct damon_ctx *ctx)
> }
>
> /*
> - * Reset the aggregated monitoring results
> + * Flush the content in the result buffer to the result file
> + */
> +static void damon_flush_rbuffer(struct damon_ctx *ctx)
> +{
> + ssize_t sz;
> + loff_t pos;
> + struct file *rfile;
> +
> + while (ctx->rbuf_offset) {
> + pos = 0;
> + rfile = filp_open(ctx->rfile_path, O_CREAT | O_RDWR | O_APPEND,
> + 0644);
> + if (IS_ERR(rfile)) {
> + pr_err("Cannot open the result file %s\n",
> + ctx->rfile_path);
> + return;
> + }
> +
> + sz = kernel_write(rfile, ctx->rbuf, ctx->rbuf_offset, &pos);
> + filp_close(rfile, NULL);
> +
> + ctx->rbuf_offset -= sz;
> + }
> +}
> +
> +/*
> + * Write a data into the result buffer
> + */
> +static void damon_write_rbuf(struct damon_ctx *ctx, void *data, ssize_t size)
> +{
> + if (!ctx->rbuf_len || !ctx->rbuf)
> + return;
> + if (ctx->rbuf_offset + size > ctx->rbuf_len)
> + damon_flush_rbuffer(ctx);
> +
> + memcpy(&ctx->rbuf[ctx->rbuf_offset], data, size);
> + ctx->rbuf_offset += size;
> +}
> +
> +/*
> + * Flush the aggregated monitoring results to the result buffer
> + *
> + * Stores current tracking results to the result buffer and reset 'nr_accesses'
> + * of each regions. The format for the result buffer is as below:
> + *
> + * <time> <number of tasks> <array of task infos>
> + *
> + * task info: <pid> <number of regions> <array of region infos>
> + * region info: <start address> <end address> <nr_accesses>
> */
> static void kdamond_flush_aggregated(struct damon_ctx *c)
> {
> struct damon_task *t;
> - struct damon_region *r;
> + struct timespec64 now;
> + unsigned int nr;
> +
> + ktime_get_coarse_ts64(&now);
> +
> + damon_write_rbuf(c, &now, sizeof(struct timespec64));
> + nr = nr_damon_tasks(c);
> + damon_write_rbuf(c, &nr, sizeof(nr));
>
> damon_for_each_task(c, t) {
> - damon_for_each_region(r, t)
> + struct damon_region *r;
> +
> + damon_write_rbuf(c, &t->pid, sizeof(t->pid));
> + nr = nr_damon_regions(t);
> + damon_write_rbuf(c, &nr, sizeof(nr));
> + damon_for_each_region(r, t) {
> + damon_write_rbuf(c, &r->vm_start, sizeof(r->vm_start));
> + damon_write_rbuf(c, &r->vm_end, sizeof(r->vm_end));
> + damon_write_rbuf(c, &r->nr_accesses,
> + sizeof(r->nr_accesses));
> r->nr_accesses = 0;
> + }
> }
> }
>
> @@ -834,6 +906,7 @@ static int kdamond_fn(void *data)
>
> usleep_range(ctx->sample_interval, ctx->sample_interval + 1);
> }
> + damon_flush_rbuffer(ctx);
> damon_for_each_task(ctx, t) {
> damon_for_each_region_safe(r, next, t)
> damon_destroy_region(r);
> @@ -912,6 +985,53 @@ static int damon_set_pids(struct damon_ctx *ctx,
> return 0;
> }
>
> +/*
> + * 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.
> + *
> + * Returns 0 on success, negative error code otherwise.
> + */
> +static int damon_set_recording(struct damon_ctx *ctx,
> + unsigned int rbuf_len, char *rfile_path)
> +{
> + size_t rfile_path_len;
> +
> + if (rbuf_len > 4 * 1024 * 1024) {
> + pr_err("too long (>%d) result buffer length\n",
> + 4 * 1024 * 1024);
> + 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;
> + }
> + ctx->rbuf_len = rbuf_len;
> + kfree(ctx->rbuf);
> + kfree(ctx->rfile_path);
> + ctx->rfile_path = NULL;
> + if (!rbuf_len) {
> + ctx->rbuf = NULL;
> + } else {
> + ctx->rbuf = kvmalloc(rbuf_len, GFP_KERNEL);
> + if (!ctx->rbuf)
> + return -ENOMEM;
> + }
> + ctx->rfile_path = kmalloc(rfile_path_len + 1, GFP_KERNEL);
> + if (!ctx->rfile_path)
> + return -ENOMEM;
> + strncpy(ctx->rfile_path, rfile_path, rfile_path_len + 1);
> + return 0;
> +}
> +
> /*
> * Set attributes for the monitoring
> *