[PATCH v2 0/4] mm/damon: introduce DAMON_STAT for simple and practical access monitoring

[PATCH v2 0/4] mm/damon: introduce DAMON_STAT for simple and practical access monitoring

[SeongJae Park]

DAMON-based access monitoring is not simple due to required DAMON
control and results visualizations.  Introduce a static kernel module
for making it simple.  The module can be enabled without manual setup
and provides access pattern metrics that easy to fetch and understand
the practical access pattern information, namely estimated memory
bandwidth and memory idle time percentiles.

Background and Problems
=======================

DAMON can be used for monitoring data access patterns of the system and
workloads.  Specifically, users can start DAMON to monitor access events
on specific address space with fine controls including address ranges to
monitor and time intervals between samplings and aggregations.  The
resulting access information snapshot contains access frequency
(nr_accesses) and how long the frequency was kept (age) for each byte.

The monitoring usage is not simple and practical enough for production
usage.  Users should first start DAMON with a number of parameters, and
wait until DAMON's monitoring results capture a reasonable amount of the
time data (age).  In production, such manual start and wait is
impractical to capture useful information from a high number of machines
in a timely manner.

The monitoring result is also too detailed to be used on production
environments.  The raw results are hard to be aggregated and/or compared
for production environments having a large scale of time, space and
machines fleet.

Users have to implement and use their own automation of DAMON control
and results processing.  It is repetitive and challenging since there is
no good reference or guideline for such automation.

Solution: DAMON_STAT
====================

Implement such automation in kernel space as a static kernel module,
namely DAMON_STAT.  It can be enabled at build, boot, or run time via
its build configuration or module parameter.  It monitors the entire
physical address space with monitoring intervals that auto-tuned for a
reasonable amount of access observations and minimum overhead.  It
converts the raw monitoring results into simpler metrics that can easily
be aggregated and compared, namely estimated memory bandwidth and idle
time percentiles.

Understanding of the metrics and the user interface of DAMON_STAT is
essential.  Refer to the commit messages of the second and the third
patches of this patch series for more details about the metrics.  For
the user interface, the standard module parameters system is used.
Refer to the fourth patch of this patch series for details of the user
interface.

Discussions
===========

The module aims to be useful on production environments constructed with
a large number of machines that run a long time.  The auto-tuned
monitoring intervals ensure a reasonable quality of the outputs.  The
auto-tuning also ensures its overhead be reasonable and low enough to be
enabled always on the production.  The simplified monitoring results
metrics can be useful for showing both coldness (idle time percentiles)
and hotness (memory bandwidth) of the system's access pattern.  We
expect the information can be useful for assessing system memory
utilization and inspiring optimizations or investigations on both kernel
and user space memory management logics for large scale fleets.

We hence expect the module is good enough to be just used in most
environments.  For special cases that require a custom access monitoring
automation, users will still benefit by using DAMON_STAT as a reference
or a guideline for their specialized automation.

Revision History
================

Changes from v1
(https://lore.kernel.org/20250526210936.2744-1-sj@kernel.org)
- Wordsmith cover letter for usage document reference
- Rebase to latest mm-new

Changes from RFC
(https://lore.kernel.org/20250519164415.43935-1-sj@kernel.org)
- Add an admin-guide documentation
- Wordsmith commit messages
- Rebase to latest mm-new

SeongJae Park (4):
  mm/damon: introduce DAMON_STAT module
  mm/damon/stat: calculate and expose estimated memory bandwidth
  mm/damon/stat: calculate and expose idle time percentiles
  Docs/admin-guide/mm/damon: add DAMON_STAT usage document

 Documentation/admin-guide/mm/damon/index.rst |   1 +
 Documentation/admin-guide/mm/damon/stat.rst  |  69 ++++++
 mm/damon/Kconfig                             |  16 ++
 mm/damon/Makefile                            |   1 +
 mm/damon/stat.c                              | 245 +++++++++++++++++++
 5 files changed, 332 insertions(+)
 create mode 100644 Documentation/admin-guide/mm/damon/stat.rst
 create mode 100644 mm/damon/stat.c


base-commit: c28c2c0b515650b906025a5c41dc0989155720dd
-- 
2.39.5

[PATCH v2 1/4] mm/damon: introduce DAMON_STAT module

[SeongJae Park]

To use DAMON for monitoring access patterns of the system, users should
manually start DAMON via DAMON sysfs ABI with a number of parameters for
specifying the monitoring target address space, address ranges, and
monitoring intervals.  After that, users should also wait until desired
amount of time data is captured into DAMON's monitoring results.  It is
bothersome and take a long time to be practical for access monitoring on
large fleet level production environments.

For access-aware system operations use cases like proactive cold memory
reclamation, similar problems existed.  We we solved those by
introducing dedicated static kernel modules such as DAMON_RECLAIM.

Implement such static kernel module for access monitoring, namely
DAMON_STAT.  It monitors the entire physical address space with
auto-tuned monitoring intervals.  The auto-tuning is set to capture 4 %
of observable access events in each snapshot while keeping the sampling
intervals 5 milliseconds in minimum and 10 seconds in maximum.  From
a few production environments, we confirmed this setup provides high
quality monitoring results with minimum overheads.  The module therefore
receives only one user input, whether to enable or disable it.  It can
be set on build or boot time via build configuration or kernel boot
command line.  It can also be overridden at runtime.

Note that this commit only implements the DAMON control part of the
module.  Users could get the monitoring results via
damon:damon_aggregated tracepoint, but that's of course not the
recommended way.  Following commits will implement convenient and
optimized ways for serving the monitoring results to users.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/Kconfig  |  16 ++++++
 mm/damon/Makefile |   1 +
 mm/damon/stat.c   | 138 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 155 insertions(+)
 create mode 100644 mm/damon/stat.c

diff --git a/mm/damon/Kconfig b/mm/damon/Kconfig
index 551745df011b..9f482e3adc67 100644
--- a/mm/damon/Kconfig
+++ b/mm/damon/Kconfig
@@ -95,4 +95,20 @@ config DAMON_LRU_SORT
 	  protect frequently accessed (hot) pages while rarely accessed (cold)
 	  pages reclaimed first under memory pressure.
 
+config DAMON_STAT
+	bool "Build data access monitoring stat (DAMON_STAT)"
+	depends on DAMON_PADDR
+	help
+	  This builds the DAMON-based access monitoring statistics subsystem.
+	  It runs DAMON and expose access monitoring results in simple stat
+	  metrics.
+
+config DAMON_STAT_ENABLED_DEFAULT
+	bool "Enable DAMON_STAT by default"
+	depends on DAMON_PADDR
+	default DAMON_STAT
+	help
+	  Whether to enable DAMON_STAT by default.  Users can disable it in
+	  boot or runtime using its 'enabled' parameter.
+
 endmenu
diff --git a/mm/damon/Makefile b/mm/damon/Makefile
index 8b49012ba8c3..d8d6bf5f8bff 100644
--- a/mm/damon/Makefile
+++ b/mm/damon/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_DAMON_PADDR)	+= ops-common.o paddr.o
 obj-$(CONFIG_DAMON_SYSFS)	+= sysfs-common.o sysfs-schemes.o sysfs.o
 obj-$(CONFIG_DAMON_RECLAIM)	+= modules-common.o reclaim.o
 obj-$(CONFIG_DAMON_LRU_SORT)	+= modules-common.o lru_sort.o
+obj-$(CONFIG_DAMON_STAT)	+= modules-common.o stat.o
diff --git a/mm/damon/stat.c b/mm/damon/stat.c
new file mode 100644
index 000000000000..852848ce844e
--- /dev/null
+++ b/mm/damon/stat.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Shows data access monitoring resutls in simple metrics.
+ */
+
+#define pr_fmt(fmt) "damon-stat: " fmt
+
+#include <linux/damon.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/sort.h>
+
+#ifdef MODULE_PARAM_PREFIX
+#undef MODULE_PARAM_PREFIX
+#endif
+#define MODULE_PARAM_PREFIX "damon_stat."
+
+static int damon_stat_enabled_store(
+		const char *val, const struct kernel_param *kp);
+
+static const struct kernel_param_ops enabled_param_ops = {
+	.set = damon_stat_enabled_store,
+	.get = param_get_bool,
+};
+
+static bool enabled __read_mostly = CONFIG_DAMON_STAT_ENABLED_DEFAULT;
+module_param_cb(enabled, &enabled_param_ops, &enabled, 0600);
+MODULE_PARM_DESC(enabled, "Enable of disable DAMON_STAT");
+
+static struct damon_ctx *damon_stat_context;
+
+static struct damon_ctx *damon_stat_build_ctx(void)
+{
+	struct damon_ctx *ctx;
+	struct damon_attrs attrs;
+	struct damon_target *target;
+	unsigned long start = 0, end = 0;
+
+	ctx = damon_new_ctx();
+	if (!ctx)
+		return NULL;
+	attrs = (struct damon_attrs) {
+		.sample_interval = 5 * USEC_PER_MSEC,
+		.aggr_interval = 100 * USEC_PER_MSEC,
+		.ops_update_interval = 60 * USEC_PER_MSEC * MSEC_PER_SEC,
+		.min_nr_regions = 10,
+		.max_nr_regions = 1000,
+	};
+	/*
+	 * auto-tune sampling and aggregation interval aiming 4% DAMON-observed
+	 * accesses ratio, keeping sampling interval in [5ms, 10s] range.
+	 */
+	attrs.intervals_goal = (struct damon_intervals_goal) {
+		.access_bp = 400, .aggrs = 3,
+		.min_sample_us = 5000, .max_sample_us = 10000000,
+	};
+	if (damon_set_attrs(ctx, &attrs))
+		goto free_out;
+
+	/*
+	 * auto-tune sampling and aggregation interval aiming 4% DAMON-observed
+	 * accesses ratio, keeping sampling interval in [5ms, 10s] range.
+	 */
+	ctx->attrs.intervals_goal = (struct damon_intervals_goal) {
+		.access_bp = 400, .aggrs = 3,
+		.min_sample_us = 5000, .max_sample_us = 10000000,
+	};
+	if (damon_select_ops(ctx, DAMON_OPS_PADDR))
+		goto free_out;
+
+	target = damon_new_target();
+	if (!target)
+		goto free_out;
+	damon_add_target(ctx, target);
+	if (damon_set_region_biggest_system_ram_default(target, &start, &end))
+		goto free_out;
+	return ctx;
+free_out:
+	damon_destroy_ctx(ctx);
+	return NULL;
+}
+
+static int damon_stat_start(void)
+{
+	damon_stat_context = damon_stat_build_ctx();
+	if (!damon_stat_context)
+		return -ENOMEM;
+	return damon_start(&damon_stat_context, 1, true);
+}
+
+static void damon_stat_stop(void)
+{
+	damon_stop(&damon_stat_context, 1);
+	damon_destroy_ctx(damon_stat_context);
+}
+
+static bool damon_stat_init_called;
+
+static int damon_stat_enabled_store(
+		const char *val, const struct kernel_param *kp)
+{
+	bool is_enabled = enabled;
+	int err;
+
+	err = kstrtobool(val, &enabled);
+	if (err)
+		return err;
+
+	if (is_enabled == enabled)
+		return 0;
+
+	if (!damon_stat_init_called)
+		/*
+		 * probably called from command line parsing (parse_args()).
+		 * Cannot call damon_new_ctx().  Let damon_stat_init() handle.
+		 */
+		return 0;
+
+	if (enabled)
+		return damon_stat_start();
+	damon_stat_stop();
+	return 0;
+}
+
+static int __init damon_stat_init(void)
+{
+	int err = 0;
+
+	damon_stat_init_called = true;
+
+	/* probably set via command line */
+	if (enabled)
+		err = damon_stat_start();
+	return err;
+}
+
+module_init(damon_stat_init);
-- 
2.39.5

[PATCH v2 2/4] mm/damon/stat: calculate and expose estimated memory bandwidth

[SeongJae Park]

The raw form of DAMON's monitoring results captures many details of the
information.  However, not every bit of the information is always
required for understanding practical access patterns.  Especially on
real world production systems of high scale time and size, the raw form
is difficult to be aggregated and compared.

Convert the raw monitoring results into a single number metric, namely
estimated memory bandwidth and expose it to users as a read-only
DAMON_STAT parameter.  The metric represents access intensiveness
(hotness) of the system.  It can easily be aggregated and compared for
high level understanding of the access pattern on large systems.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/stat.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/mm/damon/stat.c b/mm/damon/stat.c
index 852848ce844e..f9ae44db265b 100644
--- a/mm/damon/stat.c
+++ b/mm/damon/stat.c
@@ -28,8 +28,42 @@ static bool enabled __read_mostly = CONFIG_DAMON_STAT_ENABLED_DEFAULT;
 module_param_cb(enabled, &enabled_param_ops, &enabled, 0600);
 MODULE_PARM_DESC(enabled, "Enable of disable DAMON_STAT");
 
+static unsigned long estimated_memory_bandwidth __read_mostly;
+module_param(estimated_memory_bandwidth, ulong, 0400);
+MODULE_PARM_DESC(estimated_memory_bandwidth,
+		"Estimated memory bandwidth usage in bytes per second");
+
 static struct damon_ctx *damon_stat_context;
 
+static void damon_stat_set_estimated_memory_bandwidth(struct damon_ctx *c)
+{
+	struct damon_target *t;
+	struct damon_region *r;
+	unsigned long access_bytes = 0;
+
+	damon_for_each_target(t, c) {
+		damon_for_each_region(r, t)
+			access_bytes += (r->ar.end - r->ar.start) *
+				r->nr_accesses;
+	}
+	estimated_memory_bandwidth = access_bytes * USEC_PER_MSEC *
+		MSEC_PER_SEC / c->attrs.aggr_interval;
+}
+
+static int damon_stat_after_aggregation(struct damon_ctx *c)
+{
+	static unsigned long last_refresh_jiffies;
+
+	/* avoid unnecessarily frequent stat update */
+	if (time_before_eq(jiffies, last_refresh_jiffies +
+				msecs_to_jiffies(5 * MSEC_PER_SEC)))
+		return 0;
+	last_refresh_jiffies = jiffies;
+
+	damon_stat_set_estimated_memory_bandwidth(c);
+	return 0;
+}
+
 static struct damon_ctx *damon_stat_build_ctx(void)
 {
 	struct damon_ctx *ctx;
@@ -75,6 +109,7 @@ static struct damon_ctx *damon_stat_build_ctx(void)
 	damon_add_target(ctx, target);
 	if (damon_set_region_biggest_system_ram_default(target, &start, &end))
 		goto free_out;
+	ctx->callback.after_aggregation = damon_stat_after_aggregation;
 	return ctx;
 free_out:
 	damon_destroy_ctx(ctx);
-- 
2.39.5

[PATCH v2 3/4] mm/damon/stat: calculate and expose idle time percentiles

[SeongJae Park]

Knowing how much memory is how cold can be useful for understanding
coldness and utilization efficiency of memory.  The raw form of DAMON's
monitoring results has the information.  Convert the raw results into
the per-byte idle time distributions and expose it as percentiles metric
to users, as a read-only DAMON_STAT parameter.

In detail, the metrics are calculated as follows.  First, DAMON's
per-region access frequency and age information is converted into
per-byte idle time.  If access frequency of a region is higher than
zero, every byte of the region has zero idle time.  If the access
frequency of a region is zero, every byte of the region has idle time as
the age of the region.  Then the logic sorts the per-byte idle times and
provides the value at 0/100, 1/100, ..., 99/100 and 100/100 location of
the sorted array.

The metric can be easily aggregated and compared on large scale
production systems.  For example, if an average of 75-th percentile idle
time  of machines that collected on similar time is two minutes, it
means the system's 25 percent memory is not accessed at all for two
minutes or more on average.  If a workload considers two minutes as unit
work time, we can conclude its working set size is only 75 percent of
the memory.  If the system utilizes proactive reclamation and it
supports coldness-based thresholds like DAMON_RECLAIM, the idle time
percentiles can be used to find a more safe or aggressive coldness
threshold for aimed memory saving.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/stat.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/mm/damon/stat.c b/mm/damon/stat.c
index f9ae44db265b..7ef13ea22221 100644
--- a/mm/damon/stat.c
+++ b/mm/damon/stat.c
@@ -33,6 +33,11 @@ module_param(estimated_memory_bandwidth, ulong, 0400);
 MODULE_PARM_DESC(estimated_memory_bandwidth,
 		"Estimated memory bandwidth usage in bytes per second");
 
+static unsigned long memory_idle_ms_percentiles[101] __read_mostly = {0,};
+module_param_array(memory_idle_ms_percentiles, ulong, NULL, 0400);
+MODULE_PARM_DESC(memory_idle_ms_percentiles,
+		"Memory idle time percentiles in milliseconds");
+
 static struct damon_ctx *damon_stat_context;
 
 static void damon_stat_set_estimated_memory_bandwidth(struct damon_ctx *c)
@@ -50,6 +55,72 @@ static void damon_stat_set_estimated_memory_bandwidth(struct damon_ctx *c)
 		MSEC_PER_SEC / c->attrs.aggr_interval;
 }
 
+static unsigned int damon_stat_idletime(const struct damon_region *r)
+{
+	if (r->nr_accesses)
+		return 0;
+	return r->age + 1;
+}
+
+static int damon_stat_cmp_regions(const void *a, const void *b)
+{
+	const struct damon_region *ra = *(const struct damon_region **)a;
+	const struct damon_region *rb = *(const struct damon_region **)b;
+
+	return damon_stat_idletime(ra) - damon_stat_idletime(rb);
+}
+
+static int damon_stat_sort_regions(struct damon_ctx *c,
+		struct damon_region ***sorted_ptr, int *nr_regions_ptr,
+		unsigned long *total_sz_ptr)
+{
+	struct damon_target *t;
+	struct damon_region *r;
+	struct damon_region **region_pointers;
+	unsigned int nr_regions = 0;
+	unsigned long total_sz = 0;
+
+	damon_for_each_target(t, c) {
+		/* there is only one target */
+		region_pointers = kmalloc_array(damon_nr_regions(t),
+				sizeof(*region_pointers), GFP_KERNEL);
+		if (!region_pointers)
+			return -ENOMEM;
+		damon_for_each_region(r, t) {
+			region_pointers[nr_regions++] = r;
+			total_sz += r->ar.end - r->ar.start;
+		}
+	}
+	sort(region_pointers, nr_regions, sizeof(*region_pointers),
+			damon_stat_cmp_regions, NULL);
+	*sorted_ptr = region_pointers;
+	*nr_regions_ptr = nr_regions;
+	*total_sz_ptr = total_sz;
+	return 0;
+}
+
+static void damon_stat_set_idletime_percentiles(struct damon_ctx *c)
+{
+	struct damon_region **sorted_regions, *region;
+	int nr_regions;
+	unsigned long total_sz, accounted_bytes = 0;
+	int err, i, next_percentile = 0;
+
+	err = damon_stat_sort_regions(c, &sorted_regions, &nr_regions,
+			&total_sz);
+	if (err)
+		return;
+	for (i = 0; i < nr_regions; i++) {
+		region = sorted_regions[i];
+		accounted_bytes += region->ar.end - region->ar.start;
+		while (next_percentile <= accounted_bytes * 100 / total_sz)
+			memory_idle_ms_percentiles[next_percentile++] =
+				damon_stat_idletime(region) *
+				c->attrs.aggr_interval / USEC_PER_MSEC;
+	}
+	kfree(sorted_regions);
+}
+
 static int damon_stat_after_aggregation(struct damon_ctx *c)
 {
 	static unsigned long last_refresh_jiffies;
@@ -61,6 +132,7 @@ static int damon_stat_after_aggregation(struct damon_ctx *c)
 	last_refresh_jiffies = jiffies;
 
 	damon_stat_set_estimated_memory_bandwidth(c);
+	damon_stat_set_idletime_percentiles(c);
 	return 0;
 }
 
-- 
2.39.5

[PATCH v2 4/4] Docs/admin-guide/mm/damon: add DAMON_STAT usage document

[SeongJae Park]

Document DAMON_STAT usage and add a link to it on DAMON admin-guide
page.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 Documentation/admin-guide/mm/damon/index.rst |  1 +
 Documentation/admin-guide/mm/damon/stat.rst  | 69 ++++++++++++++++++++
 2 files changed, 70 insertions(+)
 create mode 100644 Documentation/admin-guide/mm/damon/stat.rst

diff --git a/Documentation/admin-guide/mm/damon/index.rst b/Documentation/admin-guide/mm/damon/index.rst
index bc7e976120e0..3ce3164480c7 100644
--- a/Documentation/admin-guide/mm/damon/index.rst
+++ b/Documentation/admin-guide/mm/damon/index.rst
@@ -14,3 +14,4 @@ access monitoring and access-aware system operations.
    usage
    reclaim
    lru_sort
+   stat
diff --git a/Documentation/admin-guide/mm/damon/stat.rst b/Documentation/admin-guide/mm/damon/stat.rst
new file mode 100644
index 000000000000..4c517c2c219a
--- /dev/null
+++ b/Documentation/admin-guide/mm/damon/stat.rst
@@ -0,0 +1,69 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===================================
+Data Access Monitoring Results Stat
+===================================
+
+Data Access Monitoring Results Stat (DAMON_STAT) is a static kernel module that
+is aimed to be used for simple access pattern monitoring.  It monitors accesses
+on the system's entire physical memory using DAMON, and provides simplified
+access monitoring results statistics, namely idle time percentiles and
+estimated memory bandwidth.
+
+Monitoring Accuracy and Overhead
+================================
+
+DAMON_STAT uses monitoring intervals :ref:`auto-tuning
+<damon_design_monitoring_intervals_autotuning>` to make its accuracy high and
+overhead minimum.  It auto-tunes the intervals aiming 4 % of observable access
+events to be captured in each snapshot, while limiting the resulting sampling
+events to be 5 milliseconds in minimum and 10 seconds in maximum.  On a few
+production server systems, it resulted in consuming only 0.x % single CPU time,
+while capturing reasonable quality of access patterns.
+
+Interface: Module Parameters
+============================
+
+To use this feature, you should first ensure your system is running on a kernel
+that is built with ``CONFIG_DAMON_STAT=y``.  The feature can be enabled by
+default at build time, by setting ``CONFIG_DAMON_STAT_ENABLED_DEFAULT`` true.
+
+To let sysadmins enable or disable it at boot and/or runtime, and read the
+monitoring results, DAMON_STAT provides module parameters.  Following
+sections are descriptions of the parameters.
+
+enabled
+-------
+
+Enable or disable DAMON_STAT.
+
+You can enable DAMON_STAT by setting the value of this parameter as ``Y``.
+Setting it as ``N`` disables DAMON_STAT.  The default value is set by
+``CONFIG_DAMON_STAT_ENABLED_DEFAULT`` build config option.
+
+estimated_memory_bandwidth
+--------------------------
+
+Estimated memory bandwidth consumption (bytes per second) of the system.
+
+DAMON_STAT reads observed access events on the current DAMON results snapshot
+and converts it to memory bandwidth consumption estimation in bytes per second.
+The resulting metric is exposed to user via this read-only parameter.  Because
+DAMON uses sampling, this is only an estimation of the access intensity rather
+than accurate memory bandwidth.
+
+memory_idle_ms_percentiles
+--------------------------
+
+Per-byte idle time (milliseconds) percentiles of the system.
+
+DAMON_STAT calculates how long each byte of the memory was not accessed until
+now (idle time), based on the current DAMON results snapshot.  If DAMON found a
+region of access frequency (nr_accesses) larger than zero, every byte of the
+region gets zero idle time.  If a region has zero access frequency
+(nr_accesses), how long the region was keeping the zero access frequency (age)
+becomes the idle time of every byte of the region.  Then, DAMON_STAT exposes
+the percentiles of the idle time values via this read-only parameter.  Reading
+the parameter returns 101 idle time values in milliseconds, separated by comma.
+Each value represents 0-th, 1st, 2nd, 3rd, ..., 99th and 100th percentile idle
+times.
-- 
2.39.5

Re: [PATCH v2 1/4] mm/damon: introduce DAMON_STAT module

[SeongJae Park]

On Wed,  4 Jun 2025 11:31:24 -0700 SeongJae Park <sj@kernel.org> wrote:

[...]
> --- /dev/null
> +++ b/mm/damon/stat.c
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Shows data access monitoring resutls in simple metrics.
> + */
> +
> +#define pr_fmt(fmt) "damon-stat: " fmt
> +
> +#include <linux/damon.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/sort.h>
> +
> +#ifdef MODULE_PARAM_PREFIX
> +#undef MODULE_PARAM_PREFIX
> +#endif
> +#define MODULE_PARAM_PREFIX "damon_stat."
> +
> +static int damon_stat_enabled_store(
> +		const char *val, const struct kernel_param *kp);
> +
> +static const struct kernel_param_ops enabled_param_ops = {
> +	.set = damon_stat_enabled_store,
> +	.get = param_get_bool,
> +};
> +
> +static bool enabled __read_mostly = CONFIG_DAMON_STAT_ENABLED_DEFAULT;

Oops, I forgot using IS_ENABLED() here.  Andrew, could you please add below
fixup?


Thanks,
SJ

===== >8 =====
From bc2a2c580f6f89f3f7d4f92c2bde3f4a4fac3409 Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj@kernel.org>
Date: Wed, 4 Jun 2025 13:48:30 -0700
Subject: [PATCH] mm/damon/stat: use IS_ENABLED() for enabled initial value
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The initial value of enabled parameter is set as
CONFIG_DAMON_STAT_ENABLED_DEFAULT, so get below build error when the
config is not enabled.  Fix it using IS_ENABLED().

    mm/damon/stat.c:27:37: error: ‘CONFIG_DAMON_STAT_ENABLED_DEFAULT’ undeclared here (not in a function)
       27 | static bool enabled __read_mostly = CONFIG_DAMON_STAT_ENABLED_DEFAULT;
          |                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/stat.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/damon/stat.c b/mm/damon/stat.c
index 7ef13ea22221..3686f67befc3 100644
--- a/mm/damon/stat.c
+++ b/mm/damon/stat.c
@@ -24,7 +24,8 @@ static const struct kernel_param_ops enabled_param_ops = {
        .get = param_get_bool,
 };

-static bool enabled __read_mostly = CONFIG_DAMON_STAT_ENABLED_DEFAULT;
+static bool enabled __read_mostly = IS_ENABLED(
+               CONFIG_DAMON_STAT_ENABLED_DEFAULT);
 module_param_cb(enabled, &enabled_param_ops, &enabled, 0600);
 MODULE_PARM_DESC(enabled, "Enable of disable DAMON_STAT");

--
2.39.5

Re: [PATCH v2 1/4] mm/damon: introduce DAMON_STAT module

[Joshua Hahn]

On Wed,  4 Jun 2025 11:31:24 -0700 SeongJae Park <sj@kernel.org> wrote:

> To use DAMON for monitoring access patterns of the system, users should
> manually start DAMON via DAMON sysfs ABI with a number of parameters for
> specifying the monitoring target address space, address ranges, and
> monitoring intervals.  After that, users should also wait until desired
> amount of time data is captured into DAMON's monitoring results.  It is
> bothersome and take a long time to be practical for access monitoring on
> large fleet level production environments.
> 
> For access-aware system operations use cases like proactive cold memory
> reclamation, similar problems existed.  We we solved those by
> introducing dedicated static kernel modules such as DAMON_RECLAIM.
> 
> Implement such static kernel module for access monitoring, namely
> DAMON_STAT.  It monitors the entire physical address space with
> auto-tuned monitoring intervals.  The auto-tuning is set to capture 4 %
> of observable access events in each snapshot while keeping the sampling
> intervals 5 milliseconds in minimum and 10 seconds in maximum.  From
> a few production environments, we confirmed this setup provides high
> quality monitoring results with minimum overheads.  The module therefore
> receives only one user input, whether to enable or disable it.  It can
> be set on build or boot time via build configuration or kernel boot
> command line.  It can also be overridden at runtime.
> 
> Note that this commit only implements the DAMON control part of the
> module.  Users could get the monitoring results via
> damon:damon_aggregated tracepoint, but that's of course not the
> recommended way.  Following commits will implement convenient and
> optimized ways for serving the monitoring results to users.
> 
> Signed-off-by: SeongJae Park <sj@kernel.org>
> ---

Hi SJ, thank you for this patch! I have been looking forward to it : -)
I had a few questions about the init function:

[...snip...]

> +static int damon_stat_start(void)
> +{
> +	damon_stat_context = damon_stat_build_ctx();
> +	if (!damon_stat_context)
> +		return -ENOMEM;
> +	return damon_start(&damon_stat_context, 1, true);
> +}
> +
> +static void damon_stat_stop(void)
> +{
> +	damon_stop(&damon_stat_context, 1);
> +	damon_destroy_ctx(damon_stat_context);
> +}
> +
> +static bool damon_stat_init_called;
> +
> +static int damon_stat_enabled_store(
> +		const char *val, const struct kernel_param *kp)
> +{
> +	bool is_enabled = enabled;
> +	int err;
> +
> +	err = kstrtobool(val, &enabled);
> +	if (err)
> +		return err;
> +
> +	if (is_enabled == enabled)
> +		return 0;
> +
> +	if (!damon_stat_init_called)
> +		/*
> +		 * probably called from command line parsing (parse_args()).
> +		 * Cannot call damon_new_ctx().  Let damon_stat_init() handle.
> +		 */
> +		return 0;

I was hoping you could educate me about how damon_stat_init_called works here.
I think my confusion comes from my lack of knowledge about kernel modules : -)
In the cover letter, you wrote that DAMON_STAT is a static kernel module.
My understanding was that this would mean damon_stat_init would always be
called, so I was wondering under what condition it would not be initialized.
I see the comment you wrote above, but was still a little bit confused.

Also, should we perhaps call damon_stat_init() if !damon_stat_init_called?
That way, the first caller would just eat up the time it takes to run
damon_stat_start().

One other thought I have is that if this config checks for whether
damon_stat_init was called, this can be moved to the beginning of the function
before the other checks are run, but that is just my thought : -) Feel free
to keep the input check first, since having this at the beginning of the
function would mean incorrect inputs would be silently ignored.

Thank you SJ! I hope you have a great day!
Joshua

> +	if (enabled)
> +		return damon_stat_start();
> +	damon_stat_stop();
> +	return 0;
> +}
> +
> +static int __init damon_stat_init(void)
> +{
> +	int err = 0;
> +
> +	damon_stat_init_called = true;
> +
> +	/* probably set via command line */
> +	if (enabled)
> +		err = damon_stat_start();
> +	return err;
> +}
> +
> +module_init(damon_stat_init);
> -- 
> 2.39.5

Re: [PATCH v2 1/4] mm/damon: introduce DAMON_STAT module

[SeongJae Park]

On Thu,  5 Jun 2025 08:25:07 -0700 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

> On Wed,  4 Jun 2025 11:31:24 -0700 SeongJae Park <sj@kernel.org> wrote:
[...]
> Hi SJ, thank you for this patch! I have been looking forward to it : -)
> I had a few questions about the init function:

Hi Joshua, I'm more than happy to get your questions :)

> 
> [...snip...]
> 
> > +static int damon_stat_start(void)
> > +{
> > +	damon_stat_context = damon_stat_build_ctx();
> > +	if (!damon_stat_context)
> > +		return -ENOMEM;
> > +	return damon_start(&damon_stat_context, 1, true);
> > +}
> > +
> > +static void damon_stat_stop(void)
> > +{
> > +	damon_stop(&damon_stat_context, 1);
> > +	damon_destroy_ctx(damon_stat_context);
> > +}
> > +
> > +static bool damon_stat_init_called;
> > +
> > +static int damon_stat_enabled_store(
> > +		const char *val, const struct kernel_param *kp)
> > +{
> > +	bool is_enabled = enabled;
> > +	int err;
> > +
> > +	err = kstrtobool(val, &enabled);
> > +	if (err)
> > +		return err;
> > +
> > +	if (is_enabled == enabled)
> > +		return 0;
> > +
> > +	if (!damon_stat_init_called)
> > +		/*
> > +		 * probably called from command line parsing (parse_args()).
> > +		 * Cannot call damon_new_ctx().  Let damon_stat_init() handle.
> > +		 */
> > +		return 0;
> 
> I was hoping you could educate me about how damon_stat_init_called works here.
> I think my confusion comes from my lack of knowledge about kernel modules : -)
> In the cover letter, you wrote that DAMON_STAT is a static kernel module.
> My understanding was that this would mean damon_stat_init would always be
> called,

To my understanding, the function is called back only when the parameteer value
is being changed.  Such changes could be made in runtime via parameter files,
and in boot time via the kernel command line.  If there is not kernel command
line for setting the parameter, this callback function is not called.

> so I was wondering under what condition it would not be initialized.

The enabled parameter value is initialized at build time, based on
CONFIG_DAMON_STAT_ENABLED_DEFAULT.  So, the parameter value will always be
initialized.

> I see the comment you wrote above, but was still a little bit confused.

The kernel command line parameters parsing is called in pretty early stage of
the bootup, before slab is ready.  Hence, if enabled parmeter is set by the
kernel command line, damon_stat_enabled_store() is called in the early stage,
and fails from damon_stat_start(), since it needs slab, to initialize DAMON
contexts.  For more details of such failure, you could refer to a previous
issue report[1].

Meanwhile, damon_stat_init() and module init functions are called later, when
slab is ready.  We therefore check the case, and defer real handling of enabled
to damon_stat_init() in the case.

Thank you for this question, I find the comment has rooms to improve.  I'll try
to make this better documented or easier to read.

> 
> Also, should we perhaps call damon_stat_init() if !damon_stat_init_called?
> That way, the first caller would just eat up the time it takes to run
> damon_stat_start().

damon_stat_init() is a module init function, and hence it will be called in
boot time, regardless of enabled parameter setup on kernel command line.  In
other words, it will be always invoked once, with !damon_stat_init_called.  And
it will call damon_stat_start(), unless enabled is unset via
CONFIG_DAMON_STAT_ENABLED_DEFAULT or kernel command line.

So, the current implementation is working as you suggested, to my
understanding.  Please let me know if I'm missing something.

> 
> One other thought I have is that if this config checks for whether
> damon_stat_init was called, this can be moved to the beginning of the function
> before the other checks are run, but that is just my thought : -) Feel free
> to keep the input check first, since having this at the beginning of the
> function would mean incorrect inputs would be silently ignored.

In the kernel command line based parameter setup scenario, later
damon_stat_init() call should see the updated 'enabled' variable value.  Hence,
the user input value check should be done here, regardless of if this is called
before or after damon_stat_init().

So I find no needs to change the code for now.  Nonetheless, I believe this
code has many rooms to improve, and I'm always getting more than glad to get
this kind of improvement ideas.  Thank you, Joshua.  Please feel free to let me
know if you get another idea later.

I hope I answered your questions, but please let me know if I'm missing
something!

> 
> Thank you SJ! I hope you have a great day!

You too.  Friday is coming!

[1] https://lore.kernel.org/linux-mm/20220604192222.1488-1-sj@kernel.org/


Thanks,
SJ

[...]

Re: [PATCH v2 1/4] mm/damon: introduce DAMON_STAT module

[Joshua Hahn]

On Thu,  5 Jun 2025 09:11:29 -0700 SeongJae Park <sj@kernel.org> wrote:

> On Thu,  5 Jun 2025 08:25:07 -0700 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> 
> > On Wed,  4 Jun 2025 11:31:24 -0700 SeongJae Park <sj@kernel.org> wrote:
> [...]
> > Hi SJ, thank you for this patch! I have been looking forward to it : -)
> > I had a few questions about the init function:
> 
> Hi Joshua, I'm more than happy to get your questions :)
> 
> > 
> > [...snip...]
> > 
> > > +static int damon_stat_start(void)
> > > +{
> > > +	damon_stat_context = damon_stat_build_ctx();
> > > +	if (!damon_stat_context)
> > > +		return -ENOMEM;
> > > +	return damon_start(&damon_stat_context, 1, true);
> > > +}
> > > +
> > > +static void damon_stat_stop(void)
> > > +{
> > > +	damon_stop(&damon_stat_context, 1);
> > > +	damon_destroy_ctx(damon_stat_context);
> > > +}
> > > +
> > > +static bool damon_stat_init_called;
> > > +
> > > +static int damon_stat_enabled_store(
> > > +		const char *val, const struct kernel_param *kp)
> > > +{
> > > +	bool is_enabled = enabled;
> > > +	int err;
> > > +
> > > +	err = kstrtobool(val, &enabled);
> > > +	if (err)
> > > +		return err;
> > > +
> > > +	if (is_enabled == enabled)
> > > +		return 0;
> > > +
> > > +	if (!damon_stat_init_called)
> > > +		/*
> > > +		 * probably called from command line parsing (parse_args()).
> > > +		 * Cannot call damon_new_ctx().  Let damon_stat_init() handle.
> > > +		 */
> > > +		return 0;
> > 
> > I was hoping you could educate me about how damon_stat_init_called works here.
> > I think my confusion comes from my lack of knowledge about kernel modules : -)
> > In the cover letter, you wrote that DAMON_STAT is a static kernel module.
> > My understanding was that this would mean damon_stat_init would always be
> > called,
> 
> To my understanding, the function is called back only when the parameteer value
> is being changed.  Such changes could be made in runtime via parameter files,
> and in boot time via the kernel command line.  If there is not kernel command
> line for setting the parameter, this callback function is not called.
> 
> > so I was wondering under what condition it would not be initialized.
> 
> The enabled parameter value is initialized at build time, based on
> CONFIG_DAMON_STAT_ENABLED_DEFAULT.  So, the parameter value will always be
> initialized.
> 
> > I see the comment you wrote above, but was still a little bit confused.
> 
> The kernel command line parameters parsing is called in pretty early stage of
> the bootup, before slab is ready.  Hence, if enabled parmeter is set by the
> kernel command line, damon_stat_enabled_store() is called in the early stage,
> and fails from damon_stat_start(), since it needs slab, to initialize DAMON
> contexts.  For more details of such failure, you could refer to a previous
> issue report[1].
> 
> Meanwhile, damon_stat_init() and module init functions are called later, when
> slab is ready.  We therefore check the case, and defer real handling of enabled
> to damon_stat_init() in the case.
> 
> Thank you for this question, I find the comment has rooms to improve.  I'll try
> to make this better documented or easier to read.

I see, I think it makes more sense to me now. Thank you for explaining this, SJ!
I think my confusion came from my lack of knowledge. Please do not feel the need
to update the comment on this section, if you feel it is already enough : -)

> > Also, should we perhaps call damon_stat_init() if !damon_stat_init_called?
> > That way, the first caller would just eat up the time it takes to run
> > damon_stat_start().
> 
> damon_stat_init() is a module init function, and hence it will be called in
> boot time, regardless of enabled parameter setup on kernel command line.  In
> other words, it will be always invoked once, with !damon_stat_init_called.  And
> it will call damon_stat_start(), unless enabled is unset via
> CONFIG_DAMON_STAT_ENABLED_DEFAULT or kernel command line.
> 
> So, the current implementation is working as you suggested, to my
> understanding.  Please let me know if I'm missing something.
> > 
> > One other thought I have is that if this config checks for whether
> > damon_stat_init was called, this can be moved to the beginning of the function
> > before the other checks are run, but that is just my thought : -) Feel free
> > to keep the input check first, since having this at the beginning of the
> > function would mean incorrect inputs would be silently ignored.
> 
> In the kernel command line based parameter setup scenario, later
> damon_stat_init() call should see the updated 'enabled' variable value.  Hence,
> the user input value check should be done here, regardless of if this is called
> before or after damon_stat_init().
> 
> So I find no needs to change the code for now.  Nonetheless, I believe this
> code has many rooms to improve, and I'm always getting more than glad to get
> this kind of improvement ideas.  Thank you, Joshua.  Please feel free to let me
> know if you get another idea later.
> 
> I hope I answered your questions, but please let me know if I'm missing
> something!

Thanks SJ, it all makes a lot more sense now. Thank you for taking the time
to explain things!

> > 
> > Thank you SJ! I hope you have a great day!
> 
> You too.  Friday is coming!
> 
> [1] https://lore.kernel.org/linux-mm/20220604192222.1488-1-sj@kernel.org/
> 
> 
> Thanks,
> SJ
> 
> [...]

Sent using hkml (https://github.com/sjp38/hackermail)