On Mon, Jun 21, 2021 at 1:31 AM SeongJae Park <sj38.park@xxxxxxxxx> wrote:
>
> From: SeongJae Park <sjpark@xxxxxxxxx>
>
> DAMON is a data access monitoring framework for the Linux kernel. The
> core mechanisms of DAMON make it
>
> - accurate (the monitoring output is useful enough for DRAM level
> performance-centric memory management; It might be inappropriate for
> CPU cache levels, though),
> - light-weight (the monitoring overhead is normally low enough to be
> applied online), and
> - scalable (the upper-bound of the overhead is in constant range
> regardless of the size of target workloads).
>
> Using this framework, hence, we can easily write efficient kernel space
> data access monitoring applications. For example, the kernel's memory
> management mechanisms can make advanced decisions using this.
> Experimental data access aware optimization works that incurring high
> access monitoring overhead could again be implemented on top of this.
>
> Due to its simple and flexible interface, providing user space interface
> would be also easy. Then, user space users who have some special
> workloads can write personalized applications for better understanding
> and optimizations of their workloads and systems.
>
> ===
>
> Nevertheless, this commit is defining and implementing only basic access
> check part without the overhead-accuracy handling core logic. The basic
> access check is as below.
>
> The output of DAMON says what memory regions are how frequently accessed
> for a given duration. The resolution of the access frequency is
> controlled by setting ``sampling interval`` and ``aggregation
> interval``. In detail, DAMON checks access to each page per ``sampling
> interval`` and aggregates the results. In other words, counts the
> number of the accesses to each region. After each ``aggregation
> interval`` passes, DAMON calls callback functions that previously
> registered by users so that users can read the aggregated results and
> then clears the results. This can be described in below simple
> pseudo-code::
>
> init()
> while monitoring_on:
> for page in monitoring_target:
> if accessed(page):
> nr_accesses[page] += 1
> if time() % aggregation_interval == 0:
> for callback in user_registered_callbacks:
> callback(monitoring_target, nr_accesses)
> for page in monitoring_target:
> nr_accesses[page] = 0
> if time() % update_interval == 0:
regions_update_interval?
> update()
> sleep(sampling interval)
>
> The target regions constructed at the beginning of the monitoring and
> updated after each ``regions_update_interval``, because the target
> regions could be dynamically changed (e.g., mmap() or memory hotplug).
> The monitoring overhead of this mechanism will arbitrarily increase as
> the size of the target workload grows.
>
> The basic monitoring primitives for actual access check and dynamic
> target regions construction aren't in the core part of DAMON. Instead,
> it allows users to implement their own primitives that are optimized for
> their use case and configure DAMON to use those. In other words, users
> cannot use current version of DAMON without some additional works.
>
> Following commits will implement the core mechanisms for the
> overhead-accuracy control and default primitives implementations.
>
> Signed-off-by: SeongJae Park <sjpark@xxxxxxxxx>
> Reviewed-by: Leonard Foerster <foersleo@xxxxxxxxx>
> Reviewed-by: Fernand Sieber <sieberf@xxxxxxxxxx>
Few nits below otherwise look good to me. You can add:
Acked-by: Shakeel Butt <shakeelb@xxxxxxxxxx>
[...]
> +/*
> + * __damon_start() - Starts monitoring with given context.
> + * @ctx: monitoring context
> + *
> + * This function should be called while damon_lock is hold.
> + *
> + * Return: 0 on success, negative error code otherwise.
> + */
> +static int __damon_start(struct damon_ctx *ctx)
> +{
> + int err = -EBUSY;
> +
> + mutex_lock(&ctx->kdamond_lock);
> + if (!ctx->kdamond) {
> + err = 0;
> + ctx->kdamond_stop = false;
> + ctx->kdamond = kthread_create(kdamond_fn, ctx, "kdamond.%d",
> + nr_running_ctxs);
> + if (IS_ERR(ctx->kdamond))
> + err = PTR_ERR(ctx->kdamond);
> + else
> + wake_up_process(ctx->kdamond);
Nit: You can use kthread_run() here.
> + }
> + mutex_unlock(&ctx->kdamond_lock);
> +
> + return err;
> +}
> +
[...]
> +static int __damon_stop(struct damon_ctx *ctx)
> +{
> + mutex_lock(&ctx->kdamond_lock);
> + if (ctx->kdamond) {
> + ctx->kdamond_stop = true;
> + mutex_unlock(&ctx->kdamond_lock);
> + while (damon_kdamond_running(ctx))
> + usleep_range(ctx->sample_interval,
> + ctx->sample_interval * 2);
Any reason to not use kthread_stop() here?
