Re: [RFC PATCH v2 3/4] mm/damon: New module with hot application detection

Gutierrez Asier
2026-03-11T13:45:10Z

On 3/11/2026 7:11 AM, SeongJae Park wrote:
> On Tue, 10 Mar 2026 16:24:19 +0000 <gutierrez.asier@huawei-partners.com> wrote:
>
>> From: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
>
> I'd like to discuss about this in a high level, but I'd like to do that on the
> cover letter. To this patch, I'm therefore adding comments for only details
> that stand out immeadiately to me.

I will update the cover letter to add more details for the next version.

>>
>> 1. It first launches a new kthread called damon_dynamic. This thread
>> will behave as a supervisor, launching new kdamond threads for all
>> the processes we want to montiored. The tasks are sorted
>> by utime delta. For the top N tasks, a new kdamond thread will be
>> launched.
>
> If I read the cover letter and the code of this patch correctly, seems the last
> two sentences of the above paragraph is outdated. Please correct me if I'm
> wrong.

Correct, I forgot to delete it.

>
>> Applications which turn cold will have their kdamond
>> stopped.
>>
>> The processes are supplied by the monitored_pids parameter. When the
>> module is enabled, it will go through all the monitored_pids, start
>> the supervisor and a new kdamond thread for each of the tasks. This
>> tasks can be modified and applied using the commit_input parameters.
>> In that case, we will stop any kdamond thread for tasks that are not
>> going to be monitored anymore, and start a new kdamond thread for each
>> new task to be monitored. For tasks that were monitored before and are
>> still monitored after commiting a new monitored_pids list, kdamond
>> threads are left intact.
>>
>> 2. Initially we don't know the min_access for each of the task. We
>> want to find the highest min_access when collapses start happening.
>> For that we have an initial threashold of 90, which we will lower
>> until a collpase occurs.
>>
>> Signed-off-by: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
>> Co-developed-by: Anatoly Stepanov <stepanov.anatoly@huawei.com>
>> ---
>> mm/damon/Kconfig | 7 +
>> mm/damon/Makefile | 1 +
>> mm/damon/hugepage.c (new) | 441 ++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 449 insertions(+)
>>
>> diff --git a/mm/damon/Kconfig b/mm/damon/Kconfig
>> index 8c868f7035fc..2355aacb6d12 100644
>> --- a/mm/damon/Kconfig
>> +++ b/mm/damon/Kconfig
>> @@ -110,4 +110,11 @@ config DAMON_STAT_ENABLED_DEFAULT
>> Whether to enable DAMON_STAT by default. Users can disable it in
>> boot or runtime using its 'enabled' parameter.
>>
>> +config DAMON_HOT_HUGEPAGE
>
> I'd suggest renaming to DAMON_HUGEPAGE.

OK

>
>> + bool "Build DAMON-based collapse of hot regions (DAMON_HOT_HUGEPAGES)"
>> + depends on DAMON_VADDR
>> + help
>> + Collapse hot region into huge pages. Hot regions are determined by
>> + DAMON-based sampling
>> +
>> endmenu
>> diff --git a/mm/damon/Makefile b/mm/damon/Makefile
>> index d8d6bf5f8bff..ac3afbc81cc7 100644
>> --- a/mm/damon/Makefile
>> +++ b/mm/damon/Makefile
>> @@ -7,3 +7,4 @@ 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
>> +obj-$(CONFIG_DAMON_HOT_HUGEPAGE) += modules-common.o hugepage.o
>> diff --git a/mm/damon/hugepage.c b/mm/damon/hugepage.c
>> new file mode 100644
>> index 000000000000..ccd31c48d391
>> --- /dev/null
>> +++ b/mm/damon/hugepage.c
>> @@ -0,0 +1,441 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2026 HUAWEI, Inc.
>> + * https://www.huawei.com
>> + *
>> + * Author: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
>> + */
>> +
>> +#define pr_fmt(fmt) "damon-hugepage: " fmt
>> +
>> +#include <linux/damon.h>
>> +#include <linux/kstrtox.h>
>> +#include <linux/module.h>
>> +
>> +#include "modules-common.h"
>> +
>> +#ifdef MODULE_PARAM_PREFIX
>> +#undef MODULE_PARAM_PREFIX
>> +#endif
>> +#define MODULE_PARAM_PREFIX "damon_hugepage."
>> +
>> +#define MAX_MONITORED_PIDS 100
>> +#define HIGHEST_MIN_ACCESS 90
>> +#define HIGH_ACC_THRESHOLD 50
>> +#define MID_ACC_THRESHOLD 15
>> +#define LOW_ACC_THRESHOLD 2
>
> On your setup where the sampling interval is 5ms and the aggregation interval
> is 100ms, nr_accesses of each DAMON region can be up to only 20. So above
> thresholds may better to be lowered.
>
>> +
>> +static struct task_struct *monitor_thread;
>> +
>> +struct mutex enable_disable_lock;
>> +
>> +/*
>> + * Enable or disable DAMON_HUGEPAGE.
>> + *
>> + * You can enable DAMON_HUGEPAGE by setting the value of this parameter
>> + * as ``Y``. Setting it as ``N`` disables DAMON_HOT_HUGEPAGE. Note that
>> + * DAMON_HOT_HUGEPAGE could do no real monitoring and reclamation due to the
>
> s/DAMON_HOT_HUGEPAGE/DAMON_HUGEPAGE/ ?

I will change it.

>
>> + * watermarks-based activation condition. Refer to below descriptions for the
>> + * watermarks parameter for this.
>
> On RFC v1 discussion [1], you mentioned you will update this comment. But I
> find no change?

I will add links to future discuessions.

>
>> + */
>> +static bool enabled __read_mostly;
>> +
>> +/*
>> + * Make DAMON_HUGEPAGE reads the input parameters again, except ``enabled``.
>> + *
>> + * Input parameters that updated while DAMON_HUGEPAGE is running are not applied
>> + * by default. Once this parameter is set as ``Y``, DAMON_HUGEPAGE reads values
>> + * of parametrs except ``enabled`` again. Once the re-reading is done, this
>> + * parameter is set as ``N``. If invalid parameters are found while the
>> + * re-reading, DAMON_HUGEPAGE will be disabled.
>> + */
>> +static bool commit_inputs __read_mostly;
>> +module_param(commit_inputs, bool, 0600);
>> +
>> +/*
>> + * DAMON_HUGEPAGE monitoring period in microseconds.
>> + * 5000000 = 5s
>> + */
>> +static unsigned long monitor_period __read_mostly = 5000000;
>> +module_param(monitor_period, ulong, 0600);
>> +
>> +static long monitored_pids[MAX_MONITORED_PIDS];
>> +static int num_monitored_pids;
>> +module_param_array(monitored_pids, long, &num_monitored_pids, 0600);
>> +
>> +static struct damos_quota damon_hugepage_quota = {
>> + /* use up to 10 ms time, reclaim up to 128 MiB per 1 sec by default */
>> + .ms = 10,
>> + .sz = 0,
>> + .reset_interval = 1000,
>> + /* Within the quota, page out older regions first. */
>> + .weight_sz = 0,
>> + .weight_nr_accesses = 0,
>> + .weight_age = 1
>
> You may want to prioritize hot memory region under the quota. If so, you would
> need to set weight_nr_acceses to non-zero.
>
>> +};
>> +DEFINE_DAMON_MODULES_DAMOS_TIME_QUOTA(damon_hugepage_quota);
>> +
>> +static struct damos_watermarks damon_hugepage_wmarks = {
>> + .metric = DAMOS_WMARK_FREE_MEM_RATE,
>> + .interval = 5000000, /* 5 seconds */
>> + .high = 900, /* 90 percent */
>> + .mid = 800, /* 80 percent */
>> + .low = 50, /* 5 percent */
>> +};
>> +DEFINE_DAMON_MODULES_WMARKS_PARAMS(damon_hugepage_wmarks);
>
> In the RFC v1 discussion, you also mentioned you will remove the above, but
> seems it is forgotten?
>
> Also, my understadning of your reply on the previous discussion was that you
> don't really want to use watermarks in this module, and therefore you want to
> remove it. Am I correct? If I'm not, maybe I'm still not understanding the
> intention of the watermarks. Can you please enlighten me?

Since I refactored heavily the previous code, I forgot to delete this bit.
I will remove it now.

>> +
>> +static struct damon_attrs damon_hugepage_mon_attrs = {
>> + .sample_interval = 5000, /* 5 ms */
>> + .aggr_interval = 100000, /* 100 ms */
>> + .ops_update_interval = 0,
>
> Is ops_update_interval = 0 really your intention?

I didn't think about it. I will set it to 60 seconds, as in other modules.

>
>> + .min_nr_regions = 10,
>> + .max_nr_regions = 1000,
>> +};
>> +DEFINE_DAMON_MODULES_MON_ATTRS_PARAMS(damon_hugepage_mon_attrs);
>> +
>> +struct hugepage_task {
>> + struct damon_ctx *ctx;
>> + int pid;
>> + struct damon_target *target;
>> + struct damon_call_control call_control;
>> + struct list_head list;
>> +};
>> +
>> +static struct damos *damon_hugepage_new_scheme(int min_access,
>> + enum damos_action action)
>> +{
>> + struct damos_access_pattern pattern = {
>> + /* Find regions having PAGE_SIZE or larger size */
>
> s/PAGE_SIZE/PMD_SIZE/ ?
>
>> + .min_sz_region = PMD_SIZE,
>> + .max_sz_region = ULONG_MAX,
>> + /* and not accessed at all */
>
> Seems the above comment should be updated.
>
>> + .min_nr_accesses = min_access,
>> + .max_nr_accesses = 100,
>
> Because you set sampling interval 5ms and aggregatin interval 100ms,
> nr_accesses of each DAMON region can be up to only 20 (100ms / 5ms).
> max_nr_accesses may better to be 20 to clarifyt that.
>
>> + /* for min_age or more micro-seconds */
>> + .min_age_region = 0,
>> + .max_age_region = UINT_MAX,
>> + };
>> +
>> + return damon_new_scheme(
>> + &pattern,
>> + /* synchrounous partial collapse as soon as found */
>> + action, 0,
>> + /* under the quota. */
>> + &damon_hugepage_quota,
>> + /* (De)activate this according to the watermarks. */
>> + &damon_hugepage_wmarks, NUMA_NO_NODE);
>
> Again, I'm not sure if you really want to use watermarks, and if so, what is
> the intention.

Right, I will remove this code.

>> +}
>> +
>> +static int damon_hugepage_apply_parameters(
>> + struct hugepage_task *monitored_task,
>> + int min_access,
>> + enum damos_action action)
>> +{
>> + struct damos *scheme;
>> + struct damon_ctx *param_ctx;
>> + struct damon_target *param_target;
>> + struct damos_filter *filter;
>> + int err;
>> + struct pid *spid;
>> +
>> + err = damon_modules_new_ctx_target(&param_ctx, &param_target,
>> + DAMON_OPS_VADDR);
>> + if (err)
>> + return err;
>
> You should deallocate param_ctx before returning the error.

Not sure about this. As far as I know, damon_modules_new_paddr_ctx_target
will destroy the context in case there is an error. In fact, reclaim and
lru_sort modules just return error when damon_modules_new_paddr_ctx_target
fails, no cleanup used.

>
>> +
>> + spid = find_get_pid(monitored_task->pid);
>> + if (!spid)
>> + return err;
>
> Again, please deallocate param_ctx before returning the error.

OK

>
>> +
>> + param_target->pid = spid;
>> +
>> + err = damon_set_attrs(param_ctx, &damon_hugepage_mon_attrs);
>> + if (err)
>> + goto out;
>> +
>> + err = -ENOMEM;
>> + scheme = damon_hugepage_new_scheme(min_access, action);
>> + if (!scheme)
>> + goto out;
>> +
>> + damon_set_schemes(param_ctx, &scheme, 1);
>> +
>> + filter = damos_new_filter(DAMOS_FILTER_TYPE_ANON, true, false);
>> + if (!filter)
>> + goto out;
>> + damos_add_filter(scheme, filter);
>
> What's the intention of the above filter?

Also, I will remove it.

>> +
>> + err = damon_commit_ctx(monitored_task->ctx, param_ctx);
>> +out:
>> + damon_destroy_ctx(param_ctx);
>> + return err;
>> +}
>> +
>> +static int damon_hugepage_damon_call_fn(void *arg)
>> +{
>> + struct hugepage_task *monitored_task = arg;
>> + struct damon_ctx *ctx = monitored_task->ctx;
>> + struct damos *scheme;
>> + int err = 0;
>> + int min_access;
>> + struct damos_stat stat;
>> +
>> + damon_for_each_scheme(scheme, ctx)
>> + stat = scheme->stat;
>> + scheme = list_first_entry(&ctx->schemes, struct damos, list);
>> +
>> + if (ctx->passed_sample_intervals < scheme->next_apply_sis)
>> + return err;
>> +
>> + if (stat.nr_applied)
>> + return err;
>> +
>> + min_access = scheme->pattern.min_nr_accesses;
>> +
>> + if (min_access > HIGH_ACC_THRESHOLD) {
>> + min_access = min_access - 10;
>> + err = damon_hugepage_apply_parameters(
>> + monitored_task, min_access, DAMOS_COLLAPSE);
>> + } else if (min_access > MID_ACC_THRESHOLD) {
>> + min_access = min_access - 5;
>> + err = damon_hugepage_apply_parameters(
>> + monitored_task, min_access, DAMOS_COLLAPSE);
>> + } else if (min_access > LOW_ACC_THRESHOLD) {
>> + min_access = min_access - 1;
>> + err = damon_hugepage_apply_parameters(
>> + monitored_task, min_access, DAMOS_COLLAPSE);
>> + }
>> + return err;
>> +}
>> +
>> +static int damon_hugepage_init_task(struct hugepage_task *monitored_task)
>> +{
>> + int err = 0;
>> + struct damon_ctx *ctx = monitored_task->ctx;
>> + struct damon_target *target = monitored_task->target;
>> + struct pid *spid;
>> +
>> + if (!ctx || !target)
>> + damon_modules_new_ctx_target(&ctx, &target, DAMON_OPS_VADDR);
>> +
>> + if (damon_is_running(ctx))
>> + return 0;
>> +
>> + spid = find_get_pid(monitored_task->pid);
>> + if (!spid)
>> + return err;
>> +
>> + target->pid = spid;
>> +
>> + monitored_task->call_control.fn = damon_hugepage_damon_call_fn;
>> + monitored_task->call_control.repeat = true;
>> + monitored_task->call_control.data = monitored_task;
>> +
>> + struct damos *scheme = damon_hugepage_new_scheme(
>> + HIGHEST_MIN_ACCESS, DAMOS_COLLAPSE);
>> + if (!scheme)
>> + return -ENOMEM;
>> +
>> + damon_set_schemes(ctx, &scheme, 1);
>> +
>> + monitored_task->ctx = ctx;
>> + err = damon_start(&monitored_task->ctx, 1, false);
>> + if (err)
>> + return err;
>> +
>> + return damon_call(monitored_task->ctx, &monitored_task->call_control);
>> +}
>> +
>> +static int add_monitored_task(int pid, struct list_head *task_monitor)
>> +{
>> + struct hugepage_task *new_hugepage_task;
>> + int err;
>> +
>> + new_hugepage_task = kzalloc_obj(*new_hugepage_task);
>> + if (!new_hugepage_task)
>> + return -ENOMEM;
>> +
>> + new_hugepage_task->pid = pid;
>> + INIT_LIST_HEAD(&new_hugepage_task->list);
>> + err = damon_hugepage_init_task(new_hugepage_task);
>> + if (err)
>> + return err;
>> + list_add(&new_hugepage_task->list, task_monitor);
>> + return 0;
>> +}
>> +
>> +static int damon_hugepage_handle_commit_inputs(
>> + struct list_head *monitored_tasks)
>> +{
>> + int i = 0;
>> + int err = 0;
>> + bool found;
>> + struct hugepage_task *monitored_task, *tmp;
>> +
>> + if (!commit_inputs)
>> + return 0;
>> +
>> + while (i < MAX_MONITORED_PIDS) {
>> + if (!monitored_pids[i])
>> + break;
>> +
>> + found = false;
>> +
>> + rcu_read_lock();
>> + if (!find_vpid(monitored_pids[i])) {
>> + rcu_read_unlock();
>> + continue;
>> + }
>> +
>> + rcu_read_unlock();
>> +
>> + list_for_each_entry_safe(monitored_task, tmp, monitored_tasks, list) {
>
> Please wrap lines for the 80 columns limit.

Interesting, when check it, i was less than 80 columns. Even if I copy this line
and paste it into any text editor, I still get less than 80 characters.

>
>> + if (monitored_task->pid == monitored_pids[i]) {
>> + list_move(&monitored_task->list, monitored_tasks);
>> + found = true;
>> + break;
>> + }
>> + }
>> + if (!found) {
>> + err = add_monitored_task(monitored_pids[i], monitored_tasks);
>> + /* Skip failed tasks */
>> + if (err)
>> + continue;
>> + }
>> + i++;
>> + }
>> +
>> + i = 0;
>> + list_for_each_entry_safe(monitored_task, tmp, monitored_tasks, list) {
>> + i++;
>> + if (i <= num_monitored_pids)
>> + continue;
>> +
>> + err = damon_stop(&monitored_task->ctx, 1);
>> + damon_destroy_ctx(monitored_task->ctx);
>> + list_del(&monitored_task->list);
>> + kfree(monitored_task);
>> + }
>> +
>> + commit_inputs = false;
>> + return err;
>> +}
>> +
>> +static int damon_manager_monitor_thread(void *data)
>> +{
>> + int err = 0;
>> + int i;
>> + struct hugepage_task *entry, *tmp;
>> +
>> + LIST_HEAD(monitored_tasks);
>> +
>> + for (i = 0; i < MAX_MONITORED_PIDS; i++) {
>> + if (!monitored_pids[i])
>> + break;
>> +
>> + rcu_read_lock();
>> + if (!find_vpid(monitored_pids[i])) {
>> + rcu_read_unlock();
>> + continue;
>> + }
>> + rcu_read_unlock();
>> +
>> + add_monitored_task(monitored_pids[i], &monitored_tasks);
>> + }
>> +
>> +
>> + while (!kthread_should_stop()) {
>> + schedule_timeout_idle(usecs_to_jiffies(monitor_period));
>> + err = damon_hugepage_handle_commit_inputs(&monitored_tasks);
>> + if (err)
>> + break;
>> + }
>> +
>> + list_for_each_entry_safe(entry, tmp, &monitored_tasks, list) {
>> + err = damon_stop(&entry->ctx, 1);
>> + damon_destroy_ctx(entry->ctx);
>> + }
>> +
>> + for (int i = 0; i < MAX_MONITORED_PIDS;) {
>
> You can just resue i after initializing here, instead of defining it again.

Again, left overs from the previous refactor. My bad.

>
>> + monitored_pids[i] = 0;
>> + i++;
>> + }
>> + return err;
>> +}
>> +
>> +static int damon_hugepage_start_monitor_thread(void)
>> +{
>> + num_monitored_pids = 0;
>> + monitor_thread = kthread_create(damon_manager_monitor_thread, NULL,
>> + "damon_dynamic");
>> +
>> + if (IS_ERR(monitor_thread))
>> + return PTR_ERR(monitor_thread);
>> +
>> + wake_up_process(monitor_thread);
>> + return 0;
>> +}
>> +
>> +static int damon_hugepage_turn(bool on)
>> +{
>> + int err = 0;
>> +
>> + mutex_lock(&enable_disable_lock);
>> + if (!on) {
>> + if (monitor_thread) {
>> + kthread_stop(monitor_thread);
>> + monitor_thread = NULL;
>> + }
>> + goto out;
>> + }
>> + err = damon_hugepage_start_monitor_thread();
>> +out:
>> + mutex_unlock(&enable_disable_lock);
>> + return err;
>> +}
>> +
>> +static int damon_hugepage_enabled_store(const char *val,
>> + const struct kernel_param *kp)
>> +{
>> + bool is_enabled = enabled;
>> + bool enable;
>> + int err;
>> +
>> + err = kstrtobool(val, &enable);
>> + if (err)
>> + return err;
>> +
>> + if (is_enabled == enable)
>> + return 0;
>> +
>> + err = damon_hugepage_turn(enable);
>> + if (err)
>> + return err;
>> +
>> + enabled = enable;
>> + return err;
>> +}
>> +
>> +static const struct kernel_param_ops enabled_param_ops = {
>> + .set = damon_hugepage_enabled_store,
>> + .get = param_get_bool,
>> +};
>> +
>> +module_param_cb(enabled, &enabled_param_ops, &enabled, 0600);
>> +MODULE_PARM_DESC(enabled,
>> + "Enable or disable DAMON_DYNAMIC_HUGEPAGES (default: disabled)");
>
> s/DYNAMIC_// ?

OK

>
>> +
>> +static int __init damon_hugepage_init(void)
>> +{
>> + int err;
>> +
>> + /* 'enabled' has set before this function, probably via command line */
>> + if (enabled)
>> + err = damon_hugepage_turn(true);
>> +
>> + if (err && enabled)
>> + enabled = false;
>> + return err;
>> +}
>> +
>> +module_init(damon_hugepage_init);
>> --
>> 2.43.0
>
> [1] https://lore.kernel.org/00aa6914-8e52-49a2-9875-588efc096c5e@huawei-partners.com
>
>
> Thanks,
> SJ
>

--
Asier Gutierrez
Huawei