damon.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr
@ 2026-08-01 17:35 SJ Park
  2026-08-01 17:35 ` [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails SJ Park
                   ` (8 more replies)
  0 siblings, 9 replies; 23+ messages in thread
From: SJ Park @ 2026-08-01 17:35 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, Usama Arif, Yueyang Pan, damon,
	linux-kernel, linux-mm

Fix misc bugs of DAMOS.  Patch 1 make DAMOS less stress memory allocator
under extreme situation.  Patch 2 fixes wrong DAMOS quota auto-tuning
feedback loop start point for PSI goal.  Patches 3 and 4 fix wrong
folios walking in DAMON_PADDR.  Patches 5 and 6 fix wrong folios walking
in DAMON_VADDR.  Patches 7-9 handle extreme and unlikely memory
situations that can cause divide by zero and underflow.

All bugs are discovered by Sashiko.

The bugs are not very critical, but better to be merged sooner than
later.  Since mm.git is closed for urgent changes, I aim for 7.3-rcX.  I
will drop the RFC tag after 7.3-rc1.

SJ Park (9):
  mm/damon/core: skip applying scheme if region split for quota fails
  mm/damon/core: initialize damos_quota_goal->last_psi_total
  mm/damon/paddr: respect folio end for DAMOS_STAT
  mm/damon/paddr: respect folio end for DAMOS actions except STAT
  mm/damon/vaddr: respect folio end for DAMOS_STAT
  mm/damon/vaddr: respect folio end for DAMOS_MIGRATE_{HOT,COLD}
  mm/damon/core: handle extreme memory state in damon_get_node_mem_bp()
  mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
  mm/damon/core: handle extreme memory state in get_in_active_mem_bp()

 mm/damon/core.c  | 29 +++++++++++++++++++++++++----
 mm/damon/paddr.c |  8 ++++----
 mm/damon/vaddr.c | 10 ++++++++--
 3 files changed, 37 insertions(+), 10 deletions(-)


base-commit: 70802c13e2c9a0ab52605035e2d63b964afe6add
-- 
2.47.3

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

* [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails
  2026-08-01 17:35 [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
@ 2026-08-01 17:35 ` SJ Park
  2026-08-01 17:53   ` sashiko-bot
  2026-08-01 17:35 ` [RFC PATCH 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total SJ Park
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: SJ Park @ 2026-08-01 17:35 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, damon, linux-kernel, linux-mm

damos_apply_scheme() splits a region and apply the action to the
subregion if it is needed for not violating the quota.  The split
operation (damon_split_region_at()) could fail for allocation failure.
In the case, the quota could be violated.  From the user's perspective,
DAMOS becomes more aggressive than expected under the extreme situation.
Handle the failure.

The user impact is not critical.  The failure of damon_split_region_at()
is unlikely since it is arguably too small to fail.  Also DAMOS being
aggressive is limited to the single region.  Users can set
min_nr_regions to set the maximum size of each region.  If it is
reasonably set, the transient overhead shouldn't be critical.

The issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260718171523.87547-1-sj@kernel.org

Fixes: 2b8a248d5873 ("mm/damon/schemes: implement size quota for schemes application speed control")
Cc: <stable@vger.kernel.org> # 5.16.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 644daf5a16560..e2900d0c984c9 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2613,7 +2613,8 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
 					c->min_region_sz);
 			if (!sz)
 				goto update_stat;
-			damon_split_region_at(t, r, sz);
+			if (damon_split_region_at(t, r, sz))
+				goto update_stat;
 		}
 		if (damos_core_filter_out(c, t, r, s))
 			return;
-- 
2.47.3

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

* [RFC PATCH 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total
  2026-08-01 17:35 [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
  2026-08-01 17:35 ` [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails SJ Park
@ 2026-08-01 17:35 ` SJ Park
  2026-08-01 17:35 ` [RFC PATCH 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT SJ Park
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-01 17:35 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, damon, linux-kernel, linux-mm

When DAMOS_QUOTA_SOME_MEM_PSI_US metric damos quota goal is set, the PSI
delta for the feedback loop is calculated using
damos_quota_goal->last_psi_total.  It is not initialized at the
beginning.  So the first iteration of the feedback loop uses the
uninitialized value and makes an unexpected starting point quota.
Initialize the value at the beginning of kdamond.

The user impact would be trivial because the issue impacts only the
initial iteration of the feedback loop.  The feedback loop also has an
internal cap of the quota adjustment.  The wrong adjustment will soon be
corrected over a few iterations.

The issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260718005316.89585-1-sj@kernel.org

Fixes: 2dbb60f789cb ("mm/damon/core: implement PSI metric DAMOS quota goal")
Cc: <stable@vger.kernel.org> # 6.9.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index e2900d0c984c9..3bdbf4fbf7147 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3728,6 +3728,17 @@ static int kdamond_wait_activation(struct damon_ctx *ctx)
 	return -EBUSY;
 }
 
+static void damos_init_quota_goal_last_psi(struct damos *s)
+{
+	struct damos_quota_goal *goal;
+
+	damos_for_each_quota_goal(goal, &s->quota) {
+		if (goal->metric != DAMOS_QUOTA_SOME_MEM_PSI_US)
+			continue;
+		goal->last_psi_total = damos_get_some_mem_psi_total();
+	}
+}
+
 static void kdamond_init_ctx(struct damon_ctx *ctx)
 {
 	unsigned long sample_interval = ctx->attrs.sample_interval ?
@@ -3744,6 +3755,7 @@ static void kdamond_init_ctx(struct damon_ctx *ctx)
 	damon_for_each_scheme(scheme, ctx) {
 		damos_set_next_apply_sis(scheme, ctx);
 		damos_set_filters_default_reject(scheme);
+		damos_init_quota_goal_last_psi(scheme);
 	}
 }
 
-- 
2.47.3

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

* [RFC PATCH 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT
  2026-08-01 17:35 [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
  2026-08-01 17:35 ` [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails SJ Park
  2026-08-01 17:35 ` [RFC PATCH 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total SJ Park
@ 2026-08-01 17:35 ` SJ Park
  2026-08-01 18:21   ` sashiko-bot
  2026-08-01 17:35 ` [RFC PATCH 4/9] mm/damon/paddr: respect folio end for DAMOS actions except STAT SJ Park
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: SJ Park @ 2026-08-01 17:35 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, Usama Arif, damon, linux-kernel,
	linux-mm

The function for applying DAMOS_STAT in DAMON physical address space
operation set (paddr), namely damon_pa_stat(), applies DAMOS filters to
folios of the given region.  For that, it gets folios of addresses in
the region.  It starts from the region start address and advances the
address by the size of the folio of the address until it goes out of the
region.  If the start address is in the middle of a large folio, and if
the next folios are small, some of the next folios could be skipped. Fix
the issue by advancing the address to exactly the start address of the
next folio.

The user impact is that the DAMOS_STAT-based page level monitoring
results become inaccurate.  Since the page level monitoring is supposed
to provide relatively high precision, this is definitely a problem.  It
is arguably not critical since it is only monitoring quality
degradation.

Fixes: bdbe1d7bc325 ("mm/damon/paddr: increment pa_stat damon address range by folio size")
Cc: <stable@vger.kernel.org> # 6.14.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/paddr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 5c6c3a597fd0b..2ab7b3842701e 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -379,7 +379,7 @@ static unsigned long damon_pa_stat(struct damon_region *r,
 
 		if (!damos_pa_filter_out(s, folio))
 			*sz_filter_passed += folio_size(folio) / addr_unit;
-		addr += folio_size(folio);
+		addr = PFN_PHYS(folio_pfn(folio)) + folio_size(folio);
 		folio_put(folio);
 	}
 	s->last_applied = folio;
-- 
2.47.3

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

* [RFC PATCH 4/9] mm/damon/paddr: respect folio end for DAMOS actions except STAT
  2026-08-01 17:35 [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
                   ` (2 preceding siblings ...)
  2026-08-01 17:35 ` [RFC PATCH 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT SJ Park
@ 2026-08-01 17:35 ` SJ Park
  2026-08-01 17:35 ` [RFC PATCH 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT SJ Park
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-01 17:35 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, Usama Arif, damon, linux-kernel,
	linux-mm

A few functions for applying DAMOS actions including pageout,
lru_[de]prio and migrate_{hot,cold} in DAMON physical address space
operation set (paddr) collect folios of the given region by getting the
folios of region-internal addresses.  Then, those functions apply the
action to the collected folios at once.  The collection starts from the
region start address and advances the address by the size of the folio
of the address until it goes out of the region.  If the start address is
in the middle of a large folio, and if the next folios are small, some
of the next folios could be skipped.  Fix the issue by advancing the
address to exactly the start address of the next folio.

The user impact is that DAMOS action is applied to less than expected
amount of memory.  Given the best effort nature of DAMON, it is no big
problem, but it is clearly a bug that is better to be fixed.

The issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260517234112.89245-1-sj@kernel.org

Fixes: 3a06696305e7 ("mm/damon/ops: have damon_get_folio return folio even for tail pages")
Cc: <stable@vger.kernel.org> # 6.15.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/paddr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 2ab7b3842701e..9ddd1ec8202b7 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -264,7 +264,7 @@ static unsigned long damon_pa_pageout(struct damon_region *r,
 		else
 			list_add(&folio->lru, &folio_list);
 put_folio:
-		addr += folio_size(folio);
+		addr = PFN_PHYS(folio_pfn(folio)) + folio_size(folio);
 		folio_put(folio);
 	}
 	if (install_young_filter)
@@ -302,7 +302,7 @@ static inline unsigned long damon_pa_de_activate(
 			folio_deactivate(folio);
 		applied += folio_nr_pages(folio);
 put_folio:
-		addr += folio_size(folio);
+		addr = PFN_PHYS(folio_pfn(folio)) + folio_size(folio);
 		folio_put(folio);
 	}
 	s->last_applied = folio;
@@ -350,7 +350,7 @@ static unsigned long damon_pa_migrate(struct damon_region *r,
 				folio_is_file_lru(folio));
 		list_add(&folio->lru, &folio_list);
 put_folio:
-		addr += folio_size(folio);
+		addr = PFN_PHYS(folio_pfn(folio)) + folio_size(folio);
 		folio_put(folio);
 	}
 	applied = damon_migrate_pages(&folio_list, s->target_nid);
-- 
2.47.3

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

* [RFC PATCH 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT
  2026-08-01 17:35 [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
                   ` (3 preceding siblings ...)
  2026-08-01 17:35 ` [RFC PATCH 4/9] mm/damon/paddr: respect folio end for DAMOS actions except STAT SJ Park
@ 2026-08-01 17:35 ` SJ Park
  2026-08-01 18:50   ` sashiko-bot
  2026-08-01 17:35 ` [RFC PATCH 6/9] mm/damon/vaddr: respect folio end for DAMOS_MIGRATE_{HOT,COLD} SJ Park
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: SJ Park @ 2026-08-01 17:35 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, Yueyang Pan, damon, linux-kernel,
	linux-mm

For applying DAMOS_STAT action to a region, DAMON virtual address space
operation set (vaddr) calls walk_page_range[_vma]() for the region.  The
pmd walk entry function, namely damon_va_stat_pmd_entry(), applies DAMOS
filters to folios of addresses of the region in the pmd.  It starts from
the walking address and advances the address by the size of the folio of
the address until it goes out of the pmd or the region.

Let's suppose it is for the first pmd of the region, and the region
start address is in the middle of a large folio.  Also, the next folios
are small.  Then, some of the next folios could be skipped.  Fix the
issue by advancing the address to exactly the start address of the next
folio.

The user impact is that the DAMOS_STAT-based page level monitoring
results become inaccurate.  Since the page level monitoring is supposed
to provide relatively high precision, this is definitely a problem.  It
is arguably not critical since it is only monitoring quality
degradation.

The issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260514015053.149396-1-sj@kernel.org

Fixes: 63f39737d1e3 ("mm/damon/vaddr: support stat-purpose DAMOS filters")
Cc: <stable@vger.kernel.org> # 6.18.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/vaddr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index 0648400b2d65b..035cd509f6b58 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -831,6 +831,8 @@ static int damos_va_stat_pmd_entry(pmd_t *pmd, unsigned long addr,
 		return 0;
 
 	for (; addr < next; pte += nr, addr += nr * PAGE_SIZE) {
+		unsigned long page_idx;
+
 		nr = 1;
 		ptent = ptep_get(pte);
 
@@ -844,7 +846,8 @@ static int damos_va_stat_pmd_entry(pmd_t *pmd, unsigned long addr,
 
 		if (!damos_va_filter_out(s, folio, vma, addr, pte, NULL))
 			*sz_filter_passed += folio_size(folio);
-		nr = folio_nr_pages(folio);
+		page_idx = folio_page_idx(folio, pte_page(*pte));
+		nr = folio_nr_pages(folio) - page_idx;
 		s->last_applied = folio;
 	}
 	pte_unmap_unlock(start_pte, ptl);
-- 
2.47.3

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

* [RFC PATCH 6/9] mm/damon/vaddr: respect folio end for DAMOS_MIGRATE_{HOT,COLD}
  2026-08-01 17:35 [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
                   ` (4 preceding siblings ...)
  2026-08-01 17:35 ` [RFC PATCH 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT SJ Park
@ 2026-08-01 17:35 ` SJ Park
  2026-08-01 17:35 ` [RFC PATCH 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() SJ Park
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-01 17:35 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, damon, linux-kernel, linux-mm

For applying DAMOS_MIGRATE_{HOT,COLD} actions to a region, DAMON virtual
address space operation set (vaddr) calls walk_page_range[_vma]() for
the region.  The pmd walk entry function, namely
damon_va_migrate_pmd_entry(), collects folios of addresses of the region
in the pmd.  It starts from the walking address and advances the address
by the size of the folio of the address until it goes out of the pmd or
the region.

Let's suppose it is for the first pmd of the region, and the region
start address is in the middle of a large folio.  Also, the next folios
are small.  Then, some of the next folios could be skipped.  Fix the
issue by advancing the address to exactly the start address of the next
folio.

The user impact is that DAMOS action is applied to less than expected
amount of memory.  Given the best effort nature of DAMON, it is no big
problem, but it is clearly a bug that is better to be fixed.

The issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260514015053.149396-1-sj@kernel.org

Fixes: 09efc56a3b1c ("mm/damon/vaddr: consistently use only pmd_entry for damos_migrate")
Cc: <stable@vger.kernel.org> # 6.19.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/vaddr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index 035cd509f6b58..aa833c2c4fa35 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -669,6 +669,8 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
 		return 0;
 
 	for (; addr < next; pte += nr, addr += nr * PAGE_SIZE) {
+		unsigned long page_idx;
+
 		nr = 1;
 		ptent = ptep_get(pte);
 
@@ -681,7 +683,8 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
 			continue;
 		damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
 				migration_lists);
-		nr = folio_nr_pages(folio);
+		page_idx = folio_page_idx(folio, pte_page(*pte));
+		nr = folio_nr_pages(folio) - page_idx;
 	}
 	pte_unmap_unlock(start_pte, ptl);
 	return 0;
-- 
2.47.3

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

* [RFC PATCH 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp()
  2026-08-01 17:35 [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
                   ` (5 preceding siblings ...)
  2026-08-01 17:35 ` [RFC PATCH 6/9] mm/damon/vaddr: respect folio end for DAMOS_MIGRATE_{HOT,COLD} SJ Park
@ 2026-08-01 17:35 ` SJ Park
  2026-08-01 19:11   ` sashiko-bot
  2026-08-01 17:35 ` [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() SJ Park
  2026-08-01 17:35 ` [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() SJ Park
  8 siblings, 1 reply; 23+ messages in thread
From: SJ Park @ 2026-08-01 17:35 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, damon, linux-kernel, linux-mm

In an extreme and unlikely situation, si_meminfo_node() might let the
caller show zero total ram.  That could cause a divide by zero in
damon_get_node_mem_bp().  Fix it by setting the totalram one byte in the
case.

The issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260328133216.9697-1-sj@kernel.org

Fixes: 0e1c773b501f ("mm/damon/core: introduce damos quota goal metrics for memory node utilization")
Cc: <stable@vger.kernel.org> # 6.16.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 3bdbf4fbf7147..269865eb8fbfa 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2816,6 +2816,8 @@ static __kernel_ulong_t damos_get_node_mem_bp(
 	}
 
 	si_meminfo_node(&i, goal->nid);
+	if (!i.totalram)
+		i.totalram = 1;
 	if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP)
 		numerator = i.totalram - i.freeram;
 	else	/* DAMOS_QUOTA_NODE_MEM_FREE_BP */
-- 
2.47.3

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

* [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
  2026-08-01 17:35 [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
                   ` (6 preceding siblings ...)
  2026-08-01 17:35 ` [RFC PATCH 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() SJ Park
@ 2026-08-01 17:35 ` SJ Park
  2026-08-01 19:31   ` sashiko-bot
  2026-08-01 17:35 ` [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() SJ Park
  8 siblings, 1 reply; 23+ messages in thread
From: SJ Park @ 2026-08-01 17:35 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, damon, linux-kernel, linux-mm

In extreme unlikely situations, total memory might be zero.  In less
extreme but still very unlikely situations, lruvec_page_state() calls
might let the caller show used memory larger than total memory.  In the
two cases, damos_get_node_memcg_used_bp() could cause division by zero,
or return underflowed value, respectively.  Handle the cases by
returning 100% and 0% for the two cases, respectively.

This issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260329154813.47382-1-sj@kernel.org

Fixes: b74a120bcf50 ("mm/damon/core: implement DAMOS_QUOTA_NODE_MEMCG_USED_BP")
Cc: <stable@vger.kernel.org> # 6.19.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 269865eb8fbfa..76764a2056f7c 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2858,10 +2858,16 @@ static unsigned long damos_get_node_memcg_used_bp(
 	mem_cgroup_put(memcg);
 
 	si_meminfo_node(&i, goal->nid);
-	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
+	if (!i.totalram)
+		return 10000;
+	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP) {
 		numerator = used_pages;
-	else	/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
+	} else {
+		/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
+		if (i.totalram < used_pages)
+			return 0;
 		numerator = i.totalram - used_pages;
+	}
 	return mult_frac(numerator, 10000, i.totalram);
 }
 
-- 
2.47.3

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

* [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp()
  2026-08-01 17:35 [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
                   ` (7 preceding siblings ...)
  2026-08-01 17:35 ` [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() SJ Park
@ 2026-08-01 17:35 ` SJ Park
  2026-08-01 19:39   ` sashiko-bot
  8 siblings, 1 reply; 23+ messages in thread
From: SJ Park @ 2026-08-01 17:35 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, damon, linux-kernel, linux-mm

damon_get_intervals_adaptation_bp() uses the sum of the active and
inactive memory amount as a denominator.  In an extreme and unlikely
environment, active and inactive memory might be zero.  In this case,
hence, it results in a divide by zero problem.  Avoid it by changing the
denominator to one if it is zero, before it is being used.

The issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260721034756.147011-1-sj@kernel.org

Fixes: 4835e2871321 ("mm/damon/core: introduce [in]active memory ratio damos quota goal metric")
Cc: <stable@vger.kernel.org> # 7.0.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 76764a2056f7c..4944cf2c5afae 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3010,7 +3010,7 @@ static unsigned int damos_get_in_active_mem_bp(bool active_ratio)
 		global_node_page_state(NR_LRU_BASE + LRU_ACTIVE_FILE);
 	inactive = global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_ANON) +
 		global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_FILE);
-	total = active + inactive;
+	total = max(active + inactive, 1);
 	if (active_ratio)
 		return mult_frac(active, 10000, total);
 	return mult_frac(inactive, 10000, total);
-- 
2.47.3

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

* Re: [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails
  2026-08-01 17:35 ` [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails SJ Park
@ 2026-08-01 17:53   ` sashiko-bot
  2026-08-01 20:01     ` SJ Park
  0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-08-01 17:53 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unhandled failure of damon_split_region_at() in damos_filter_match() leads to bypassing or incorrectly applying address filters.
--

--- Patch [1]: [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails ---
commit 812934b542c69c596afadb66403f8c9c11c5bf91
Author: SJ Park <sj@kernel.org>

mm/damon/core: skip applying scheme if region split for quota fails

damos_apply_scheme() splits a region and apply the action to the
subregion if it is needed for not violating the quota.  The split
operation (damon_split_region_at()) could fail for allocation failure.
In the case, the quota could be violated.  From the user's perspective,
DAMOS becomes more aggressive than expected under the extreme situation.
Handle the failure.

Link: https://lore.kernel.org/20260718171523.87547-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 644daf5a16560..e2900d0c984c9 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2613,7 +2613,8 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
>  					c->min_region_sz);
>  			if (!sz)
>  				goto update_stat;
> -			damon_split_region_at(t, r, sz);
> +			if (damon_split_region_at(t, r, sz))
> +				goto update_stat;

[Severity: High]
This is a pre-existing issue, but since we are handling the failure of
damon_split_region_at() here, should we also handle it in
damos_filter_match()?

If a memory region partially overlaps a filter range and the split
allocation fails in damos_filter_match(), the return value is ignored:

mm/damon/core.c:damos_filter_match() {
    ...
		/* start before the range and overlap */
		if (r->ar.start < start) {
			damon_split_region_at(t, r, start - r->ar.start);
			matched = false;
			break;
		}
		/* start inside the range */
		damon_split_region_at(t, r, end - r->ar.start);
		matched = true;
		break;
    ...
}

If the split fails, the intact region spanning both outside and inside the
filter range is unconditionally evaluated as 'matched = false' or
'matched = true', incorrectly bypassing the filter or applying it to unrelated
memory outside the intended range.

Could this lead to destructive actions (e.g., PAGEOUT) being applied to
explicitly protected memory?

>  		}
>  		if (damos_core_filter_out(c, t, r, s))
>  			return;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=1

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

* Re: [RFC PATCH 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT
  2026-08-01 17:35 ` [RFC PATCH 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT SJ Park
@ 2026-08-01 18:21   ` sashiko-bot
  2026-08-01 20:07     ` SJ Park
  0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-08-01 18:21 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix for large folio address iteration boundary bug.
--

--- Patch [3]: [RFC PATCH 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT ---
commit b98d6f7836f4a90b6724f683127a934a339f77d5
Author: SJ Park <sj@kernel.org>

mm/damon/paddr: respect folio end for DAMOS_STAT

This commit fixes an issue in damon_pa_stat() where advancing the address
by folio_size(folio) could skip subsequent folios if the start address
is unaligned within a large folio. It corrects this by advancing the
address to exactly the start address of the next folio.

> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index 5c6c3a597fd0b..2ab7b3842701e 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -379,7 +379,7 @@ static unsigned long damon_pa_stat(struct damon_region *r,
>  
>  		if (!damos_pa_filter_out(s, folio))
>  			*sz_filter_passed += folio_size(folio) / addr_unit;
> -		addr += folio_size(folio);
> +		addr = PFN_PHYS(folio_pfn(folio)) + folio_size(folio);

[Severity: High]
This is a pre-existing issue, but does this same bug exist in other DAMON
operations that iterate over folios?

For example, looking at damon_pa_pageout():

mm/damon/paddr.c:damon_pa_pageout() {
    ...
		else
			list_add(&folio->lru, &folio_list);
put_folio:
		addr += folio_size(folio);
		folio_put(folio);
	}
    ...
}

It appears damon_pa_pageout(), damon_pa_de_activate(), and damon_pa_migrate()
all still advance the address by folio_size(folio) instead of calculating the
exact folio end.

Could these operations also overshoot subsequent memory and skip valid memory
folios if the region starts unaligned inside a large folio?

>  		folio_put(folio);
>  	}
>  	s->last_applied = folio;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=3

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

* Re: [RFC PATCH 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT
  2026-08-01 17:35 ` [RFC PATCH 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT SJ Park
@ 2026-08-01 18:50   ` sashiko-bot
  2026-08-01 20:12     ` SJ Park
  0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-08-01 18:50 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Direct dereference of PTE pointer `*pte` instead of using the local safely fetched `ptent`.

Pre-existing issues:
- [High] Batched PTE advancement blindly skips interleaved folios and PTE-level Accessed/Young bits within a large folio's virtual footprint.
- [High] The exact page iteration bug fixed in `damos_va_stat_pmd_entry()` is also present in `damos_va_migrate_pmd_entry()` but was missed by this patch.
--

--- Patch [5]: [RFC PATCH 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT ---
commit 20f53909d4e28dc2c1b2dd4ce52890349fc6c85d
Author: SJ Park <sj@kernel.org>

mm/damon/vaddr: respect folio end for DAMOS_STAT

For applying DAMOS_STAT action to a region, DAMON virtual address space
operation set (vaddr) calls walk_page_range[_vma]() for the region.  The
pmd walk entry function, namely damon_va_stat_pmd_entry(), applies DAMOS
filters to folios of addresses of the region in the pmd.  It starts from
the walking address and advances the address by the size of the folio of
the address until it goes out of the pmd or the region.

Link: https://lore.kernel.org/20260514015053.149396-1-sj@kernel.org

> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index 0648400b2d65b..035cd509f6b58 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -831,6 +831,8 @@ static int damos_va_stat_pmd_entry(pmd_t *pmd, unsigned long addr,
>  		return 0;
>  
>  	for (; addr < next; pte += nr, addr += nr * PAGE_SIZE) {
> +		unsigned long page_idx;
> +
>  		nr = 1;
>  		ptent = ptep_get(pte);
>  
> @@ -844,7 +846,8 @@ static int damos_va_stat_pmd_entry(pmd_t *pmd, unsigned long addr,
>  
>  		if (!damos_va_filter_out(s, folio, vma, addr, pte, NULL))
>  			*sz_filter_passed += folio_size(folio);
> -		nr = folio_nr_pages(folio);
> +		page_idx = folio_page_idx(folio, pte_page(*pte));

[Severity: Medium]
Is it safe to directly dereference *pte here instead of using the local
safely fetched ptent?

Reading a PTE directly on 32-bit architectures (like x86 PAE) can result in
a torn read if the hardware concurrently updates the Accessed/Dirty bits,
potentially leading to an incorrect PFN and subsequent wild pointer
arithmetic.

The local snapshot ptent was already obtained via ptep_get(pte) and should
likely be used here.

> +		nr = folio_nr_pages(folio) - page_idx;

[Severity: High]
This is a pre-existing issue, but does this batched PTE advancement blindly
skip interleaved folios and PTE-level Accessed/Young bits within a large
folio's virtual footprint?

For PTE-mapped large folios, hardware tracks the Accessed bit independently
per 4K PTE. By checking only the first PTE and blindly skipping the rest,
DAMON ignores accesses to subsequent pages of the folio, potentially
classifying hot memory as cold.

Furthermore, if userspace partially unmaps or interleaves different mappings,
the blind pte += nr skips those unrelated folios completely without
verifying they map the same folio.

>  		s->last_applied = folio;
>  	}
>  	pte_unmap_unlock(start_pte, ptl);

While reviewing this patch, I noticed the exact page iteration bug fixed in
damos_va_stat_pmd_entry() is also present in damos_va_migrate_pmd_entry()
but was missed by this patch.

[Severity: High]
This is a pre-existing issue, but does the DAMOS migrate action also advance
the PTE pointer unconditionally by the full size of the large folio?

mm/damon/vaddr.c:damos_va_migrate_pmd_entry() {
	...
	for (; addr < next; pte += nr, addr += nr * PAGE_SIZE) {
		...
		damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
				migration_lists);
		nr = folio_nr_pages(folio);
	}
	...
}

If the scanning starts in the middle of a large folio, it will advance past
the end of the large folio and skip unrelated folios that follow it, failing
to migrate them.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=5

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

* Re: [RFC PATCH 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp()
  2026-08-01 17:35 ` [RFC PATCH 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() SJ Park
@ 2026-08-01 19:11   ` sashiko-bot
  2026-08-01 20:18     ` SJ Park
  0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-08-01 19:11 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Medium] Falsely reporting 100% memory utilization for an empty node due to a flawed division-by-zero fix.

Pre-existing issues:
- [High] Potential integer underflow when evaluating `i.totalram - i.freeram` due to lockless vmstat reads.
- [High] The patch fixes a potential divide-by-zero in `damos_get_node_mem_bp()`, but fails to apply the same fix to `damos_get_node_memcg_used_bp()`.
- [High] Multiplication overflow in basis-point calculation on 32-bit architectures.
- [High] Potential divide by zero in `damos_get_in_active_mem_bp()` if the system has no active or inactive LRU pages.
--

--- Patch [7]: [RFC PATCH 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() ---
commit 0e2bb1b3ade5000769644aa674efbcfec5b43152
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/core: handle extreme memory state in damon_get_node_mem_bp()

In an extreme and unlikely situation, si_meminfo_node() might let the caller
show zero total ram. That could cause a divide by zero in
damon_get_node_mem_bp(). Fix it by setting the totalram one byte in the case.

Link: https://lore.kernel.org/20260328133216.9697-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2816,6 +2816,8 @@ static __kernel_ulong_t damos_get_node_mem_bp(
>  	}
>  
>  	si_meminfo_node(&i, goal->nid);
> +	if (!i.totalram)
> +		i.totalram = 1;
> +	if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP)
> +		numerator = i.totalram - i.freeram;

[Severity: Medium]
Does setting totalram to 1 artificially inflate the utilization for an empty
node?

If si_meminfo_node() returns 0 for both totalram and freeram, numerator
becomes 1 - 0 = 1. This would result in 100 percent used memory (10000 basis
points), which contradicts the fallback logic that reports 0 percent for an
empty node. Will this trigger DAMOS quota actions erroneously?

[Severity: High]
This is a pre-existing issue, but can this subtraction underflow?

Because si_meminfo_node() computes freeram via lockless zone stats, per-CPU
stat lag can temporarily cause freeram to exceed totalram. If freeram >
totalram, the unsigned long numerator will underflow to a massive value.

If we force totalram to 1 here and freeram is greater than 0, doesn't it
guarantee this underflow will occur?

> +	else	/* DAMOS_QUOTA_NODE_MEM_FREE_BP */

[ ... ]

[Severity: High]
This is a pre-existing issue, but does the same divide-by-zero happen in the
adjacent damos_get_node_memcg_used_bp() function?

mm/damon/core.c:damos_get_node_memcg_used_bp() {
        ...
        si_meminfo_node(&i, goal->nid);
        ...
        return mult_frac(numerator, 10000, i.totalram);
}

Since si_meminfo_node() can return 0 for totalram, wouldn't this also cause
a divide-by-zero here if i.totalram isn't validated?

[Severity: High]
This is a pre-existing issue, but can the basis-point calculation overflow
on 32-bit architectures?

mm/damon/core.c:damos_get_node_mem_bp() {
        ...
        return mult_frac(numerator, 10000, i.totalram);
}

Since mult_frac(numerator, 10000, denom) computes the remainder times 10000,
if the remainder exceeds roughly 429,496, won't this overflow a 32-bit
unsigned long and produce an incorrect result?

[Severity: High]
This is a pre-existing issue, but is there a potential divide-by-zero here
if the system has no active or inactive LRU pages?

mm/damon/core.c:damos_get_in_active_mem_bp() {
        ...
        total = active + inactive;
        if (active_ratio)
                return mult_frac(active, 10000, total);
        return mult_frac(inactive, 10000, total);
}

If all memory is mlocked or purely kernel-allocated, both active and
inactive could be 0, making total 0. Will mult_frac() panic when dividing
by total?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=7

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

* Re: [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
  2026-08-01 17:35 ` [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() SJ Park
@ 2026-08-01 19:31   ` sashiko-bot
  2026-08-01 20:26     ` SJ Park
  0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-08-01 19:31 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Fallback values for invalid nodes and missing memcgs incorrectly encourage aggressive scheme application instead of throttling.
- [High] Division by zero in `damos_get_in_active_mem_bp()` when total LRU memory is zero.
- [High] Division by zero in `damos_quota_score()` when a quota goal has a `target_value` of 0.
--

--- Patch [8]: [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() ---
commit 7261d4dd3fd172b597cdefa3462cc680deabe142
Author: SJ Park <sj@kernel.org>

mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()

This commit adds fallback logic to handle extreme situations where total 
memory is zero, or when lruvec_page_state() returns used memory larger 
than total memory. It returns 100% and 0% respectively to avoid division 
by zero or underflow in damos_get_node_memcg_used_bp().

Link: https://lore.kernel.org/20260329154813.47382-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 269865eb8fbfa..76764a2056f7c 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2858,10 +2858,16 @@ static unsigned long damos_get_node_memcg_used_bp(
>  	mem_cgroup_put(memcg);
>  
>  	si_meminfo_node(&i, goal->nid);
> -	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
> +	if (!i.totalram)
> +		return 10000;

[Severity: High]
This isn't a bug introduced by this patch, but returning 0 in other fallback 
paths for missing memcgs or invalid nodes incorrectly encourages aggressive 
scheme application. 

In the DAMOS quota auto-tuner (damon_feed_loop_next_input()), a metric score
of 0 evaluates as severe under-achievement, causing the tuner to aggressively
maximize the quota. To safely halt a scheme for invalid targets, the metric
getter should probably return a value that yields a high score (e.g., 10000).

However, multiple pre-existing fallback paths incorrectly return 0:

mm/damon/core.c:damos_get_node_memcg_used_bp()
	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
		return 0;

mm/damon/core.c:damos_get_node_mem_bp()
	else	/* DAMOS_QUOTA_NODE_MEM_FREE_BP */
		numerator = i.freeram;
	return mult_frac(numerator, 10000, i.totalram);
(when !totalram, this evaluates to 0)

mm/damon/core.c:damos_get_node_mem_bp()
	static __kernel_ulong_t damos_get_node_mem_bp(
			struct damos_quota_goal *goal)
	{
		return 0;
	}

Could these fallbacks cause the auto-tuner to unleash maximum operations and
lead to severe over-reclaim or wasted CPU when a node becomes invalid or a
memcg is destroyed?

> +	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP) {
>  		numerator = used_pages;
> -	else	/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
> +	} else {
> +		/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
> +		if (i.totalram < used_pages)
> +			return 0;
>  		numerator = i.totalram - used_pages;
> +	}
>  	return mult_frac(numerator, 10000, i.totalram);
>  }

[Severity: High]
This is a pre-existing issue with a similar root cause, but does this same
division by zero problem exist in damos_get_in_active_mem_bp()?

mm/damon/core.c:damos_get_in_active_mem_bp()
	if (active_ratio)
		return mult_frac(active, 10000, total);
	return mult_frac(inactive, 10000, total);

If the system has 0 pages on the LRU (e.g., during early boot, in extremely
constrained environments, or if all memory is unevictable), total evaluates
to 0. Would this result in a kernel panic when passed to mult_frac()?


[Severity: High]
This is another pre-existing issue, but does damos_quota_score() also suffer
from a division by zero when a quota goal has a target_value of 0?

mm/damon/core.c:damos_quota_score()
	highest_score = max(highest_score,
		mult_frac(goal->current_value, 10000,
			goal->target_value));

While the sysfs interface drops goals with a 0 target value, the core
allocation function damos_new_quota_goal() does not validate it. The mtier
sample module exposes these as module parameters:

samples/damon/mtier.c:damon_sample_mtier_build_ctx()
	quota_goal = damos_new_quota_goal(
		promote ? DAMOS_QUOTA_NODE_MEM_USED_BP :
		DAMOS_QUOTA_NODE_MEM_FREE_BP,
		promote ? node0_mem_used_bp : node0_mem_free_bp);

If an administrator or privileged user writes 0 to the parameter and enables
the module, would this immediately crash the kernel when the DAMOS worker
thread evaluates the quota?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=8

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

* Re: [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp()
  2026-08-01 17:35 ` [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() SJ Park
@ 2026-08-01 19:39   ` sashiko-bot
  2026-08-01 20:29     ` SJ Park
  0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-08-01 19:39 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The commit message incorrectly identifies the function being modified, failing to align its claims with the code changes.

Pre-existing issues:
- [High] Multiplication overflow in `mult_frac()` and other basis point scaling calculations on 32-bit architectures leads to incorrect memory ratio calculations and scaling.
--

--- Patch [9]: [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() ---
commit fee9e9f299e48399ca370c1f9725936302eaeb8c
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/core: handle extreme memory state in get_in_active_mem_bp()

This patch prevents a divide-by-zero risk in DAMON's active memory ratio
calculations. In extreme environments where active and inactive memory
both amount to zero, the patch clamps the total memory denominator to a
minimum of 1 to avoid a crash.

Link: https://lore.kernel.org/20260721034756.147011-1-sj@kernel.org

> damon_get_intervals_adaptation_bp() uses the sum of the active and
> inactive memory amount as a denominator.  In an extreme and unlikely
> environment, active and inactive memory might be zero.

[Severity: Medium]
Is the function name referenced in the commit message correct?

The commit message explicitly states that damon_get_intervals_adaptation_bp()
is being modified to fix a divide-by-zero bug, but the patch actually
modifies damos_get_in_active_mem_bp().

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 76764a2056f7c..4944cf2c5afae 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3010,7 +3010,7 @@ static unsigned int damos_get_in_active_mem_bp(bool active_ratio)
>  		global_node_page_state(NR_LRU_BASE + LRU_ACTIVE_FILE);
>  	inactive = global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_ANON) +
>  		global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_FILE);
> -	total = active + inactive;
> +	total = max(active + inactive, 1);
>  	if (active_ratio)
>  		return mult_frac(active, 10000, total);

[Severity: High]
This is a pre-existing issue, but does this calculation overflow on 32-bit
architectures?

In damos_get_in_active_mem_bp(), if active memory exceeds approximately
429,496 pages (~1.75 GB), multiplying the active page count by 10000 inside
mult_frac() will overflow an unsigned long on 32-bit systems before the
division by total. This could result in artificially low active/inactive
memory ratios and break DAMON's adaptive tuning.

Does this same overflow affect damon_feed_loop_next_input()?

That function returns last_input * score_cg / goal. When last_input is
passed as 100,000,000 and score_cg can be up to 10,000, that multiplication
would similarly wrap around 32-bit boundaries.

>  	return mult_frac(inactive, 10000, total);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=9

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

* Re: [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails
  2026-08-01 17:53   ` sashiko-bot
@ 2026-08-01 20:01     ` SJ Park
  2026-08-01 20:31       ` SJ Park
  0 siblings, 1 reply; 23+ messages in thread
From: SJ Park @ 2026-08-01 20:01 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 01 Aug 2026 17:53:34 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Unhandled failure of damon_split_region_at() in damos_filter_match() leads to bypassing or incorrectly applying address filters.
> --
> 
> --- Patch [1]: [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails ---
> commit 812934b542c69c596afadb66403f8c9c11c5bf91
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: skip applying scheme if region split for quota fails
> 
> damos_apply_scheme() splits a region and apply the action to the
> subregion if it is needed for not violating the quota.  The split
> operation (damon_split_region_at()) could fail for allocation failure.
> In the case, the quota could be violated.  From the user's perspective,
> DAMOS becomes more aggressive than expected under the extreme situation.
> Handle the failure.
> 
> Link: https://lore.kernel.org/20260718171523.87547-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 644daf5a16560..e2900d0c984c9 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2613,7 +2613,8 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
> >  					c->min_region_sz);
> >  			if (!sz)
> >  				goto update_stat;
> > -			damon_split_region_at(t, r, sz);
> > +			if (damon_split_region_at(t, r, sz))
> > +				goto update_stat;
> 
> [Severity: High]
> This is a pre-existing issue, but since we are handling the failure of
> damon_split_region_at() here, should we also handle it in
> damos_filter_match()?
> 
> If a memory region partially overlaps a filter range and the split
> allocation fails in damos_filter_match(), the return value is ignored:
> 
> mm/damon/core.c:damos_filter_match() {
>     ...
> 		/* start before the range and overlap */
> 		if (r->ar.start < start) {
> 			damon_split_region_at(t, r, start - r->ar.start);
> 			matched = false;
> 			break;
> 		}
> 		/* start inside the range */
> 		damon_split_region_at(t, r, end - r->ar.start);
> 		matched = true;
> 		break;
>     ...
> }
> 
> If the split fails, the intact region spanning both outside and inside the
> filter range is unconditionally evaluated as 'matched = false' or
> 'matched = true', incorrectly bypassing the filter or applying it to unrelated
> memory outside the intended range.
> 
> Could this lead to destructive actions (e.g., PAGEOUT) being applied to
> explicitly protected memory?

Nice catch.  I will fix this together.

> 
> >  		}
> >  		if (damos_core_filter_out(c, t, r, s))
> >  			return;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=1


Thanks,
SJ

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

* Re: [RFC PATCH 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT
  2026-08-01 18:21   ` sashiko-bot
@ 2026-08-01 20:07     ` SJ Park
  0 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-01 20:07 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 01 Aug 2026 18:21:01 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incomplete fix for large folio address iteration boundary bug.
> --
> 
> --- Patch [3]: [RFC PATCH 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT ---
> commit b98d6f7836f4a90b6724f683127a934a339f77d5
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/paddr: respect folio end for DAMOS_STAT
> 
> This commit fixes an issue in damon_pa_stat() where advancing the address
> by folio_size(folio) could skip subsequent folios if the start address
> is unaligned within a large folio. It corrects this by advancing the
> address to exactly the start address of the next folio.
> 
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index 5c6c3a597fd0b..2ab7b3842701e 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> > @@ -379,7 +379,7 @@ static unsigned long damon_pa_stat(struct damon_region *r,
> >  
> >  		if (!damos_pa_filter_out(s, folio))
> >  			*sz_filter_passed += folio_size(folio) / addr_unit;
> > -		addr += folio_size(folio);
> > +		addr = PFN_PHYS(folio_pfn(folio)) + folio_size(folio);
> 
> [Severity: High]
> This is a pre-existing issue, but does this same bug exist in other DAMON
> operations that iterate over folios?
> 
> For example, looking at damon_pa_pageout():
> 
> mm/damon/paddr.c:damon_pa_pageout() {
>     ...
> 		else
> 			list_add(&folio->lru, &folio_list);
> put_folio:
> 		addr += folio_size(folio);
> 		folio_put(folio);
> 	}
>     ...
> }
> 
> It appears damon_pa_pageout(), damon_pa_de_activate(), and damon_pa_migrate()
> all still advance the address by folio_size(folio) instead of calculating the
> exact folio end.
> 
> Could these operations also overshoot subsequent memory and skip valid memory
> folios if the region starts unaligned inside a large folio?

A later patch of this series is fixinfg the bug.

> 
> >  		folio_put(folio);
> >  	}
> >  	s->last_applied = folio;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=3


Thanks,
SJ

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

* Re: [RFC PATCH 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT
  2026-08-01 18:50   ` sashiko-bot
@ 2026-08-01 20:12     ` SJ Park
  0 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-01 20:12 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 01 Aug 2026 18:50:13 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [Medium] Direct dereference of PTE pointer `*pte` instead of using the local safely fetched `ptent`.
> 
> Pre-existing issues:
> - [High] Batched PTE advancement blindly skips interleaved folios and PTE-level Accessed/Young bits within a large folio's virtual footprint.
> - [High] The exact page iteration bug fixed in `damos_va_stat_pmd_entry()` is also present in `damos_va_migrate_pmd_entry()` but was missed by this patch.
> --
> 
> --- Patch [5]: [RFC PATCH 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT ---
> commit 20f53909d4e28dc2c1b2dd4ce52890349fc6c85d
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/vaddr: respect folio end for DAMOS_STAT
> 
> For applying DAMOS_STAT action to a region, DAMON virtual address space
> operation set (vaddr) calls walk_page_range[_vma]() for the region.  The
> pmd walk entry function, namely damon_va_stat_pmd_entry(), applies DAMOS
> filters to folios of addresses of the region in the pmd.  It starts from
> the walking address and advances the address by the size of the folio of
> the address until it goes out of the pmd or the region.
> 
> Link: https://lore.kernel.org/20260514015053.149396-1-sj@kernel.org
> 
> > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > index 0648400b2d65b..035cd509f6b58 100644
> > --- a/mm/damon/vaddr.c
> > +++ b/mm/damon/vaddr.c
> > @@ -831,6 +831,8 @@ static int damos_va_stat_pmd_entry(pmd_t *pmd, unsigned long addr,
> >  		return 0;
> >  
> >  	for (; addr < next; pte += nr, addr += nr * PAGE_SIZE) {
> > +		unsigned long page_idx;
> > +
> >  		nr = 1;
> >  		ptent = ptep_get(pte);
> >  
> > @@ -844,7 +846,8 @@ static int damos_va_stat_pmd_entry(pmd_t *pmd, unsigned long addr,
> >  
> >  		if (!damos_va_filter_out(s, folio, vma, addr, pte, NULL))
> >  			*sz_filter_passed += folio_size(folio);
> > -		nr = folio_nr_pages(folio);
> > +		page_idx = folio_page_idx(folio, pte_page(*pte));
> 
> [Severity: Medium]
> Is it safe to directly dereference *pte here instead of using the local
> safely fetched ptent?
> 
> Reading a PTE directly on 32-bit architectures (like x86 PAE) can result in
> a torn read if the hardware concurrently updates the Accessed/Dirty bits,
> potentially leading to an incorrect PFN and subsequent wild pointer
> arithmetic.
> 
> The local snapshot ptent was already obtained via ptep_get(pte) and should
> likely be used here.

Good catch, I will fix this in the next version.

> 
> > +		nr = folio_nr_pages(folio) - page_idx;
> 
> [Severity: High]
> This is a pre-existing issue, but does this batched PTE advancement blindly
> skip interleaved folios and PTE-level Accessed/Young bits within a large
> folio's virtual footprint?
> 
> For PTE-mapped large folios, hardware tracks the Accessed bit independently
> per 4K PTE. By checking only the first PTE and blindly skipping the rest,
> DAMON ignores accesses to subsequent pages of the folio, potentially
> classifying hot memory as cold.
> 
> Furthermore, if userspace partially unmaps or interleaves different mappings,
> the blind pte += nr skips those unrelated folios completely without
> verifying they map the same folio.

Good points that can further improved.  But seems handling these will make it
too complicated, compared to the gain from the improvements.  I will keep this
as is unless others raise different opinions.

> 
> >  		s->last_applied = folio;
> >  	}
> >  	pte_unmap_unlock(start_pte, ptl);
> 
> While reviewing this patch, I noticed the exact page iteration bug fixed in
> damos_va_stat_pmd_entry() is also present in damos_va_migrate_pmd_entry()
> but was missed by this patch.
> 
> [Severity: High]
> This is a pre-existing issue, but does the DAMOS migrate action also advance
> the PTE pointer unconditionally by the full size of the large folio?
> 
> mm/damon/vaddr.c:damos_va_migrate_pmd_entry() {
> 	...
> 	for (; addr < next; pte += nr, addr += nr * PAGE_SIZE) {
> 		...
> 		damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
> 				migration_lists);
> 		nr = folio_nr_pages(folio);
> 	}
> 	...
> }
> 
> If the scanning starts in the middle of a large folio, it will advance past
> the end of the large folio and skip unrelated folios that follow it, failing
> to migrate them.

Yes, a later patch in this series will fix it.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=5


Thanks,
SJ

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

* Re: [RFC PATCH 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp()
  2026-08-01 19:11   ` sashiko-bot
@ 2026-08-01 20:18     ` SJ Park
  0 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-01 20:18 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 01 Aug 2026 19:11:30 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> 
> New issues:
> - [Medium] Falsely reporting 100% memory utilization for an empty node due to a flawed division-by-zero fix.
> 
> Pre-existing issues:
> - [High] Potential integer underflow when evaluating `i.totalram - i.freeram` due to lockless vmstat reads.
> - [High] The patch fixes a potential divide-by-zero in `damos_get_node_mem_bp()`, but fails to apply the same fix to `damos_get_node_memcg_used_bp()`.
> - [High] Multiplication overflow in basis-point calculation on 32-bit architectures.
> - [High] Potential divide by zero in `damos_get_in_active_mem_bp()` if the system has no active or inactive LRU pages.
> --
> 
> --- Patch [7]: [RFC PATCH 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() ---
> commit 0e2bb1b3ade5000769644aa674efbcfec5b43152
> Author: SJ Park <sj@kernel.org>
> Subject: mm/damon/core: handle extreme memory state in damon_get_node_mem_bp()
> 
> In an extreme and unlikely situation, si_meminfo_node() might let the caller
> show zero total ram. That could cause a divide by zero in
> damon_get_node_mem_bp(). Fix it by setting the totalram one byte in the case.
> 
> Link: https://lore.kernel.org/20260328133216.9697-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2816,6 +2816,8 @@ static __kernel_ulong_t damos_get_node_mem_bp(
> >  	}
> >  
> >  	si_meminfo_node(&i, goal->nid);
> > +	if (!i.totalram)
> > +		i.totalram = 1;
> > +	if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP)
> > +		numerator = i.totalram - i.freeram;
> 
> [Severity: Medium]
> Does setting totalram to 1 artificially inflate the utilization for an empty
> node?
> 
> If si_meminfo_node() returns 0 for both totalram and freeram, numerator
> becomes 1 - 0 = 1. This would result in 100 percent used memory (10000 basis
> points), which contradicts the fallback logic that reports 0 percent for an
> empty node. Will this trigger DAMOS quota actions erroneously?

I think this is fine.  Just different perspectives, and no real problem in real
life.  We can later revisit for consistent interpretation of this extreme
situations.

> 
> [Severity: High]
> This is a pre-existing issue, but can this subtraction underflow?
> 
> Because si_meminfo_node() computes freeram via lockless zone stats, per-CPU
> stat lag can temporarily cause freeram to exceed totalram. If freeram >
> totalram, the unsigned long numerator will underflow to a massive value.
> 
> If we force totalram to 1 here and freeram is greater than 0, doesn't it
> guarantee this underflow will occur?

Good point, I will fix in the next version.

> 
> > +	else	/* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> 
> [ ... ]
> 
> [Severity: High]
> This is a pre-existing issue, but does the same divide-by-zero happen in the
> adjacent damos_get_node_memcg_used_bp() function?
> 
> mm/damon/core.c:damos_get_node_memcg_used_bp() {
>         ...
>         si_meminfo_node(&i, goal->nid);
>         ...
>         return mult_frac(numerator, 10000, i.totalram);
> }
> 
> Since si_meminfo_node() can return 0 for totalram, wouldn't this also cause
> a divide-by-zero here if i.totalram isn't validated?

A later patch in this series will fix it.

> 
> [Severity: High]
> This is a pre-existing issue, but can the basis-point calculation overflow
> on 32-bit architectures?
> 
> mm/damon/core.c:damos_get_node_mem_bp() {
>         ...
>         return mult_frac(numerator, 10000, i.totalram);
> }
> 
> Since mult_frac(numerator, 10000, denom) computes the remainder times 10000,
> if the remainder exceeds roughly 429,496, won't this overflow a 32-bit
> unsigned long and produce an incorrect result?

Known low priority issue.

> 
> [Severity: High]
> This is a pre-existing issue, but is there a potential divide-by-zero here
> if the system has no active or inactive LRU pages?
> 
> mm/damon/core.c:damos_get_in_active_mem_bp() {
>         ...
>         total = active + inactive;
>         if (active_ratio)
>                 return mult_frac(active, 10000, total);
>         return mult_frac(inactive, 10000, total);
> }
> 
> If all memory is mlocked or purely kernel-allocated, both active and
> inactive could be 0, making total 0. Will mult_frac() panic when dividing
> by total?

A later patch in this series will fix it.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=7


Thanks,
SJ

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

* Re: [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
  2026-08-01 19:31   ` sashiko-bot
@ 2026-08-01 20:26     ` SJ Park
  0 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-01 20:26 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 01 Aug 2026 19:31:01 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Fallback values for invalid nodes and missing memcgs incorrectly encourage aggressive scheme application instead of throttling.
> - [High] Division by zero in `damos_get_in_active_mem_bp()` when total LRU memory is zero.
> - [High] Division by zero in `damos_quota_score()` when a quota goal has a `target_value` of 0.
> --
> 
> --- Patch [8]: [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() ---
> commit 7261d4dd3fd172b597cdefa3462cc680deabe142
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
> 
> This commit adds fallback logic to handle extreme situations where total 
> memory is zero, or when lruvec_page_state() returns used memory larger 
> than total memory. It returns 100% and 0% respectively to avoid division 
> by zero or underflow in damos_get_node_memcg_used_bp().
> 
> Link: https://lore.kernel.org/20260329154813.47382-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 269865eb8fbfa..76764a2056f7c 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2858,10 +2858,16 @@ static unsigned long damos_get_node_memcg_used_bp(
> >  	mem_cgroup_put(memcg);
> >  
> >  	si_meminfo_node(&i, goal->nid);
> > -	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
> > +	if (!i.totalram)
> > +		return 10000;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but returning 0 in other fallback 
> paths for missing memcgs or invalid nodes incorrectly encourages aggressive 
> scheme application. 
> 
> In the DAMOS quota auto-tuner (damon_feed_loop_next_input()), a metric score
> of 0 evaluates as severe under-achievement, causing the tuner to aggressively
> maximize the quota. To safely halt a scheme for invalid targets, the metric
> getter should probably return a value that yields a high score (e.g., 10000).
> 
> However, multiple pre-existing fallback paths incorrectly return 0:
> 
> mm/damon/core.c:damos_get_node_memcg_used_bp()
> 	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
> 		return 0;
> 
> mm/damon/core.c:damos_get_node_mem_bp()
> 	else	/* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> 		numerator = i.freeram;
> 	return mult_frac(numerator, 10000, i.totalram);
> (when !totalram, this evaluates to 0)
> 
> mm/damon/core.c:damos_get_node_mem_bp()
> 	static __kernel_ulong_t damos_get_node_mem_bp(
> 			struct damos_quota_goal *goal)
> 	{
> 		return 0;
> 	}
> 
> Could these fallbacks cause the auto-tuner to unleash maximum operations and
> lead to severe over-reclaim or wasted CPU when a node becomes invalid or a
> memcg is destroyed?

This is an intended behavior.

> 
> > +	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP) {
> >  		numerator = used_pages;
> > -	else	/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
> > +	} else {
> > +		/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
> > +		if (i.totalram < used_pages)
> > +			return 0;
> >  		numerator = i.totalram - used_pages;
> > +	}
> >  	return mult_frac(numerator, 10000, i.totalram);
> >  }
> 
> [Severity: High]
> This is a pre-existing issue with a similar root cause, but does this same
> division by zero problem exist in damos_get_in_active_mem_bp()?
> 
> mm/damon/core.c:damos_get_in_active_mem_bp()
> 	if (active_ratio)
> 		return mult_frac(active, 10000, total);
> 	return mult_frac(inactive, 10000, total);
> 
> If the system has 0 pages on the LRU (e.g., during early boot, in extremely
> constrained environments, or if all memory is unevictable), total evaluates
> to 0. Would this result in a kernel panic when passed to mult_frac()?

A later patch in this series will fix it.

> 
> 
> [Severity: High]
> This is another pre-existing issue, but does damos_quota_score() also suffer
> from a division by zero when a quota goal has a target_value of 0?
> 
> mm/damon/core.c:damos_quota_score()
> 	highest_score = max(highest_score,
> 		mult_frac(goal->current_value, 10000,
> 			goal->target_value));
> 
> While the sysfs interface drops goals with a 0 target value, the core
> allocation function damos_new_quota_goal() does not validate it. The mtier
> sample module exposes these as module parameters:
> 
> samples/damon/mtier.c:damon_sample_mtier_build_ctx()
> 	quota_goal = damos_new_quota_goal(
> 		promote ? DAMOS_QUOTA_NODE_MEM_USED_BP :
> 		DAMOS_QUOTA_NODE_MEM_FREE_BP,
> 		promote ? node0_mem_used_bp : node0_mem_free_bp);
> 
> If an administrator or privileged user writes 0 to the parameter and enables
> the module, would this immediately crash the kernel when the DAMOS worker
> thread evaluates the quota?

Good catch, I will work on this separately or as a part of this series.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=8


Thanks,
SJ

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

* Re: [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp()
  2026-08-01 19:39   ` sashiko-bot
@ 2026-08-01 20:29     ` SJ Park
  0 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-01 20:29 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 01 Aug 2026 19:39:48 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Medium] The commit message incorrectly identifies the function being modified, failing to align its claims with the code changes.
> 
> Pre-existing issues:
> - [High] Multiplication overflow in `mult_frac()` and other basis point scaling calculations on 32-bit architectures leads to incorrect memory ratio calculations and scaling.
> --
> 
> --- Patch [9]: [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() ---
> commit fee9e9f299e48399ca370c1f9725936302eaeb8c
> Author: SJ Park <sj@kernel.org>
> Subject: mm/damon/core: handle extreme memory state in get_in_active_mem_bp()
> 
> This patch prevents a divide-by-zero risk in DAMON's active memory ratio
> calculations. In extreme environments where active and inactive memory
> both amount to zero, the patch clamps the total memory denominator to a
> minimum of 1 to avoid a crash.
> 
> Link: https://lore.kernel.org/20260721034756.147011-1-sj@kernel.org
> 
> > damon_get_intervals_adaptation_bp() uses the sum of the active and
> > inactive memory amount as a denominator.  In an extreme and unlikely
> > environment, active and inactive memory might be zero.
> 
> [Severity: Medium]
> Is the function name referenced in the commit message correct?
> 
> The commit message explicitly states that damon_get_intervals_adaptation_bp()
> is being modified to fix a divide-by-zero bug, but the patch actually
> modifies damos_get_in_active_mem_bp().

Good catch, I will fix.

> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 76764a2056f7c..4944cf2c5afae 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3010,7 +3010,7 @@ static unsigned int damos_get_in_active_mem_bp(bool active_ratio)
> >  		global_node_page_state(NR_LRU_BASE + LRU_ACTIVE_FILE);
> >  	inactive = global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_ANON) +
> >  		global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_FILE);
> > -	total = active + inactive;
> > +	total = max(active + inactive, 1);
> >  	if (active_ratio)
> >  		return mult_frac(active, 10000, total);
> 
> [Severity: High]
> This is a pre-existing issue, but does this calculation overflow on 32-bit
> architectures?
> 
> In damos_get_in_active_mem_bp(), if active memory exceeds approximately
> 429,496 pages (~1.75 GB), multiplying the active page count by 10000 inside
> mult_frac() will overflow an unsigned long on 32-bit systems before the
> division by total. This could result in artificially low active/inactive
> memory ratios and break DAMON's adaptive tuning.
> 
> Does this same overflow affect damon_feed_loop_next_input()?
> 
> That function returns last_input * score_cg / goal. When last_input is
> passed as 100,000,000 and score_cg can be up to 10,000, that multiplication
> would similarly wrap around 32-bit boundaries.

Known low priority issue.

> 
> >  	return mult_frac(inactive, 10000, total);
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=9


Thanks,
SJ

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

* Re: [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails
  2026-08-01 20:01     ` SJ Park
@ 2026-08-01 20:31       ` SJ Park
  0 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-01 20:31 UTC (permalink / raw)
  To: SJ Park; +Cc: sashiko-bot, damon

On Sat, 01 Aug 2026 13:02:08 -0700 SJ Park <sj@kernel.org> wrote:

> On Sat, 01 Aug 2026 17:53:34 +0000 sashiko-bot@kernel.org wrote:
> 
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > 
> > Pre-existing issues:
> > - [High] Unhandled failure of damon_split_region_at() in damos_filter_match() leads to bypassing or incorrectly applying address filters.
> > --
> > 
> > --- Patch [1]: [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails ---
> > commit 812934b542c69c596afadb66403f8c9c11c5bf91
> > Author: SJ Park <sj@kernel.org>
> > 
> > mm/damon/core: skip applying scheme if region split for quota fails
> > 
> > damos_apply_scheme() splits a region and apply the action to the
> > subregion if it is needed for not violating the quota.  The split
> > operation (damon_split_region_at()) could fail for allocation failure.
> > In the case, the quota could be violated.  From the user's perspective,
> > DAMOS becomes more aggressive than expected under the extreme situation.
> > Handle the failure.
> > 
> > Link: https://lore.kernel.org/20260718171523.87547-1-sj@kernel.org
> > 
> > > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > > index 644daf5a16560..e2900d0c984c9 100644
> > > --- a/mm/damon/core.c
> > > +++ b/mm/damon/core.c
> > > @@ -2613,7 +2613,8 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
> > >  					c->min_region_sz);
> > >  			if (!sz)
> > >  				goto update_stat;
> > > -			damon_split_region_at(t, r, sz);
> > > +			if (damon_split_region_at(t, r, sz))
> > > +				goto update_stat;
> > 
> > [Severity: High]
> > This is a pre-existing issue, but since we are handling the failure of
> > damon_split_region_at() here, should we also handle it in
> > damos_filter_match()?
> > 
> > If a memory region partially overlaps a filter range and the split
> > allocation fails in damos_filter_match(), the return value is ignored:
> > 
> > mm/damon/core.c:damos_filter_match() {
> >     ...
> > 		/* start before the range and overlap */
> > 		if (r->ar.start < start) {
> > 			damon_split_region_at(t, r, start - r->ar.start);
> > 			matched = false;
> > 			break;
> > 		}
> > 		/* start inside the range */
> > 		damon_split_region_at(t, r, end - r->ar.start);
> > 		matched = true;
> > 		break;
> >     ...
> > }
> > 
> > If the split fails, the intact region spanning both outside and inside the
> > filter range is unconditionally evaluated as 'matched = false' or
> > 'matched = true', incorrectly bypassing the filter or applying it to unrelated
> > memory outside the intended range.
> > 
> > Could this lead to destructive actions (e.g., PAGEOUT) being applied to
> > explicitly protected memory?
> 
> Nice catch.  I will fix this together.

But, it is unclear what to do in this case.  Maybe it is better to just back
off and cancel the DAMOS action.  And that would be no small change.  I will
work on it separately.

> 
> > 
> > >  		}
> > >  		if (damos_core_filter_out(c, t, r, s))
> > >  			return;
> > 
> > -- 
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=1
> 
> 
> Thanks,
> SJ
> 


Thanks,
SJ

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

end of thread, other threads:[~2026-08-01 20:31 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-01 17:35 [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
2026-08-01 17:35 ` [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails SJ Park
2026-08-01 17:53   ` sashiko-bot
2026-08-01 20:01     ` SJ Park
2026-08-01 20:31       ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total SJ Park
2026-08-01 17:35 ` [RFC PATCH 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT SJ Park
2026-08-01 18:21   ` sashiko-bot
2026-08-01 20:07     ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 4/9] mm/damon/paddr: respect folio end for DAMOS actions except STAT SJ Park
2026-08-01 17:35 ` [RFC PATCH 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT SJ Park
2026-08-01 18:50   ` sashiko-bot
2026-08-01 20:12     ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 6/9] mm/damon/vaddr: respect folio end for DAMOS_MIGRATE_{HOT,COLD} SJ Park
2026-08-01 17:35 ` [RFC PATCH 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() SJ Park
2026-08-01 19:11   ` sashiko-bot
2026-08-01 20:18     ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() SJ Park
2026-08-01 19:31   ` sashiko-bot
2026-08-01 20:26     ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() SJ Park
2026-08-01 19:39   ` sashiko-bot
2026-08-01 20:29     ` SJ Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).