public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v3 00/10] mm/damon: introduce DAMOS failed region quota charge ratio
@ 2026-04-07  1:05 SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 01/10] mm/damon/core: handle <min_region_sz remaining quota as empty SeongJae Park
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: SeongJae Park @ 2026-04-07  1:05 UTC (permalink / raw)
  Cc: SeongJae Park, Liam R. Howlett, Andrew Morton, Brendan Higgins,
	David Gow, David Hildenbrand, Jonathan Corbet, Lorenzo Stoakes,
	Michal Hocko, Mike Rapoport, Shuah Khan, Shuah Khan,
	Suren Baghdasaryan, Vlastimil Babka, damon, kunit-dev, linux-doc,
	linux-kernel, linux-kselftest, linux-mm

TL; DR: Let users set different DAMOS quota charge ratios for DAMOS
action failed regions, for deterministic and consistent DAMOS action
progress.

Common Reports: Unexpectedly Slow DAMOS
=======================================

One common issue report that we get from DAMON users is that DAMOS
action applying progress speed is sometimes much slower than expected.
And one common root cause is that the DAMOS quota is exceeded by the
action applying failed memory regions.

For example, a group of users tried to run DAMOS-based proactive memory
reclamation (DAMON_RECLAIM) with 100 MiB per second DAMOS quota.  They
ran it on a system having no active workload which means all memory of
the system is cold.  The expectation was that the system will show 100
MiB per second reclamation until (nearly) all memory is reclaimed. But
what they found is that the speed is quite inconsistent and sometimes it
becomes very slower than the expectation, sometimes even no reclamation
at all for about tens of seconds.  The upper limit of the speed (100 MiB
per second) was being kept as expected, though.

By monitoring the qt_exceeds (number of DAMOS quota exceed events) DAMOS
stat, we found DAMOS quota is always exceeded when the speed is slow. By
monitoring sz_tried and sz_applied (the total amount of DAMOS action
tried memory and succeeded memory) DAMOS stats together, we found the
reclamation attempts nearly always failed when the speed is slow.

DAMOS quota charges DAMOS action tried regions regardless of the
successfulness of the try.  Hence in the example reported case, there
was unreclaimable memory spread around the system memory.  Sometimes
nearly 100 MiB of memory that DAMOS tried to reclaim in the given quota
interval was reclaimable, and therefore showed nearly 100 MiB per second
speed.  Sometimes nearly 99 MiB of memory that DAMOS was trying to
reclaim in the given quota interval was unreclaimable, and therefore
showing only about 1 MiB per second reclaim speed.

We explained it is an expected behavior of the feature rather than a
bug, as DAMOS quota is there for only the upper-limit of the speed.  The
users agreed and later reported a huge win from the adoption of
DAMON_RECLAIM on their products.

It is Not a Bug but a Feature; But...
=====================================

So nothing is broken.  DAMOS quota is working as intended, as the upper
limit of the speed.  It also provides its behavior observability via
DAMOS stat.  In the real world production environment that runs long
term active workloads and matters stability, the speed sometimes being
slow is not a real problem.

But, the non-deterministic behavior is sometimes annoying, especially in
lab environments.  Even in a realistic production environment, when
there is a huge amount of DAMOS action unapplicable memory, the speed
could be problematically slow.  Let's suppose a virtual machines
provider that setup 99% of the host memory as hugetlb pages that cannot
be reclaimed, to give it to virtual machines.  Also, when aim-oriented
DAMOS auto-tuning is applied, this could also make the internal feedback
loop confused.

The intention of the current behavior was that trying DAMOS action to
regions would anyway impose some overhead, and therefore somehow be
charged.  But in the real world, the overhead for failed action is much
lighter than successful action.  Charging those at the same ratio may be
unfair, or at least suboptimum in some environments.

DAMOS Action Failed Region Quota Charge Ratio
=============================================

Let users set the charge ratio for the action-failed memory, for more
optimal and deterministic use of DAMOS.  It allows users to specify the
numerator and the denominator of the ratio for flexible setup.  For
example, let's suppose the numerator and the denominator are set to 1
and 4,096, respectively.  The ratio is 1 / 4,096.  A DAMOS scheme action
is applied to 5 GiB memory.  For 1 GiB of the memory, the action is
succeeded.  For the rest (4 GiB), the action is failed.  Then, only 1
GiB and 1 MiB quota is charged.

The optimal charge ratio will depend on the use case and
system/workload.  I'd recommend starting from setting the nominator as 1
and the denominator as PAGE_SIZE and tune based on the results, because
many DAMOS actions are applied at page level.

Tests
=====

I tested this feature in the steps below.

1. Allocate 50% of system memory and mlock() it using a test program.
2. Fill up the page cache to exhaust nearly all free memory.
3. Start DAMON-based proactive reclamation with 100 MiB/second DAMOS
   hard-quota.  Auto-tune the DAMOS soft-quota under the hard-quota for
   achieving 40% free memory of the system with 'temporal' tuner.

For step 1, I run a simple C program that is written by Gemini.  It is
quite straightforward, so I'm not sharing the code here.

For step 2, I use dd command like below:

   dd if=/dev/zero of=foo bs=1M count=$50_percent_of_system_memory

For step 3, I use the latest version of DAMON user-space tool (damo)
like below.

    sudo damo start --damos_action pageout \
            ` # Do the pageout only up to 100 MiB per second ` \
            --damos_quota_space 100M --damos_quota_interval 1s \
            ` # Auto-tune the quota below the hard quota aiming` \
            ` # 40% free memory of the node 0 ` \
            ` # (entire node of the test system)` \
            --damos_quota_goal node_mem_free_bp 40% 0 \
            ` # use temporal tuner, which is easy to understnd ` \
            --damos_quota_goal_tuner temporal

As expected, the progress of the reclamation is not consistent, because
the quota is exceeded for the failed reclamation of the unreclaimable
memory.

I do this again, but with the failed region charge ratio feature.  For
this, the above 'damo' command is used, after appending command line
option for setup of the charge ratio like below.  Note that the option
was added to 'damo' after v3.1.9.

    sudo ./damo start --damos_action pageout \
            [...]
            ` # quota-charge only 1/4096 for pageout-failed regions ` \
            --damos_quota_fail_charge_ratio 1 4096

The progress of the reclamation was nearly 100 MiB per second until the
goal was achieved, meeting the expectation.

Patches Sequence
================

Patch 1 updates fully charged quota check to handle <min_region_sz
remaining quota, which will be able to exist after this series is
applied.  Patch 2 implements the feature and exposes it via DAMON core
API.  Patch 3 implements DAMON sysfs ABI for the feature.  Three
following patches (4-6) document the feature and ABI on design, usage,
and ABI documents, respectively.  Four patches for testing of the new
feature follow.  Patch 7 implements a kunit test for the feature.
Patches 8 and 9 extend DAMON selftest helpers for DAMON sysfs control
and internal state dumping for adding a new selftest for the feature.
Patch 10 extends existing DAMON sysfs interface selftest to test the new
feature using the extended helper scripts.

Changelog
=========

Changes from RFC v2
(https://lore.kernel.org/20260405151232.102690-1-sj@kernel.org)
- Handle <min_region_sz remaining quota.
- Document zero denum behavior.
- Fix typos: s/selftets/selftests/
Changes from RFC v1
(https://lore.kernel.org/20260404163943.89278-1-sj@kernel.org)
- Avoid overflows in charge amount calculation.
- Fix/wordsmith documentation for grammar, typo, and wrong examples.
- Improve unit test for more consistent comparison source use.

SeongJae Park (10):
  mm/damon/core: handle <min_region_sz remaining quota as empty
  mm/damon/core: introduce failed region quota charge ratio
  mm/damon/sysfs-schemes: implement fail_charge_{num,denom} files
  Docs/mm/damon/design: document fail_charge_{num,denom}
  Docs/admin-guide/mm/damon/usage: document fail_charge_{num,denom}
    files
  Docs/ABI/damon: document fail_charge_{num,denom}
  mm/damon/tests/core-kunit: test fail_charge_{num,denom} committing
  selftests/damon/_damon_sysfs: support failed region quota charge ratio
  selftests/damon/drgn_dump_damon_status: support failed region quota
    charge ratio
  selftests/damon/sysfs.py: test failed region quota charge ratio

 .../ABI/testing/sysfs-kernel-mm-damon         | 12 +++++
 Documentation/admin-guide/mm/damon/usage.rst  | 18 +++++--
 Documentation/mm/damon/design.rst             | 22 ++++++++
 include/linux/damon.h                         |  9 ++++
 mm/damon/core.c                               | 38 ++++++++++---
 mm/damon/sysfs-schemes.c                      | 54 +++++++++++++++++++
 mm/damon/tests/core-kunit.h                   |  6 +++
 tools/testing/selftests/damon/_damon_sysfs.py | 21 +++++++-
 .../selftests/damon/drgn_dump_damon_status.py |  2 +
 tools/testing/selftests/damon/sysfs.py        |  6 +++
 10 files changed, 175 insertions(+), 13 deletions(-)


base-commit: b1ca86c92674eaf92a32ce3a2d89a0349e406df1
-- 
2.47.3

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH v3 01/10] mm/damon/core: handle <min_region_sz remaining quota as empty
  2026-04-07  1:05 [RFC PATCH v3 00/10] mm/damon: introduce DAMOS failed region quota charge ratio SeongJae Park
@ 2026-04-07  1:05 ` SeongJae Park
  2026-04-07 14:12   ` (sashiko review) " SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 02/10] mm/damon/core: introduce failed region quota charge ratio SeongJae Park
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: SeongJae Park @ 2026-04-07  1:05 UTC (permalink / raw)
  Cc: SeongJae Park, Andrew Morton, damon, linux-kernel, linux-mm

Less than min_region_sz remaining quota effectively means the quota is
fully charged.  In other words, no remaining quota.  This is because
DAMOS actions are applied in the region granularity, and each region
should have min_region_sz or larger size.  However the existing fully
charged quota check, which is also used for setting charge_target_from
and charge_addr_from of the quota, is not aware of the case.  For the
reason, charge_target_from and charge_addr_from of the quota will not be
updated in the case.  This can result in DAMOS action being applied more
frequently to a specific area of the memory.

The case is unreal because quota charging is also made in the region
granularity.  It could be changed in future, though.  Actually, the
following commit will make the change, by allowing users to set
arbitrary quota charging ratio for action-failed regions.  To be
prepared for the change, update the fully charged quota checks to treat
having less than min_region_sz remaining quota as fully charged.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index e680716972506..2a2f767d1a46e 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2042,6 +2042,14 @@ static void damos_walk_cancel(struct damon_ctx *ctx)
 	mutex_unlock(&ctx->walk_control_lock);
 }
 
+static bool damos_quota_is_full(struct damos_quota *quota,
+		unsigned long min_region_sz)
+{
+	if (!damos_quota_is_set(quota))
+		return false;
+	return quota->charged_sz + min_region_sz > quota->esz;
+}
+
 static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
 		struct damon_region *r, struct damos *s)
 {
@@ -2099,8 +2107,7 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
 		quota->total_charged_ns += timespec64_to_ns(&end) -
 			timespec64_to_ns(&begin);
 		quota->charged_sz += sz;
-		if (damos_quota_is_set(quota) &&
-				quota->charged_sz >= quota->esz) {
+		if (damos_quota_is_full(quota, c->min_region_sz)) {
 			quota->charge_target_from = t;
 			quota->charge_addr_from = r->ar.end + 1;
 		}
@@ -2128,8 +2135,7 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 			continue;
 
 		/* Check the quota */
-		if (damos_quota_is_set(quota) &&
-				quota->charged_sz >= quota->esz)
+		if (damos_quota_is_full(quota, c->min_region_sz))
 			continue;
 
 		if (damos_skip_charged_region(t, r, s, c->min_region_sz))
@@ -2455,8 +2461,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 	if (!time_in_range_open(jiffies, quota->charged_from,
 				quota->charged_from +
 				msecs_to_jiffies(quota->reset_interval))) {
-		if (damos_quota_is_set(quota) &&
-				quota->charged_sz >= quota->esz)
+		if (damos_quota_is_full(quota, c->min_region_sz))
 			s->stat.qt_exceeds++;
 		quota->total_charged_sz += quota->charged_sz;
 		quota->charged_from = jiffies;
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH v3 02/10] mm/damon/core: introduce failed region quota charge ratio
  2026-04-07  1:05 [RFC PATCH v3 00/10] mm/damon: introduce DAMOS failed region quota charge ratio SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 01/10] mm/damon/core: handle <min_region_sz remaining quota as empty SeongJae Park
@ 2026-04-07  1:05 ` SeongJae Park
  2026-04-07 15:51   ` (sashiko review) " SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 03/10] mm/damon/sysfs-schemes: implement fail_charge_{num,denom} files SeongJae Park
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: SeongJae Park @ 2026-04-07  1:05 UTC (permalink / raw)
  Cc: SeongJae Park, Andrew Morton, damon, linux-kernel, linux-mm

DAMOS quota is charged to all DAMOS action application attempted memory,
regardless of how much of the memory the action was successful and
failed.  This makes understanding quota behavior without DAMOS stat but
only with end level metrics (e.g., increased amount of free memory for
DAMOS_PAGEOUT action) difficult.  Also, charging action-failed memory
same as action-successful memory is somewhat unfair, as successful
action application will induce more overhead in most cases.

Introduce DAMON core API for setting the charge ratio for such
action-failed memory.  It allows API callers to specify the ratio in a
flexible way, by setting the numerator and the denominator.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/linux/damon.h |  9 +++++++++
 mm/damon/core.c       | 21 ++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 4b69f4553267d..9ab7331775b9e 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -233,6 +233,8 @@ enum damos_quota_goal_tuner {
  * @goals:		Head of quota tuning goals (&damos_quota_goal) list.
  * @goal_tuner:		Goal-based @esz tuning algorithm to use.
  * @esz:		Effective size quota in bytes.
+ * @fail_charge_num:	Failed regions charge rate numerator.
+ * @fail_charge_denom:	Failed regions charge rate denominator.
  *
  * @weight_sz:		Weight of the region's size for prioritization.
  * @weight_nr_accesses:	Weight of the region's nr_accesses for prioritization.
@@ -262,6 +264,10 @@ enum damos_quota_goal_tuner {
  *
  * The resulting effective size quota in bytes is set to @esz.
  *
+ * For DAMOS action applying failed amount of regions, charging those same to
+ * those that the action has successfully applied may be unfair.  For the
+ * reason, 'the size * @fail_charge_num / @fail_charge_denom' is charged.
+ *
  * For selecting regions within the quota, DAMON prioritizes current scheme's
  * target memory regions using the &struct damon_operations->get_scheme_score.
  * You could customize the prioritization logic by setting &weight_sz,
@@ -276,6 +282,9 @@ struct damos_quota {
 	enum damos_quota_goal_tuner goal_tuner;
 	unsigned long esz;
 
+	unsigned int fail_charge_num;
+	unsigned int fail_charge_denom;
+
 	unsigned int weight_sz;
 	unsigned int weight_nr_accesses;
 	unsigned int weight_age;
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 2a2f767d1a46e..692bd3e5dfe09 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -918,6 +918,8 @@ static int damos_commit_quota(struct damos_quota *dst, struct damos_quota *src)
 	if (err)
 		return err;
 	dst->goal_tuner = src->goal_tuner;
+	dst->fail_charge_num = src->fail_charge_num;
+	dst->fail_charge_denom = src->fail_charge_denom;
 	dst->weight_sz = src->weight_sz;
 	dst->weight_nr_accesses = src->weight_nr_accesses;
 	dst->weight_age = src->weight_age;
@@ -2042,6 +2044,23 @@ static void damos_walk_cancel(struct damon_ctx *ctx)
 	mutex_unlock(&ctx->walk_control_lock);
 }
 
+static void damos_charge_quota(struct damos_quota *quota,
+		unsigned long sz_region, unsigned long sz_applied)
+{
+	/*
+	 * sz_applied could be bigger than sz_region, depending on ops
+	 * implementation of the action, e.g., damos_pa_pageout().  Charge only
+	 * the region size in the case.
+	 */
+	if (!quota->fail_charge_denom || sz_applied > sz_region)
+		quota->charged_sz += sz_region;
+	else
+		quota->charged_sz += sz_applied + mult_frac(
+				(sz_region - sz_applied),
+				quota->fail_charge_num,
+				quota->fail_charge_denom);
+}
+
 static bool damos_quota_is_full(struct damos_quota *quota,
 		unsigned long min_region_sz)
 {
@@ -2106,7 +2125,7 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
 		ktime_get_coarse_ts64(&end);
 		quota->total_charged_ns += timespec64_to_ns(&end) -
 			timespec64_to_ns(&begin);
-		quota->charged_sz += sz;
+		damos_charge_quota(quota, sz, sz_applied);
 		if (damos_quota_is_full(quota, c->min_region_sz)) {
 			quota->charge_target_from = t;
 			quota->charge_addr_from = r->ar.end + 1;
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH v3 03/10] mm/damon/sysfs-schemes: implement fail_charge_{num,denom} files
  2026-04-07  1:05 [RFC PATCH v3 00/10] mm/damon: introduce DAMOS failed region quota charge ratio SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 01/10] mm/damon/core: handle <min_region_sz remaining quota as empty SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 02/10] mm/damon/core: introduce failed region quota charge ratio SeongJae Park
@ 2026-04-07  1:05 ` SeongJae Park
  2026-04-07 15:55   ` (sashiko review) " SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 04/10] Docs/mm/damon/design: document fail_charge_{num,denom} SeongJae Park
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: SeongJae Park @ 2026-04-07  1:05 UTC (permalink / raw)
  Cc: SeongJae Park, Andrew Morton, damon, linux-kernel, linux-mm

Implement the user-space ABI for the DAMOS action failed region
quota-charge ratio setup.  For this, add two new sysfs files under the
DAMON sysfs interface for DAMOS quotas.  Names of the files are
fail_charge_num and fail_charge_denom, and work for reading and setting
the numerator and denominator of the failed regions charge ratio.

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

diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 5186966dafb35..d5bfba79f105f 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -1489,6 +1489,8 @@ struct damon_sysfs_quotas {
 	unsigned long reset_interval_ms;
 	unsigned long effective_sz;	/* Effective size quota in bytes */
 	enum damos_quota_goal_tuner goal_tuner;
+	unsigned int fail_charge_num;
+	unsigned int fail_charge_denom;
 };
 
 static struct damon_sysfs_quotas *damon_sysfs_quotas_alloc(void)
@@ -1663,6 +1665,48 @@ static ssize_t goal_tuner_store(struct kobject *kobj,
 	return -EINVAL;
 }
 
+static ssize_t fail_charge_num_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	struct damon_sysfs_quotas *quotas = container_of(kobj,
+			struct damon_sysfs_quotas, kobj);
+
+	return sysfs_emit(buf, "%u\n", quotas->fail_charge_num);
+}
+
+static ssize_t fail_charge_num_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	struct damon_sysfs_quotas *quotas = container_of(kobj,
+			struct damon_sysfs_quotas, kobj);
+	int err = kstrtouint(buf, 0, &quotas->fail_charge_num);
+
+	if (err)
+		return -EINVAL;
+	return count;
+}
+
+static ssize_t fail_charge_denom_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	struct damon_sysfs_quotas *quotas = container_of(kobj,
+			struct damon_sysfs_quotas, kobj);
+
+	return sysfs_emit(buf, "%u\n", quotas->fail_charge_denom);
+}
+
+static ssize_t fail_charge_denom_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	struct damon_sysfs_quotas *quotas = container_of(kobj,
+			struct damon_sysfs_quotas, kobj);
+	int err = kstrtouint(buf, 0, &quotas->fail_charge_denom);
+
+	if (err)
+		return -EINVAL;
+	return count;
+}
+
 static void damon_sysfs_quotas_release(struct kobject *kobj)
 {
 	kfree(container_of(kobj, struct damon_sysfs_quotas, kobj));
@@ -1683,12 +1727,20 @@ static struct kobj_attribute damon_sysfs_quotas_effective_bytes_attr =
 static struct kobj_attribute damon_sysfs_quotas_goal_tuner_attr =
 		__ATTR_RW_MODE(goal_tuner, 0600);
 
+static struct kobj_attribute damon_sysfs_quotas_fail_charge_num_attr =
+		__ATTR_RW_MODE(fail_charge_num, 0600);
+
+static struct kobj_attribute damon_sysfs_quotas_fail_charge_denom_attr =
+		__ATTR_RW_MODE(fail_charge_denom, 0600);
+
 static struct attribute *damon_sysfs_quotas_attrs[] = {
 	&damon_sysfs_quotas_ms_attr.attr,
 	&damon_sysfs_quotas_sz_attr.attr,
 	&damon_sysfs_quotas_reset_interval_ms_attr.attr,
 	&damon_sysfs_quotas_effective_bytes_attr.attr,
 	&damon_sysfs_quotas_goal_tuner_attr.attr,
+	&damon_sysfs_quotas_fail_charge_num_attr.attr,
+	&damon_sysfs_quotas_fail_charge_denom_attr.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(damon_sysfs_quotas);
@@ -2776,6 +2828,8 @@ static struct damos *damon_sysfs_mk_scheme(
 		.weight_nr_accesses = sysfs_weights->nr_accesses,
 		.weight_age = sysfs_weights->age,
 		.goal_tuner = sysfs_quotas->goal_tuner,
+		.fail_charge_num = sysfs_quotas->fail_charge_num,
+		.fail_charge_denom = sysfs_quotas->fail_charge_denom,
 	};
 	struct damos_watermarks wmarks = {
 		.metric = sysfs_wmarks->metric,
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH v3 04/10] Docs/mm/damon/design: document fail_charge_{num,denom}
  2026-04-07  1:05 [RFC PATCH v3 00/10] mm/damon: introduce DAMOS failed region quota charge ratio SeongJae Park
                   ` (2 preceding siblings ...)
  2026-04-07  1:05 ` [RFC PATCH v3 03/10] mm/damon/sysfs-schemes: implement fail_charge_{num,denom} files SeongJae Park
@ 2026-04-07  1:05 ` SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 05/10] Docs/admin-guide/mm/damon/usage: document fail_charge_{num,denom} files SeongJae Park
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: SeongJae Park @ 2026-04-07  1:05 UTC (permalink / raw)
  Cc: SeongJae Park, Liam R. Howlett, Andrew Morton, David Hildenbrand,
	Jonathan Corbet, Lorenzo Stoakes, Michal Hocko, Mike Rapoport,
	Shuah Khan, Suren Baghdasaryan, Vlastimil Babka, damon, linux-doc,
	linux-kernel, linux-mm

Update DAMON design document for the DAMOS action failed region quota
charge ratio.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 Documentation/mm/damon/design.rst | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
index 510ec6375178d..94e898b671d15 100644
--- a/Documentation/mm/damon/design.rst
+++ b/Documentation/mm/damon/design.rst
@@ -572,6 +572,28 @@ interface <sysfs_interface>`, refer to :ref:`weights <sysfs_quotas>` part of
 the documentation.
 
 
+.. _damon_design_damos_quotas_failed_memory_charging_ratio:
+
+Action-failed Memory Charging Ratio
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+DAMOS action to a given region can fail for some subsets of the memory of the
+region.  For example, if the action is ``pageout`` and the region has some
+unreclaimable pages, applying the action to the pages will fail.  The amount of
+system resource that is taken for such failed action applications is usually
+different from that for successful action applications.  For such cases, users
+can set different charging ratio for such failed memory.  The ratio can be
+specified using ``fail_charge_num`` and ``fail_charge_denom`` parameters.  The
+two parameters represent the numerator and denominator of the ratio.  The
+feature is enabled only if ``fail_charge_denom`` is not zero.
+
+For example, let's suppose a DAMOS action is applied to a region of 1,000 MiB
+size.  The action is successfully applied to only 700 MiB of the region.
+``fail_charge_num`` and ``fail_charge_denom`` are set to ``1`` and ``1024``,
+respectively.  Then only 700 MiB and 300 KiB of size (``700 MiB + 300 MiB * 1 /
+1024``) will be charged.
+
+
 .. _damon_design_damos_quotas_auto_tuning:
 
 Aim-oriented Feedback-driven Auto-tuning
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH v3 05/10] Docs/admin-guide/mm/damon/usage: document fail_charge_{num,denom} files
  2026-04-07  1:05 [RFC PATCH v3 00/10] mm/damon: introduce DAMOS failed region quota charge ratio SeongJae Park
                   ` (3 preceding siblings ...)
  2026-04-07  1:05 ` [RFC PATCH v3 04/10] Docs/mm/damon/design: document fail_charge_{num,denom} SeongJae Park
@ 2026-04-07  1:05 ` SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 06/10] Docs/ABI/damon: document fail_charge_{num,denom} SeongJae Park
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: SeongJae Park @ 2026-04-07  1:05 UTC (permalink / raw)
  Cc: SeongJae Park, Liam R. Howlett, Andrew Morton, David Hildenbrand,
	Jonathan Corbet, Lorenzo Stoakes, Michal Hocko, Mike Rapoport,
	Shuah Khan, Suren Baghdasaryan, Vlastimil Babka, damon, linux-doc,
	linux-kernel, linux-mm

Update DAMON usage document for the DAMOS action failed regions quota
charge ratio control sysfs files.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 Documentation/admin-guide/mm/damon/usage.rst | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/mm/damon/usage.rst b/Documentation/admin-guide/mm/damon/usage.rst
index bfdb717441f05..d5548e460857c 100644
--- a/Documentation/admin-guide/mm/damon/usage.rst
+++ b/Documentation/admin-guide/mm/damon/usage.rst
@@ -84,7 +84,9 @@ comma (",").
     │ │ │ │ │ │ │ │ sz/min,max
     │ │ │ │ │ │ │ │ nr_accesses/min,max
     │ │ │ │ │ │ │ │ age/min,max
-    │ │ │ │ │ │ │ :ref:`quotas <sysfs_quotas>`/ms,bytes,reset_interval_ms,effective_bytes,goal_tuner
+    │ │ │ │ │ │ │ :ref:`quotas <sysfs_quotas>`/ms,bytes,reset_interval_ms,
+    │ │ │ │ │ │ │     effective_bytes,goal_tuner,
+    │ │ │ │ │ │ │     fail_charge_num,fail_charge_denom
     │ │ │ │ │ │ │ │ weights/sz_permil,nr_accesses_permil,age_permil
     │ │ │ │ │ │ │ │ :ref:`goals <sysfs_schemes_quota_goals>`/nr_goals
     │ │ │ │ │ │ │ │ │ 0/target_metric,target_value,current_value,nid,path
@@ -381,9 +383,10 @@ schemes/<N>/quotas/
 The directory for the :ref:`quotas <damon_design_damos_quotas>` of the given
 DAMON-based operation scheme.
 
-Under ``quotas`` directory, five files (``ms``, ``bytes``,
-``reset_interval_ms``, ``effective_bytes`` and ``goal_tuner``) and two
-directories (``weights`` and ``goals``) exist.
+Under ``quotas`` directory, seven files (``ms``, ``bytes``,
+``reset_interval_ms``, ``effective_bytes``, ``goal_tuner``, ``fail_charge_num``
+and ``fail_charge_denom``) and two directories (``weights`` and ``goals``)
+exist.
 
 You can set the ``time quota`` in milliseconds, ``size quota`` in bytes, and
 ``reset interval`` in milliseconds by writing the values to the three files,
@@ -402,6 +405,13 @@ the background design of the feature and the name of the selectable algorithms.
 Refer to :ref:`goals directory <sysfs_schemes_quota_goals>` for the goals
 setup.
 
+You can set the action-failed memory quota charging ratio by writing the
+numerator and the denominator for the ratio to ``fail_charge_num`` and
+``fail_charge_denom`` files, respectively.  Reading those files will return the
+current set values.  Refer to :ref:`design
+<damon_design_damos_quotas_failed_memory_charging_ratio>` for more details of
+the ratio feature.
+
 The time quota is internally transformed to a size quota.  Between the
 transformed size quota and user-specified size quota, smaller one is applied.
 Based on the user-specified :ref:`goal <sysfs_schemes_quota_goals>`, the
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH v3 06/10] Docs/ABI/damon: document fail_charge_{num,denom}
  2026-04-07  1:05 [RFC PATCH v3 00/10] mm/damon: introduce DAMOS failed region quota charge ratio SeongJae Park
                   ` (4 preceding siblings ...)
  2026-04-07  1:05 ` [RFC PATCH v3 05/10] Docs/admin-guide/mm/damon/usage: document fail_charge_{num,denom} files SeongJae Park
@ 2026-04-07  1:05 ` SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 07/10] mm/damon/tests/core-kunit: test fail_charge_{num,denom} committing SeongJae Park
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: SeongJae Park @ 2026-04-07  1:05 UTC (permalink / raw)
  Cc: SeongJae Park, damon, linux-kernel, linux-mm

Update DAMON ABI document for the DAMOS action failed regions quota
charge ratio control sysfs files.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 Documentation/ABI/testing/sysfs-kernel-mm-damon | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-damon b/Documentation/ABI/testing/sysfs-kernel-mm-damon
index 7059f540940f0..971c22e34e722 100644
--- a/Documentation/ABI/testing/sysfs-kernel-mm-damon
+++ b/Documentation/ABI/testing/sysfs-kernel-mm-damon
@@ -329,6 +329,18 @@ Contact:	SeongJae Park <sj@kernel.org>
 Description:	Writing to and reading from this file sets and gets the
 		goal-based effective quota auto-tuning algorithm to use.
 
+What:		/sys/kernel/mm/damon/admin/kdamonds/<K>/contexts/<C>/schemes/<S>/quotas/fail_charge_num
+Date:		Mar 2026
+Contact:	SeongJae Park <sj@kernel.org>
+Description:	Writing to and reading from this file sets and gets the
+		action-failed memory quota charging ratio numerator.
+
+What:		/sys/kernel/mm/damon/admin/kdamonds/<K>/contexts/<C>/schemes/<S>/quotas/fail_charge_denom
+Date:		Mar 2026
+Contact:	SeongJae Park <sj@kernel.org>
+Description:	Writing to and reading from this file sets and gets the
+		action-failed memory quota charging ratio denominator.
+
 What:		/sys/kernel/mm/damon/admin/kdamonds/<K>/contexts/<C>/schemes/<S>/quotas/weights/sz_permil
 Date:		Mar 2022
 Contact:	SeongJae Park <sj@kernel.org>
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH v3 07/10] mm/damon/tests/core-kunit: test fail_charge_{num,denom} committing
  2026-04-07  1:05 [RFC PATCH v3 00/10] mm/damon: introduce DAMOS failed region quota charge ratio SeongJae Park
                   ` (5 preceding siblings ...)
  2026-04-07  1:05 ` [RFC PATCH v3 06/10] Docs/ABI/damon: document fail_charge_{num,denom} SeongJae Park
@ 2026-04-07  1:05 ` SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 08/10] selftests/damon/_damon_sysfs: support failed region quota charge ratio SeongJae Park
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: SeongJae Park @ 2026-04-07  1:05 UTC (permalink / raw)
  Cc: SeongJae Park, Andrew Morton, Brendan Higgins, David Gow, damon,
	kunit-dev, linux-kernel, linux-kselftest, linux-mm

Extend damos_test_commit_quotas() kunit test to ensure
damos_commit_quota() handles fail_charge_{num,denom} parameters.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/tests/core-kunit.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 0030f682b23b7..1b23a22ac04c4 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -694,6 +694,8 @@ static void damos_test_commit_quota(struct kunit *test)
 		.ms = 2,
 		.sz = 3,
 		.goal_tuner = DAMOS_QUOTA_GOAL_TUNER_CONSIST,
+		.fail_charge_num = 2,
+		.fail_charge_denom = 3,
 		.weight_sz = 4,
 		.weight_nr_accesses = 5,
 		.weight_age = 6,
@@ -703,6 +705,8 @@ static void damos_test_commit_quota(struct kunit *test)
 		.ms = 8,
 		.sz = 9,
 		.goal_tuner = DAMOS_QUOTA_GOAL_TUNER_TEMPORAL,
+		.fail_charge_num = 1,
+		.fail_charge_denom = 1024,
 		.weight_sz = 10,
 		.weight_nr_accesses = 11,
 		.weight_age = 12,
@@ -717,6 +721,8 @@ static void damos_test_commit_quota(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, dst.ms, src.ms);
 	KUNIT_EXPECT_EQ(test, dst.sz, src.sz);
 	KUNIT_EXPECT_EQ(test, dst.goal_tuner, src.goal_tuner);
+	KUNIT_EXPECT_EQ(test, dst.fail_charge_num, src.fail_charge_num);
+	KUNIT_EXPECT_EQ(test, dst.fail_charge_denom, src.fail_charge_denom);
 	KUNIT_EXPECT_EQ(test, dst.weight_sz, src.weight_sz);
 	KUNIT_EXPECT_EQ(test, dst.weight_nr_accesses, src.weight_nr_accesses);
 	KUNIT_EXPECT_EQ(test, dst.weight_age, src.weight_age);
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH v3 08/10] selftests/damon/_damon_sysfs: support failed region quota charge ratio
  2026-04-07  1:05 [RFC PATCH v3 00/10] mm/damon: introduce DAMOS failed region quota charge ratio SeongJae Park
                   ` (6 preceding siblings ...)
  2026-04-07  1:05 ` [RFC PATCH v3 07/10] mm/damon/tests/core-kunit: test fail_charge_{num,denom} committing SeongJae Park
@ 2026-04-07  1:05 ` SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 09/10] selftests/damon/drgn_dump_damon_status: " SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 10/10] selftests/damon/sysfs.py: test " SeongJae Park
  9 siblings, 0 replies; 14+ messages in thread
From: SeongJae Park @ 2026-04-07  1:05 UTC (permalink / raw)
  Cc: SeongJae Park, Shuah Khan, damon, linux-kernel, linux-kselftest,
	linux-mm

Extend _damon_sysfs.py for DAMOS action failed regions quota charge
ratio setup, so that we can add kselftest for the new feature.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 tools/testing/selftests/damon/_damon_sysfs.py | 21 +++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py
index 120b96ecbd741..8b12cc0484405 100644
--- a/tools/testing/selftests/damon/_damon_sysfs.py
+++ b/tools/testing/selftests/damon/_damon_sysfs.py
@@ -132,14 +132,17 @@ class DamosQuota:
     goals = None                # quota goals
     goal_tuner = None           # quota goal tuner
     reset_interval_ms = None    # quota reset interval
+    fail_charge_num = None
+    fail_charge_denom = None
     weight_sz_permil = None
     weight_nr_accesses_permil = None
     weight_age_permil = None
     scheme = None               # owner scheme
 
     def __init__(self, sz=0, ms=0, goals=None, goal_tuner='consist',
-                 reset_interval_ms=0, weight_sz_permil=0,
-                 weight_nr_accesses_permil=0, weight_age_permil=0):
+                 reset_interval_ms=0, fail_charge_num=0, fail_charge_denom=0,
+                 weight_sz_permil=0, weight_nr_accesses_permil=0,
+                 weight_age_permil=0):
         self.sz = sz
         self.ms = ms
         self.reset_interval_ms = reset_interval_ms
@@ -151,6 +154,8 @@ class DamosQuota:
         for idx, goal in enumerate(self.goals):
             goal.idx = idx
             goal.quota = self
+        self.fail_charge_num = fail_charge_num
+        self.fail_charge_denom = fail_charge_denom
 
     def sysfs_dir(self):
         return os.path.join(self.scheme.sysfs_dir(), 'quotas')
@@ -197,6 +202,18 @@ class DamosQuota:
                 os.path.join(self.sysfs_dir(), 'goal_tuner'), self.goal_tuner)
         if err is not None:
             return err
+
+        err = write_file(
+                os.path.join(self.sysfs_dir(), 'fail_charge_num'),
+                self.fail_charge_num)
+        if err is not None:
+            return err
+        err = write_file(
+                os.path.join(self.sysfs_dir(), 'fail_charge_denom'),
+                self.fail_charge_denom)
+        if err is not None:
+            return err
+
         return None
 
 class DamosWatermarks:
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH v3 09/10] selftests/damon/drgn_dump_damon_status: support failed region quota charge ratio
  2026-04-07  1:05 [RFC PATCH v3 00/10] mm/damon: introduce DAMOS failed region quota charge ratio SeongJae Park
                   ` (7 preceding siblings ...)
  2026-04-07  1:05 ` [RFC PATCH v3 08/10] selftests/damon/_damon_sysfs: support failed region quota charge ratio SeongJae Park
@ 2026-04-07  1:05 ` SeongJae Park
  2026-04-07  1:05 ` [RFC PATCH v3 10/10] selftests/damon/sysfs.py: test " SeongJae Park
  9 siblings, 0 replies; 14+ messages in thread
From: SeongJae Park @ 2026-04-07  1:05 UTC (permalink / raw)
  Cc: SeongJae Park, Shuah Khan, damon, linux-kernel, linux-kselftest,
	linux-mm

Extend drgn_dump_damon_status.py to dump DAMON internal state for DAMOS
action failed regions quota charge ratio, to be able to show if the
internal state for the feature is working, with future DAMON selftests.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 tools/testing/selftests/damon/drgn_dump_damon_status.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/damon/drgn_dump_damon_status.py b/tools/testing/selftests/damon/drgn_dump_damon_status.py
index 5b90eb8e7ef88..972948e6215f1 100755
--- a/tools/testing/selftests/damon/drgn_dump_damon_status.py
+++ b/tools/testing/selftests/damon/drgn_dump_damon_status.py
@@ -112,6 +112,8 @@ def damos_quota_to_dict(quota):
         ['goals', damos_quota_goals_to_list],
         ['goal_tuner', int],
         ['esz', int],
+        ['fail_charge_num', int],
+        ['fail_charge_denom', int],
         ['weight_sz', int],
         ['weight_nr_accesses', int],
         ['weight_age', int],
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH v3 10/10] selftests/damon/sysfs.py: test failed region quota charge ratio
  2026-04-07  1:05 [RFC PATCH v3 00/10] mm/damon: introduce DAMOS failed region quota charge ratio SeongJae Park
                   ` (8 preceding siblings ...)
  2026-04-07  1:05 ` [RFC PATCH v3 09/10] selftests/damon/drgn_dump_damon_status: " SeongJae Park
@ 2026-04-07  1:05 ` SeongJae Park
  9 siblings, 0 replies; 14+ messages in thread
From: SeongJae Park @ 2026-04-07  1:05 UTC (permalink / raw)
  Cc: SeongJae Park, Shuah Khan, damon, linux-kernel, linux-kselftest,
	linux-mm

Extend sysfs.py DAMON selftest to setup DAMOS action failed region quota
charge ratio and assert the setup is made into DAMON internal state.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 tools/testing/selftests/damon/sysfs.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
index b8d6e0f8fd906..d8d4717128898 100755
--- a/tools/testing/selftests/damon/sysfs.py
+++ b/tools/testing/selftests/damon/sysfs.py
@@ -73,6 +73,10 @@ def assert_quota_committed(quota, dump):
             }
     assert_true(dump['goal_tuner'] == tuner_val[quota.goal_tuner],
                 'goal_tuner', dump)
+    assert_true(dump['fail_charge_num'] == quota.fail_charge_num,
+                'fail_charge_num', dump)
+    assert_true(dump['fail_charge_denom'] == quota.fail_charge_denom,
+                'fail_charge_denom', dump)
     assert_true(dump['weight_sz'] == quota.weight_sz_permil, 'weight_sz', dump)
     assert_true(dump['weight_nr_accesses'] == quota.weight_nr_accesses_permil,
                 'weight_nr_accesses', dump)
@@ -277,6 +281,8 @@ def main():
                         nid=1)],
                     goal_tuner='temporal',
                     reset_interval_ms=1500,
+                    fail_charge_num=1,
+                    fail_charge_denom=4096,
                     weight_sz_permil=20,
                     weight_nr_accesses_permil=200,
                     weight_age_permil=1000),
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: (sashiko review) [RFC PATCH v3 01/10] mm/damon/core: handle <min_region_sz remaining quota as empty
  2026-04-07  1:05 ` [RFC PATCH v3 01/10] mm/damon/core: handle <min_region_sz remaining quota as empty SeongJae Park
@ 2026-04-07 14:12   ` SeongJae Park
  0 siblings, 0 replies; 14+ messages in thread
From: SeongJae Park @ 2026-04-07 14:12 UTC (permalink / raw)
  To: SeongJae Park; +Cc: damon, linux-kernel, linux-mm

TL; DR: sashiko made a good suggestion.  I will address that in the next
revision.

Forwarding full sashiko review in a reply format with my 
inline comments below, for sharing details of my view and
doing followup discussions via mails if needed.

> # review url: https://sashiko.dev/#/patchset/20260407010536.83603-2-sj@kernel.org
> # start of sashiko.dev inline review
> commit 3657261e42ea1ce93233b322fabf0d2c5c335a35
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: handle <min_region_sz remaining quota as empty
> 
> This commit updates the DAMON quota checks to treat a remaining quota of less
> than min_region_sz as fully charged. It introduces a new helper function,
> damos_quota_is_full(), and replaces the existing explicit quota checks with it.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 3bc7a2bbfe7de..c29f70bcce894 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -2041,6 +2041,14 @@ static void damos_walk_cancel(struct damon_ctx *ctx)
> >  	mutex_unlock(&ctx->walk_control_lock);
> >  }
> >  
> > +static bool damos_quota_is_full(struct damos_quota *quota,
> > +		unsigned long min_region_sz)
> > +{
> > +	if (!damos_quota_is_set(quota))
> > +		return false;
> > +	return quota->charged_sz + min_region_sz > quota->esz;
> 
> Can quota->charged_sz + min_region_sz overflow here?
> 
> Since both variables are unsigned long, if min_region_sz is configured to a
> very large value via sysfs, or on 32-bit systems where memory regions can
> naturally be a large fraction of ULONG_MAX, the sum could wrap around.
> 
> If it wraps around, it would evaluate to a value smaller than quota->esz.
> This would cause damos_quota_is_full() to incorrectly return false even when
> the quota is fully exhausted, bypassing the DAMON action quota limits.
> 
> Could this be written using subtraction to prevent overflow?
> 
> return quota->charged_sz >= quota->esz ||
>        quota->esz - quota->charged_sz < min_region_sz;

Good suggestion, I will do so in the next revision.

> 
> [ ... ]
> 
> 
> # end of sashiko.dev inline review
> # review url: https://sashiko.dev/#/patchset/20260407010536.83603-2-sj@kernel.org


Thanks,
SJ

# hkml [1] generated a draft of this mail.  You can regenerate
# this using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260407010536.83603-2-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: (sashiko review) [RFC PATCH v3 02/10] mm/damon/core: introduce failed region quota charge ratio
  2026-04-07  1:05 ` [RFC PATCH v3 02/10] mm/damon/core: introduce failed region quota charge ratio SeongJae Park
@ 2026-04-07 15:51   ` SeongJae Park
  0 siblings, 0 replies; 14+ messages in thread
From: SeongJae Park @ 2026-04-07 15:51 UTC (permalink / raw)
  To: SeongJae Park; +Cc: Andrew Morton, damon, linux-kernel, linux-mm

TL; DR: sashiko made a good finding that better to be addressed.  I will
address it in the next revision.  sashiko also made two findings but I will
ignore those.

Forwarding full sashiko review in a reply format with my inline comments below,
for sharing details of my view and doing followup discussions via mails if
needed.

> # review url: https://sashiko.dev/#/patchset/20260407010536.83603-3-sj@kernel.org
> # start of sashiko.dev inline review
> commit 09593130776383947d3d32b60b59b00c4bc2e692
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: introduce failed region quota charge ratio
> 
> This patch introduces a configuration to charge a different quota ratio
> for memory regions where DAMOS actions failed. It aims to improve fairness
> in quota accounting between successful and failed actions.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index c29f70bcce894..2a09d6bdf5f3e 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -2041,6 +2043,23 @@ static void damos_walk_cancel(struct damon_ctx *ctx)
> >  	mutex_unlock(&ctx->walk_control_lock);
> >  }
> >  
> > +static void damos_charge_quota(struct damos_quota *quota,
> > +		unsigned long sz_region, unsigned long sz_applied)
> > +{
> > +	/*
> > +	 * sz_applied could be bigger than sz_region, depending on ops
> > +	 * implementation of the action, e.g., damos_pa_pageout().  Charge only
> > +	 * the region size in the case.
> > +	 */
> > +	if (!quota->fail_charge_denom || sz_applied > sz_region)
> > +		quota->charged_sz += sz_region;
> > +	else
> > +		quota->charged_sz += sz_applied + mult_frac(
> > +				(sz_region - sz_applied),
> > +				quota->fail_charge_num,
> > +				quota->fail_charge_denom);
> > +}
> 
> Can this cause an integer overflow on 32-bit architectures?
> 
> The fail_charge_num and fail_charge_denom are unsigned int and can be set
> to arbitrary values by userspace.
> 
> If a user configures a large numerator (e.g., close to UINT_MAX), the
> multiplications inside mult_frac() could overflow the 32-bit unsigned long
> boundary on 32-bit architectures, leading to an incorrectly truncated value
> being added to quota->charged_sz.

sashiko is asking a same question again.  As I replied on the previous version
thread, I will keep this as is for simplicity of the code, unless a human
argues differently.

> 
> [ ... ]
> > @@ -2105,7 +2124,7 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
> >  		ktime_get_coarse_ts64(&end);
> >  		quota->total_charged_ns += timespec64_to_ns(&end) -
> >  			timespec64_to_ns(&begin);
> > -		quota->charged_sz += sz;
> > +		damos_charge_quota(quota, sz, sz_applied);
> 
> Could this permanently disable a scheme if actions completely fail during the
> first time window?
> 
> If fail_charge_num is 0 and all actions fail across all regions in the first
> charge window, quota->charged_sz evaluates to 0.
> 
> At the end of the window, total_charged_sz becomes 0, while total_charged_ns
> records the actual time spent attempting. The calculated throughput
> becomes 0, which sets the effective size quota (esz) to 0 for the
> next window.
> 
> With esz = 0, damos_quota_is_full() always evaluates to true, causing DAMON
> to skip applying the scheme to any regions.  Since no regions are processed,
> total_charged_ns and total_charged_sz never increase again, permanently
> bricking the scheme.

sashiko is asking a same question again.  As I replied to the previous
question, this is not cuased by this patch.  So I'm working separately for the
issue.  I recently posted the fix:
https://lore.kernel.org/20260407003153.79589-1-sj@kernel.org

> 
> Does this code create an infinite region splitting loop if an action fails
> near the quota limit?
> 
> In damos_apply_scheme(), if the current region size exceeds the remaining
> quota, it splits the region into sz and the remainder:
> 
>     sz = ALIGN_DOWN(quota->esz - quota->charged_sz, c->min_region_sz);
>     damon_split_region_at(t, r, sz);
> 
> If the action on the split region fails, and fail_charge_num is less than
> fail_charge_denom, charged_sz increases by less than sz (or by 0).
> Because charged_sz did not increase fully, the remaining quota remains at
> least min_region_sz, so damos_quota_is_full() evaluates to false.
> 
> When the scheme loop proceeds to the remainder of the region, it will split
> it again by the same sz.  Could this repeat until the entire target memory
> is shattered into tiny regions, causing a massive increase in allocations
> and a kernel lockup?

Good finding.  Yes, this can increase number of region splits on the code path.
I think it should be safe from lockup, since each DAMOS action implementation
uses cond_resched().  But the increased number of regions makes me worried.  I
think we should merge the splitted region back, after DAMOS action is applied.
I will add the change in the next revision.

> 
> >  		if (damos_quota_is_full(quota, c->min_region_sz)) {
> >  			quota->charge_target_from = t;
> >  			quota->charge_addr_from = r->ar.end + 1;
> >  		}
> 
> 
> # end of sashiko.dev inline review
> # review url: https://sashiko.dev/#/patchset/20260407010536.83603-3-sj@kernel.org


Thanks,
SJ

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: (sashiko review) [RFC PATCH v3 03/10] mm/damon/sysfs-schemes: implement fail_charge_{num,denom} files
  2026-04-07  1:05 ` [RFC PATCH v3 03/10] mm/damon/sysfs-schemes: implement fail_charge_{num,denom} files SeongJae Park
@ 2026-04-07 15:55   ` SeongJae Park
  0 siblings, 0 replies; 14+ messages in thread
From: SeongJae Park @ 2026-04-07 15:55 UTC (permalink / raw)
  To: SeongJae Park; +Cc: damon, linux-kernel, linux-mm

TL; DR: sashiko argues two issues that I think ok to ignore, so I will do so.

Forwarding full sashiko review in a reply format with my 
inline comments below, for sharing details of my view and
doing followup discussions via mails if needed.

> # review url: https://sashiko.dev/#/patchset/20260407010536.83603-4-sj@kernel.org
> # start of sashiko.dev inline review
> commit 233e1cffd501d91418c4d1f0badd7f5b509664c0
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/sysfs-schemes: implement fail_charge_{num,denom} files
> 
> Implement the user-space ABI for the DAMOS action failed region
> quota-charge ratio setup.  For this, add two new sysfs files under the
> DAMON sysfs interface for DAMOS quotas.
> 
> > diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> > index 5186966dafb35..d5bfba79f105f 100644
> > --- a/mm/damon/sysfs-schemes.c
> > +++ b/mm/damon/sysfs-schemes.c
> [ ... ]
> > @@ -1663,6 +1665,48 @@ static ssize_t goal_tuner_store(struct kobject *kobj,
> >  	return -EINVAL;
> >  }
> >  
> > +static ssize_t fail_charge_num_show(struct kobject *kobj,
> > +		struct kobj_attribute *attr, char *buf)
> > +{
> > +	struct damon_sysfs_quotas *quotas = container_of(kobj,
> > +			struct damon_sysfs_quotas, kobj);
> > +
> > +	return sysfs_emit(buf, "%u\n", quotas->fail_charge_num);
> > +}
> > +
> > +static ssize_t fail_charge_num_store(struct kobject *kobj,
> > +		struct kobj_attribute *attr, const char *buf, size_t count)
> > +{
> > +	struct damon_sysfs_quotas *quotas = container_of(kobj,
> > +			struct damon_sysfs_quotas, kobj);
> > +	int err = kstrtouint(buf, 0, &quotas->fail_charge_num);
> 
> Is it possible for large values of fail_charge_num to cause integer
> overflows in the quota calculation?
> 
> Because kstrtouint allows values up to UINT_MAX without bounds checking,
> the multiplication in mult_frac(sz_region - sz_applied,
> quota->fail_charge_num, quota->fail_charge_denom) during
> damos_charge_quota() might silently overflow the unsigned long capacity.
> 
> Upon overflow, the calculated charge wraps around to a significantly
> smaller value.
> 
> Could this artificially suppress the accumulated quota->charged_sz,
> causing damos_quota_is_full() to incorrectly return false and completely
> bypass the user-configured quota limits?

As I mentioned on the second patch thread, I will just let this overflows if
the user really wants to set such large number, since it will make DAMON
behavior bit weird but not breaking something serious, unless other humans have
different opinions.

> 
> > +
> > +	if (err)
> > +		return -EINVAL;
> > +	return count;
> > +}
> > +
> > +static ssize_t fail_charge_denom_show(struct kobject *kobj,
> > +		struct kobj_attribute *attr, char *buf)
> > +{
> > +	struct damon_sysfs_quotas *quotas = container_of(kobj,
> > +			struct damon_sysfs_quotas, kobj);
> > +
> > +	return sysfs_emit(buf, "%u\n", quotas->fail_charge_denom);
> > +}
> > +
> > +static ssize_t fail_charge_denom_store(struct kobject *kobj,
> > +		struct kobj_attribute *attr, const char *buf, size_t count)
> > +{
> > +	struct damon_sysfs_quotas *quotas = container_of(kobj,
> > +			struct damon_sysfs_quotas, kobj);
> > +	int err = kstrtouint(buf, 0, &quotas->fail_charge_denom);
> 
> Can the lack of bounds checking on fail_charge_denom also contribute to
> unexpected behaviors or overflows in the same mult_frac() calculation?

Ditto.

> 
> > +
> > +	if (err)
> > +		return -EINVAL;
> > +	return count;
> > +}
> 
> 
> # end of sashiko.dev inline review
> # review url: https://sashiko.dev/#/patchset/20260407010536.83603-4-sj@kernel.org


Thanks,
SJ

# hkml [1] generated a draft of this mail.  You can regenerate
# this using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260407010536.83603-4-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-04-07 15:55 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-07  1:05 [RFC PATCH v3 00/10] mm/damon: introduce DAMOS failed region quota charge ratio SeongJae Park
2026-04-07  1:05 ` [RFC PATCH v3 01/10] mm/damon/core: handle <min_region_sz remaining quota as empty SeongJae Park
2026-04-07 14:12   ` (sashiko review) " SeongJae Park
2026-04-07  1:05 ` [RFC PATCH v3 02/10] mm/damon/core: introduce failed region quota charge ratio SeongJae Park
2026-04-07 15:51   ` (sashiko review) " SeongJae Park
2026-04-07  1:05 ` [RFC PATCH v3 03/10] mm/damon/sysfs-schemes: implement fail_charge_{num,denom} files SeongJae Park
2026-04-07 15:55   ` (sashiko review) " SeongJae Park
2026-04-07  1:05 ` [RFC PATCH v3 04/10] Docs/mm/damon/design: document fail_charge_{num,denom} SeongJae Park
2026-04-07  1:05 ` [RFC PATCH v3 05/10] Docs/admin-guide/mm/damon/usage: document fail_charge_{num,denom} files SeongJae Park
2026-04-07  1:05 ` [RFC PATCH v3 06/10] Docs/ABI/damon: document fail_charge_{num,denom} SeongJae Park
2026-04-07  1:05 ` [RFC PATCH v3 07/10] mm/damon/tests/core-kunit: test fail_charge_{num,denom} committing SeongJae Park
2026-04-07  1:05 ` [RFC PATCH v3 08/10] selftests/damon/_damon_sysfs: support failed region quota charge ratio SeongJae Park
2026-04-07  1:05 ` [RFC PATCH v3 09/10] selftests/damon/drgn_dump_damon_status: " SeongJae Park
2026-04-07  1:05 ` [RFC PATCH v3 10/10] selftests/damon/sysfs.py: test " SeongJae Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox