* [RFC PATCH v1.1 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr
@ 2026-08-02 16:26 SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 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-02 16:26 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.
Changes from RFC
- RFC: https://lore.kernel.org/20260801173554.94710-1-sj@kernel.org
- Use cached pte content (patch 5 and 6).
- Handle totalram < freeram case (patch 7).
- Fix wrong function name in commit message (patch 9).
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 | 37 +++++++++++++++++++++++++++++++------
mm/damon/paddr.c | 8 ++++----
mm/damon/vaddr.c | 10 ++++++++--
3 files changed, 43 insertions(+), 12 deletions(-)
base-commit: 481be0db7cfd092f0aea64e65ab0a9cacff88453
--
2.47.3
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH v1.1 1/9] mm/damon/core: skip applying scheme if region split for quota fails
2026-08-02 16:26 [RFC PATCH v1.1 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
@ 2026-08-02 16:26 ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total SJ Park
` (7 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-02 16:26 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 v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total
2026-08-02 16:26 [RFC PATCH v1.1 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 1/9] mm/damon/core: skip applying scheme if region split for quota fails SJ Park
@ 2026-08-02 16:26 ` SJ Park
2026-08-02 16:43 ` sashiko-bot
2026-08-02 16:26 ` [RFC PATCH v1.1 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT SJ Park
` (6 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: SJ Park @ 2026-08-02 16:26 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 v1.1 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT
2026-08-02 16:26 [RFC PATCH v1.1 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 1/9] mm/damon/core: skip applying scheme if region split for quota fails SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total SJ Park
@ 2026-08-02 16:26 ` SJ Park
2026-08-02 16:34 ` sashiko-bot
2026-08-02 16:26 ` [RFC PATCH v1.1 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-02 16:26 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 v1.1 4/9] mm/damon/paddr: respect folio end for DAMOS actions except STAT
2026-08-02 16:26 [RFC PATCH v1.1 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
` (2 preceding siblings ...)
2026-08-02 16:26 ` [RFC PATCH v1.1 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT SJ Park
@ 2026-08-02 16:26 ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 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-02 16:26 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 v1.1 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT
2026-08-02 16:26 [RFC PATCH v1.1 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
` (3 preceding siblings ...)
2026-08-02 16:26 ` [RFC PATCH v1.1 4/9] mm/damon/paddr: respect folio end for DAMOS actions except STAT SJ Park
@ 2026-08-02 16:26 ` SJ Park
2026-08-02 16:36 ` sashiko-bot
2026-08-02 16:26 ` [RFC PATCH v1.1 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-02 16:26 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..4b0b5edf67952 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(ptent));
+ 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 v1.1 6/9] mm/damon/vaddr: respect folio end for DAMOS_MIGRATE_{HOT,COLD}
2026-08-02 16:26 [RFC PATCH v1.1 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
` (4 preceding siblings ...)
2026-08-02 16:26 ` [RFC PATCH v1.1 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT SJ Park
@ 2026-08-02 16:26 ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 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-02 16:26 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 4b0b5edf67952..c8c32b2ae0402 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(ptent));
+ 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 v1.1 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp()
2026-08-02 16:26 [RFC PATCH v1.1 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
` (5 preceding siblings ...)
2026-08-02 16:26 ` [RFC PATCH v1.1 6/9] mm/damon/vaddr: respect folio end for DAMOS_MIGRATE_{HOT,COLD} SJ Park
@ 2026-08-02 16:26 ` SJ Park
2026-08-02 16:35 ` sashiko-bot
2026-08-02 16:26 ` [RFC PATCH v1.1 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 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-02 16:26 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 | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 3bdbf4fbf7147..e3f3ee75a3d33 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2816,10 +2816,16 @@ static __kernel_ulong_t damos_get_node_mem_bp(
}
si_meminfo_node(&i, goal->nid);
- if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP)
+ if (!i.totalram)
+ return 10000;
+ if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP) {
numerator = i.totalram - i.freeram;
- else /* DAMOS_QUOTA_NODE_MEM_FREE_BP */
+ } else {
+ /* DAMOS_QUOTA_NODE_MEM_FREE_BP */
+ if (i.totalram < i.freeram)
+ return 0;
numerator = i.freeram;
+ }
return mult_frac(numerator, 10000, i.totalram);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH v1.1 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
2026-08-02 16:26 [RFC PATCH v1.1 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
` (6 preceding siblings ...)
2026-08-02 16:26 ` [RFC PATCH v1.1 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() SJ Park
@ 2026-08-02 16:26 ` SJ Park
2026-08-02 16:38 ` sashiko-bot
2026-08-02 16:26 ` [RFC PATCH v1.1 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-02 16:26 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 e3f3ee75a3d33..67ad1f07c29a4 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2862,10 +2862,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 v1.1 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp()
2026-08-02 16:26 [RFC PATCH v1.1 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
` (7 preceding siblings ...)
2026-08-02 16:26 ` [RFC PATCH v1.1 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() SJ Park
@ 2026-08-02 16:26 ` SJ Park
2026-08-02 16:51 ` sashiko-bot
8 siblings, 1 reply; 23+ messages in thread
From: SJ Park @ 2026-08-02 16:26 UTC (permalink / raw)
Cc: SJ Park, stable, Andrew Morton, damon, linux-kernel, linux-mm
damos_get_in_active_mem_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 67ad1f07c29a4..c79b854e1a971 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3014,7 +3014,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 v1.1 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT
2026-08-02 16:26 ` [RFC PATCH v1.1 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT SJ Park
@ 2026-08-02 16:34 ` sashiko-bot
2026-08-02 17:43 ` SJ Park
0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-08-02 16:34 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] This problem wasn't introduced by this patch, but the patch provides an incomplete fix for an address advancement logic bug. While it corrects the `addr += folio_size(folio)` issue in `damon_pa_stat()` to correctly align the next address to the end of the large folio, it fails to apply the exact same fix to `damon_pa_pageout()`, `damon_pa_de_activate()`, and `damon_pa_migrate()`.
--
--- Patch [3]: [RFC PATCH v1.1 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT ---
commit 99e0c032eda9c903df8d327aa77b989129d9b193
Author: SJ Park <sj@kernel.org>
mm/damon/paddr: respect folio end for DAMOS_STAT
This commit fixes the address advancement in damon_pa_stat(). When a region
starts in the middle of a large folio, it now properly aligns to the start of
the next folio instead of simply adding the folio size and overshooting.
> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> --- 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 isn't a bug introduced by this patch, but does the same address
advancement logic regression still exist in other DAMON actions?
While you fixed the overshoot behavior here in damon_pa_stat(), it looks like
damon_pa_pageout(), damon_pa_de_activate(), and damon_pa_migrate() in
mm/damon/paddr.c might still be using the flawed logic.
For example, in damon_pa_pageout():
put_folio:
addr += folio_size(folio);
folio_put(folio);
Could this cause DAMON actions to silently skip sections of valid memory
if a region starts in the middle of a large folio?
> folio_put(folio);
> }
> s->last_applied = folio;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=3
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v1.1 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp()
2026-08-02 16:26 ` [RFC PATCH v1.1 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() SJ Park
@ 2026-08-02 16:35 ` sashiko-bot
2026-08-02 17:49 ` SJ Park
0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-08-02 16:35 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Returning 0 for DAMOS_QUOTA_NODE_MEM_FREE_BP when freeram > totalram incorrectly forces maximum scheme aggressiveness instead of reporting 100% free memory.
- [Medium] The commit message contradicts the implementation, and the unconditional return of 10000 for zero totalram breaks metric semantics.
Pre-existing issues:
- [High] Unhandled integer underflow in MEM_USED_BP and MEMCG_FREE_BP calculations when counters drift.
- [High] The patch fixes a potential divide-by-zero in damos_get_node_mem_bp() but misses identical unvalidated divisions in related functions.
--
--- Patch [7]: [RFC PATCH v1.1 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() ---
commit 05d27d4d51952669ff23ec9285ccb83a273556ad
Author: SJ Park <sj@kernel.org>
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
> 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.
[Severity: Medium]
The commit message states "Fix it by setting the totalram one byte in the
case."
However, the implementation actually bypasses the metric type check and returns
10000.
Does this unconditional return contradict the intent documented in the commit
message?
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 3bdbf4fbf7147..e3f3ee75a3d33 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2816,10 +2816,16 @@ static __kernel_ulong_t damos_get_node_mem_bp(
> }
>
> si_meminfo_node(&i, goal->nid);
> - if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP)
> + if (!i.totalram)
> + return 10000;
[Severity: Medium]
If the metric is DAMOS_QUOTA_NODE_MEM_FREE_BP, shouldn't it report 0% free
rather than 100% free if the memory is absent (which is what totalram=1,
freeram=0 would yield)?
Does returning 10000 here break the semantic constraint that used + free = 100%?
> + if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP) {
> numerator = i.totalram - i.freeram;
[Severity: High]
This is a pre-existing issue, but if counter drift results in
freeram > totalram, does this subtraction silently underflow, producing a
massive unsigned value?
If this is passed to mult_frac(), could it generate a huge metric value,
leading the auto-tuner to mistakenly act as if the memory is massively
overused?
An identical unhandled underflow also appears to exist in
damos_get_node_memcg_used_bp() when evaluating DAMOS_QUOTA_NODE_MEMCG_FREE_BP
if used_pages exceeds totalram:
mm/damon/core.c:damos_get_node_memcg_used_bp() {
...
numerator = i.totalram - used_pages;
...
}
> - else /* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> + } else {
> + /* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> + if (i.totalram < i.freeram)
> + return 0;
[Severity: High]
When counter drift causes freeram > totalram, the node effectively has
~100% free memory.
By returning 0 (0% free memory) in this scenario, doesn't the DAMON quota
auto-tuning feedback loop interpret this as severely under-achieving the free
memory goal, causing it to wastefully and drastically increase the reclamation
budget (e.g., pageout/demotion) on a node that is already completely free?
Should this return 10000 (100% free) instead?
> numerator = i.freeram;
> + }
> return mult_frac(numerator, 10000, i.totalram);
> }
[Severity: High]
This is a pre-existing issue, but while this patch addresses a potential
divide-by-zero when si_meminfo_node() reports totalram == 0, does the same
unvalidated division exist in related functions?
For example, damos_get_node_memcg_used_bp() divides by i.totalram without a
zero-check:
mm/damon/core.c:damos_get_node_memcg_used_bp() {
...
si_meminfo_node(&i, goal->nid);
...
return mult_frac(numerator, 10000, i.totalram);
}
Similarly, damos_get_in_active_mem_bp() divides by total (active + inactive)
via mult_frac() without checking if total is zero:
mm/damon/core.c:damos_get_in_active_mem_bp() {
...
if (goal->metric == DAMOS_QUOTA_SOME_MEM_USED_BP)
return mult_frac(active, 10000, total);
return mult_frac(inactive, 10000, total);
}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=7
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v1.1 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT
2026-08-02 16:26 ` [RFC PATCH v1.1 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT SJ Park
@ 2026-08-02 16:36 ` sashiko-bot
2026-08-02 17:45 ` SJ Park
0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-08-02 16:36 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] Unsafe blind PTE batching skips intermediate mappings and misses per-PTE Accessed/Young bits for large folios.
--
--- Patch [5]: [RFC PATCH v1.1 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT ---
commit d6ffeea9e85e0fc3aa2c2575b8113904f9da4123
Author: SJ Park <sj@kernel.org>
mm/damon/vaddr: respect folio end for DAMOS_STAT
This commit fixes an issue in DAMOS_STAT page-level monitoring where the
address advancement could incorrectly skip folios if a region starts in the
middle of a large folio. It ensures the address is advanced exactly to the
start of the next folio by calculating the correct page index.
[1] https://lore.kernel.org/20260514015053.149396-1-sj@kernel.org
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index 0648400b2d65b..4b0b5edf67952 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
[ ... ]
> @@ -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(ptent));
> + nr = folio_nr_pages(folio) - page_idx;
[Severity: High]
This is a pre-existing issue, but does this blind PTE batching skip
intermediate mappings and miss per-PTE Accessed/Young bits for large folios?
When damos_va_filter_out() tests the young filter, it only checks the
accessed bit of the first pte. By unconditionally advancing pte += nr without
aggregating the accessed bits of the remaining subpages, could this miss
hardware accesses on PTE-mapped large folios (like mTHP) and cause DAMON to
falsely classify actively used memory as cold?
Additionally, if userspace partially unmaps or remaps portions of this large
folio (for example, via MADV_DONTNEED or mmap), the page table might contain
different folios or empty entries in the skipped range. By blindly advancing
the PTE walker based purely on the first folio's size, does this ignore those
mixed mappings and fail to monitor the substituted folios?
> s->last_applied = folio;
> }
> pte_unmap_unlock(start_pte, ptl);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=5
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v1.1 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
2026-08-02 16:26 ` [RFC PATCH v1.1 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() SJ Park
@ 2026-08-02 16:38 ` sashiko-bot
2026-08-02 17:51 ` SJ Park
0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-08-02 16:38 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] This problem wasn't introduced by this patch, but damos_get_node_mem_bp() contains a severe logic bug and underflow when free memory exceeds total memory (i.totalram < i.freeram).
--
--- Patch [8]: [RFC PATCH v1.1 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() ---
commit 0fb31bc202c766c30f5d776d57b867135fba4aa6
Author: SJ Park <sj@kernel.org>
mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
This commit addresses division by zero and underflow issues in
damos_get_node_memcg_used_bp() by handling cases where total memory is zero or
used memory exceeds total memory. It returns 100% and 0% for these respective
edge cases to prevent anomalous behavior.
Link: https://lore.kernel.org/20260329154813.47382-1-sj@kernel.org
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index e3f3ee75a3d33..67ad1f07c29a4 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2862,10 +2862,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;
[Severity: High]
This isn't a bug introduced by this patch, but does the adjacent
damos_get_node_mem_bp() function contain a logic bug when free memory exceeds
total memory?
If i.totalram < i.freeram, it appears that for DAMOS_QUOTA_NODE_MEM_USED_BP,
i.totalram - i.freeram will unconditionally underflow, falsely throttling or
activating schemes. For DAMOS_QUOTA_NODE_MEM_FREE_BP, returning 0 means it
reports 0% free memory when the node is actually >100% free, which could cause
schemes waiting for low free memory to trigger aggressively.
mm/damon/core.c:damos_get_node_mem_bp() {
...
if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP) {
numerator = i.totalram - i.freeram;
} else {
/* DAMOS_QUOTA_NODE_MEM_FREE_BP */
if (i.totalram < i.freeram)
return 0;
numerator = i.freeram;
}
...
}
Should this pre-existing issue in damos_get_node_mem_bp() be addressed as well,
perhaps by returning 0 for USED_BP and 10000 for FREE_BP when
i.totalram < i.freeram?
> numerator = i.totalram - used_pages;
> + }
> return mult_frac(numerator, 10000, i.totalram);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=8
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total
2026-08-02 16:26 ` [RFC PATCH v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total SJ Park
@ 2026-08-02 16:43 ` sashiko-bot
2026-08-02 17:41 ` SJ Park
0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-08-02 16:43 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:
- [Medium] The patch incompletely fixes the uninitialized `last_psi_total` variable usage. While it initializes `last_psi_total` when the `kdamond` thread starts, it fails to initialize it for schemes or quota goals added dynamically at runtime via sysfs.
--
--- Patch [2]: [RFC PATCH v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total ---
commit ca3c4694fe111f676d0de56148cf986c02532428
Author: SJ Park <sj@kernel.org>
mm/damon/core: initialize damos_quota_goal->last_psi_total
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.
This commit initializes the value at the beginning of kdamond.
Link: https://lore.kernel.org/20260718005316.89585-1-sj@kernel.org
> 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
[ ... ]
> @@ -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);
[Severity: Medium]
This is a pre-existing issue, but does this completely fix the uninitialized
variable usage?
While this initializes last_psi_total when the kdamond thread starts, it
appears goals added dynamically at runtime via sysfs will still be
uninitialized.
When a goal is parsed from sysfs, memory is allocated without zeroing the
structure, leaving last_psi_total filled with slab garbage:
mm/damon/sysfs-schemes.c:damos_sysfs_add_quota_score() {
...
goal = damos_new_quota_goal(sysfs_goal->metric,
sysfs_goal->target_value);
...
}
During the context commit phase, the goal is dynamically allocated but
last_psi_total is intentionally bypassed:
mm/damon/core.c:damos_commit_quota_goals() {
...
new_goal = damos_new_quota_goal(
src_goal->metric, src_goal->target_value);
...
}
When the feedback loop executes, this uninitialized memory is consumed:
mm/damon/core.c:damos_set_quota_goal_current_value() {
...
goal->current_value = now_psi_total - goal->last_psi_total;
...
}
Will this cause a random, extremely large or small initial quota adjustment
when a user adds a new DAMON scheme or quota goal with the
DAMOS_QUOTA_SOME_MEM_PSI_US metric to a running context via sysfs?
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=2
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v1.1 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp()
2026-08-02 16:26 ` [RFC PATCH v1.1 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() SJ Park
@ 2026-08-02 16:51 ` sashiko-bot
2026-08-02 17:51 ` SJ Park
0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-08-02 16:51 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] 32-bit integer overflows and truncations in DAMON memory metric calculations cause wildly incorrect quota logic on 32-bit architectures.
--
--- Patch [9]: [RFC PATCH v1.1 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() ---
commit e72445b8d3c8d0866c458b7707c7433d7912c2e6
Author: SJ Park <sj@kernel.org>
mm/damon/core: handle extreme memory state in get_in_active_mem_bp()
This commit addresses a divide-by-zero issue in damos_get_in_active_mem_bp().
It ensures the denominator is at least 1 when calculating the ratio of
active or inactive memory, preventing a crash if both are zero.
Link: https://lore.kernel.org/20260721034756.147011-1-sj@kernel.org
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 67ad1f07c29a4..c79b854e1a971 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3014,7 +3014,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)
[Severity: High]
This is a pre-existing issue, but does this code overflow 32-bit unsigned
math on 32-bit architectures?
The mult_frac(x, n, d) macro computes (x / d) * n + (x % d) * n / d. Since
n is 10000, the multiplication x * 10000 is performed using 32-bit unsigned
arithmetic on 32-bit systems (where unsigned long is 32 bits). If active
memory exceeds 429,496 pages (~1.76 GB), the multiplication overflows
ULONG_MAX, resulting in an incorrect ratio.
Does this same overflow also affect other DAMON memory metric calculations?
For example, in damos_get_node_eligible_mem_bp():
return mult_frac((unsigned long)node_eligible, 10000,
(unsigned long)total_eligible);
This explicitly casts a 64-bit phys_addr_t to unsigned long, truncating
capacities over 4GB on 32-bit systems. It also overflows mult_frac() if the
eligible memory exceeds just ~419 KB.
Similarly, in damon_get_intervals_score():
max_access_events += sz_region * c->attrs.aggr_samples;
access_events += sz_region * r->nr_accesses;
Because sz_region is in bytes, accumulating these into an unsigned long
seems to overflow for regions larger than a few tens of megabytes.
> return mult_frac(active, 10000, total);
> return mult_frac(inactive, 10000, total);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=9
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total
2026-08-02 16:43 ` sashiko-bot
@ 2026-08-02 17:41 ` SJ Park
0 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-02 17:41 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Sun, 02 Aug 2026 16:43:14 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Medium] The patch incompletely fixes the uninitialized `last_psi_total` variable usage. While it initializes `last_psi_total` when the `kdamond` thread starts, it fails to initialize it for schemes or quota goals added dynamically at runtime via sysfs.
> --
>
> --- Patch [2]: [RFC PATCH v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total ---
> commit ca3c4694fe111f676d0de56148cf986c02532428
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: initialize damos_quota_goal->last_psi_total
>
> 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.
> This commit initializes the value at the beginning of kdamond.
>
> Link: https://lore.kernel.org/20260718005316.89585-1-sj@kernel.org
>
> > 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
>
> [ ... ]
>
> > @@ -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);
>
> [Severity: Medium]
> This is a pre-existing issue, but does this completely fix the uninitialized
> variable usage?
>
> While this initializes last_psi_total when the kdamond thread starts, it
> appears goals added dynamically at runtime via sysfs will still be
> uninitialized.
>
> When a goal is parsed from sysfs, memory is allocated without zeroing the
> structure, leaving last_psi_total filled with slab garbage:
>
> mm/damon/sysfs-schemes.c:damos_sysfs_add_quota_score() {
> ...
> goal = damos_new_quota_goal(sysfs_goal->metric,
> sysfs_goal->target_value);
> ...
> }
>
> During the context commit phase, the goal is dynamically allocated but
> last_psi_total is intentionally bypassed:
>
> mm/damon/core.c:damos_commit_quota_goals() {
> ...
> new_goal = damos_new_quota_goal(
> src_goal->metric, src_goal->target_value);
> ...
> }
>
> When the feedback loop executes, this uninitialized memory is consumed:
>
> mm/damon/core.c:damos_set_quota_goal_current_value() {
> ...
> goal->current_value = now_psi_total - goal->last_psi_total;
> ...
> }
>
> Will this cause a random, extremely large or small initial quota adjustment
> when a user adds a new DAMON scheme or quota goal with the
> DAMOS_QUOTA_SOME_MEM_PSI_US metric to a running context via sysfs?
Good catch. To handle this, the fix will be little bit complicated. It may
not deserve stable@ backport. I will drop this from the next version of this
series and work on more complete fix.
>
> > }
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=2
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v1.1 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT
2026-08-02 16:34 ` sashiko-bot
@ 2026-08-02 17:43 ` SJ Park
0 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-02 17:43 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Sun, 02 Aug 2026 16:34:57 +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] This problem wasn't introduced by this patch, but the patch provides an incomplete fix for an address advancement logic bug. While it corrects the `addr += folio_size(folio)` issue in `damon_pa_stat()` to correctly align the next address to the end of the large folio, it fails to apply the exact same fix to `damon_pa_pageout()`, `damon_pa_de_activate()`, and `damon_pa_migrate()`.
> --
>
> --- Patch [3]: [RFC PATCH v1.1 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT ---
> commit 99e0c032eda9c903df8d327aa77b989129d9b193
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/paddr: respect folio end for DAMOS_STAT
>
> This commit fixes the address advancement in damon_pa_stat(). When a region
> starts in the middle of a large folio, it now properly aligns to the start of
> the next folio instead of simply adding the folio size and overshooting.
>
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > --- 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 isn't a bug introduced by this patch, but does the same address
> advancement logic regression still exist in other DAMON actions?
>
> While you fixed the overshoot behavior here in damon_pa_stat(), it looks like
> damon_pa_pageout(), damon_pa_de_activate(), and damon_pa_migrate() in
> mm/damon/paddr.c might still be using the flawed logic.
>
> For example, in damon_pa_pageout():
>
> put_folio:
> addr += folio_size(folio);
> folio_put(folio);
>
> Could this cause DAMON actions to silently skip sections of valid memory
> if a region starts in the middle of a large folio?
A later patch of this series fix that.
>
> > folio_put(folio);
> > }
> > s->last_applied = folio;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=3
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v1.1 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT
2026-08-02 16:36 ` sashiko-bot
@ 2026-08-02 17:45 ` SJ Park
0 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-02 17:45 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Sun, 02 Aug 2026 16:36:30 +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] Unsafe blind PTE batching skips intermediate mappings and misses per-PTE Accessed/Young bits for large folios.
> --
>
> --- Patch [5]: [RFC PATCH v1.1 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT ---
> commit d6ffeea9e85e0fc3aa2c2575b8113904f9da4123
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/vaddr: respect folio end for DAMOS_STAT
>
> This commit fixes an issue in DAMOS_STAT page-level monitoring where the
> address advancement could incorrectly skip folios if a region starts in the
> middle of a large folio. It ensures the address is advanced exactly to the
> start of the next folio by calculating the correct page index.
>
> [1] https://lore.kernel.org/20260514015053.149396-1-sj@kernel.org
>
> > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > index 0648400b2d65b..4b0b5edf67952 100644
> > --- a/mm/damon/vaddr.c
> > +++ b/mm/damon/vaddr.c
> [ ... ]
> > @@ -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(ptent));
> > + nr = folio_nr_pages(folio) - page_idx;
>
> [Severity: High]
> This is a pre-existing issue, but does this blind PTE batching skip
> intermediate mappings and miss per-PTE Accessed/Young bits for large folios?
>
> When damos_va_filter_out() tests the young filter, it only checks the
> accessed bit of the first pte. By unconditionally advancing pte += nr without
> aggregating the accessed bits of the remaining subpages, could this miss
> hardware accesses on PTE-mapped large folios (like mTHP) and cause DAMON to
> falsely classify actively used memory as cold?
>
> Additionally, if userspace partially unmaps or remaps portions of this large
> folio (for example, via MADV_DONTNEED or mmap), the page table might contain
> different folios or empty entries in the skipped range. By blindly advancing
> the PTE walker based purely on the first folio's size, does this ignore those
> mixed mappings and fail to monitor the substituted folios?
Handling all the cases would be too complicated for backporting. I will
revisit this later.
>
> > s->last_applied = folio;
> > }
> > pte_unmap_unlock(start_pte, ptl);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=5
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v1.1 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp()
2026-08-02 16:35 ` sashiko-bot
@ 2026-08-02 17:49 ` SJ Park
2026-08-02 17:59 ` SJ Park
0 siblings, 1 reply; 23+ messages in thread
From: SJ Park @ 2026-08-02 17:49 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Sun, 02 Aug 2026 16:35:44 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
>
> New issues:
> - [High] Returning 0 for DAMOS_QUOTA_NODE_MEM_FREE_BP when freeram > totalram incorrectly forces maximum scheme aggressiveness instead of reporting 100% free memory.
> - [Medium] The commit message contradicts the implementation, and the unconditional return of 10000 for zero totalram breaks metric semantics.
>
> Pre-existing issues:
> - [High] Unhandled integer underflow in MEM_USED_BP and MEMCG_FREE_BP calculations when counters drift.
> - [High] The patch fixes a potential divide-by-zero in damos_get_node_mem_bp() but misses identical unvalidated divisions in related functions.
> --
>
> --- Patch [7]: [RFC PATCH v1.1 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() ---
> commit 05d27d4d51952669ff23ec9285ccb83a273556ad
> Author: SJ Park <sj@kernel.org>
>
> 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
>
> > 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.
>
> [Severity: Medium]
> The commit message states "Fix it by setting the totalram one byte in the
> case."
>
> However, the implementation actually bypasses the metric type check and returns
> 10000.
>
> Does this unconditional return contradict the intent documented in the commit
> message?
Good catch, I will update the commit message.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 3bdbf4fbf7147..e3f3ee75a3d33 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2816,10 +2816,16 @@ static __kernel_ulong_t damos_get_node_mem_bp(
> > }
> >
> > si_meminfo_node(&i, goal->nid);
> > - if (goal->metric = DAMOS_QUOTA_NODE_MEM_USED_BP)
> > + if (!i.totalram)
> > + return 10000;
>
> [Severity: Medium]
> If the metric is DAMOS_QUOTA_NODE_MEM_FREE_BP, shouldn't it report 0% free
> rather than 100% free if the memory is absent (which is what totalram=1,
> freeram=0 would yield)?
>
> Does returning 10000 here break the semantic constraint that used + free = 100%?
I made mistakes at revisioning. I will fix this.
>
> > + if (goal->metric = DAMOS_QUOTA_NODE_MEM_USED_BP) {
> > numerator = i.totalram - i.freeram;
>
> [Severity: High]
> This is a pre-existing issue, but if counter drift results in
> freeram > totalram, does this subtraction silently underflow, producing a
> massive unsigned value?
>
> If this is passed to mult_frac(), could it generate a huge metric value,
> leading the auto-tuner to mistakenly act as if the memory is massively
> overused?
>
> An identical unhandled underflow also appears to exist in
> damos_get_node_memcg_used_bp() when evaluating DAMOS_QUOTA_NODE_MEMCG_FREE_BP
> if used_pages exceeds totalram:
>
> mm/damon/core.c:damos_get_node_memcg_used_bp() {
> ...
> numerator = i.totalram - used_pages;
> ...
> }
The intention of this revision was handling this, but I made a complete
mistake. I will fix.
>
> > - else /* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> > + } else {
> > + /* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> > + if (i.totalram < i.freeram)
> > + return 0;
>
> [Severity: High]
> When counter drift causes freeram > totalram, the node effectively has
> ~100% free memory.
>
> By returning 0 (0% free memory) in this scenario, doesn't the DAMON quota
> auto-tuning feedback loop interpret this as severely under-achieving the free
> memory goal, causing it to wastefully and drastically increase the reclamation
> budget (e.g., pageout/demotion) on a node that is already completely free?
>
> Should this return 10000 (100% free) instead?
That depends on the perspective. I think 0% makes sense.
>
> > numerator = i.freeram;
> > + }
> > return mult_frac(numerator, 10000, i.totalram);
> > }
>
> [Severity: High]
> This is a pre-existing issue, but while this patch addresses a potential
> divide-by-zero when si_meminfo_node() reports totalram = 0, does the same
> unvalidated division exist in related functions?
>
> For example, damos_get_node_memcg_used_bp() divides by i.totalram without a
> zero-check:
>
> mm/damon/core.c:damos_get_node_memcg_used_bp() {
> ...
> si_meminfo_node(&i, goal->nid);
> ...
> return mult_frac(numerator, 10000, i.totalram);
> }
>
> Similarly, damos_get_in_active_mem_bp() divides by total (active + inactive)
> via mult_frac() without checking if total is zero:
>
> mm/damon/core.c:damos_get_in_active_mem_bp() {
> ...
> if (goal->metric = DAMOS_QUOTA_SOME_MEM_USED_BP)
> return mult_frac(active, 10000, total);
> return mult_frac(inactive, 10000, total);
> }
Later patches in this series fix those.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=7
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v1.1 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
2026-08-02 16:38 ` sashiko-bot
@ 2026-08-02 17:51 ` SJ Park
0 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-02 17:51 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Sun, 02 Aug 2026 16:38:33 +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] This problem wasn't introduced by this patch, but damos_get_node_mem_bp() contains a severe logic bug and underflow when free memory exceeds total memory (i.totalram < i.freeram).
> --
>
> --- Patch [8]: [RFC PATCH v1.1 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() ---
> commit 0fb31bc202c766c30f5d776d57b867135fba4aa6
> Author: SJ Park <sj@kernel.org>
> mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
>
> This commit addresses division by zero and underflow issues in
> damos_get_node_memcg_used_bp() by handling cases where total memory is zero or
> used memory exceeds total memory. It returns 100% and 0% for these respective
> edge cases to prevent anomalous behavior.
>
> Link: https://lore.kernel.org/20260329154813.47382-1-sj@kernel.org
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index e3f3ee75a3d33..67ad1f07c29a4 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2862,10 +2862,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;
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does the adjacent
> damos_get_node_mem_bp() function contain a logic bug when free memory exceeds
> total memory?
>
> If i.totalram < i.freeram, it appears that for DAMOS_QUOTA_NODE_MEM_USED_BP,
> i.totalram - i.freeram will unconditionally underflow, falsely throttling or
> activating schemes. For DAMOS_QUOTA_NODE_MEM_FREE_BP, returning 0 means it
> reports 0% free memory when the node is actually >100% free, which could cause
> schemes waiting for low free memory to trigger aggressively.
>
> mm/damon/core.c:damos_get_node_mem_bp() {
> ...
> if (goal->metric = DAMOS_QUOTA_NODE_MEM_USED_BP) {
> numerator = i.totalram - i.freeram;
> } else {
> /* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> if (i.totalram < i.freeram)
> return 0;
> numerator = i.freeram;
> }
> ...
> }
>
> Should this pre-existing issue in damos_get_node_mem_bp() be addressed as well,
> perhaps by returning 0 for USED_BP and 10000 for FREE_BP when
> i.totalram < i.freeram?
The previous patch was completely broken by my mistake. I will fix.
>
> > numerator = i.totalram - used_pages;
> > + }
> > return mult_frac(numerator, 10000, i.totalram);
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=8
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v1.1 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp()
2026-08-02 16:51 ` sashiko-bot
@ 2026-08-02 17:51 ` SJ Park
0 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-02 17:51 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Sun, 02 Aug 2026 16:51:05 +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] 32-bit integer overflows and truncations in DAMON memory metric calculations cause wildly incorrect quota logic on 32-bit architectures.
> --
>
> --- Patch [9]: [RFC PATCH v1.1 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() ---
> commit e72445b8d3c8d0866c458b7707c7433d7912c2e6
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: handle extreme memory state in get_in_active_mem_bp()
>
> This commit addresses a divide-by-zero issue in damos_get_in_active_mem_bp().
> It ensures the denominator is at least 1 when calculating the ratio of
> active or inactive memory, preventing a crash if both are zero.
>
> Link: https://lore.kernel.org/20260721034756.147011-1-sj@kernel.org
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 67ad1f07c29a4..c79b854e1a971 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3014,7 +3014,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)
>
> [Severity: High]
> This is a pre-existing issue, but does this code overflow 32-bit unsigned
> math on 32-bit architectures?
>
> The mult_frac(x, n, d) macro computes (x / d) * n + (x % d) * n / d. Since
> n is 10000, the multiplication x * 10000 is performed using 32-bit unsigned
> arithmetic on 32-bit systems (where unsigned long is 32 bits). If active
> memory exceeds 429,496 pages (~1.76 GB), the multiplication overflows
> ULONG_MAX, resulting in an incorrect ratio.
>
> Does this same overflow also affect other DAMON memory metric calculations?
> For example, in damos_get_node_eligible_mem_bp():
>
> return mult_frac((unsigned long)node_eligible, 10000,
> (unsigned long)total_eligible);
>
> This explicitly casts a 64-bit phys_addr_t to unsigned long, truncating
> capacities over 4GB on 32-bit systems. It also overflows mult_frac() if the
> eligible memory exceeds just ~419 KB.
>
> Similarly, in damon_get_intervals_score():
>
> max_access_events += sz_region * c->attrs.aggr_samples;
> access_events += sz_region * r->nr_accesses;
>
> Because sz_region is in bytes, accumulating these into an unsigned long
> seems to overflow for regions larger than a few tens of megabytes.
Known low priority issue.
>
> > return mult_frac(active, 10000, total);
> > return mult_frac(inactive, 10000, total);
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=9
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v1.1 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp()
2026-08-02 17:49 ` SJ Park
@ 2026-08-02 17:59 ` SJ Park
0 siblings, 0 replies; 23+ messages in thread
From: SJ Park @ 2026-08-02 17:59 UTC (permalink / raw)
To: SJ Park; +Cc: sashiko-bot, damon
On Sun, 2 Aug 2026 10:49:50 -0700 SJ Park <sj@kernel.org> wrote:
> On Sun, 02 Aug 2026 16:35:44 +0000 sashiko-bot@kernel.org wrote:
>
> > Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> >
> > New issues:
> > - [High] Returning 0 for DAMOS_QUOTA_NODE_MEM_FREE_BP when freeram > totalram incorrectly forces maximum scheme aggressiveness instead of reporting 100% free memory.
> > - [Medium] The commit message contradicts the implementation, and the unconditional return of 10000 for zero totalram breaks metric semantics.
> >
> > Pre-existing issues:
> > - [High] Unhandled integer underflow in MEM_USED_BP and MEMCG_FREE_BP calculations when counters drift.
> > - [High] The patch fixes a potential divide-by-zero in damos_get_node_mem_bp() but misses identical unvalidated divisions in related functions.
> > --
> >
> > --- Patch [7]: [RFC PATCH v1.1 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() ---
> > commit 05d27d4d51952669ff23ec9285ccb83a273556ad
> > Author: SJ Park <sj@kernel.org>
> >
> > 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
> >
> > > 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.
> >
> > [Severity: Medium]
> > The commit message states "Fix it by setting the totalram one byte in the
> > case."
> >
> > However, the implementation actually bypasses the metric type check and returns
> > 10000.
> >
> > Does this unconditional return contradict the intent documented in the commit
> > message?
>
> Good catch, I will update the commit message.
>
> >
> > > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > > index 3bdbf4fbf7147..e3f3ee75a3d33 100644
> > > --- a/mm/damon/core.c
> > > +++ b/mm/damon/core.c
> > > @@ -2816,10 +2816,16 @@ static __kernel_ulong_t damos_get_node_mem_bp(
> > > }
> > >
> > > si_meminfo_node(&i, goal->nid);
> > > - if (goal->metric = DAMOS_QUOTA_NODE_MEM_USED_BP)
> > > + if (!i.totalram)
> > > + return 10000;
> >
> > [Severity: Medium]
> > If the metric is DAMOS_QUOTA_NODE_MEM_FREE_BP, shouldn't it report 0% free
> > rather than 100% free if the memory is absent (which is what totalram=1,
> > freeram=0 would yield)?
> >
> > Does returning 10000 here break the semantic constraint that used + free = 100%?
>
> I made mistakes at revisioning. I will fix this.
And this semantic gap exist in the next patch, too. I will fix it together.
>
> >
> > > + if (goal->metric = DAMOS_QUOTA_NODE_MEM_USED_BP) {
> > > numerator = i.totalram - i.freeram;
> >
> > [Severity: High]
> > This is a pre-existing issue, but if counter drift results in
> > freeram > totalram, does this subtraction silently underflow, producing a
> > massive unsigned value?
> >
> > If this is passed to mult_frac(), could it generate a huge metric value,
> > leading the auto-tuner to mistakenly act as if the memory is massively
> > overused?
> >
> > An identical unhandled underflow also appears to exist in
> > damos_get_node_memcg_used_bp() when evaluating DAMOS_QUOTA_NODE_MEMCG_FREE_BP
> > if used_pages exceeds totalram:
> >
> > mm/damon/core.c:damos_get_node_memcg_used_bp() {
> > ...
> > numerator = i.totalram - used_pages;
> > ...
> > }
>
> The intention of this revision was handling this, but I made a complete
> mistake. I will fix.
>
> >
> > > - else /* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> > > + } else {
> > > + /* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> > > + if (i.totalram < i.freeram)
> > > + return 0;
> >
> > [Severity: High]
> > When counter drift causes freeram > totalram, the node effectively has
> > ~100% free memory.
> >
> > By returning 0 (0% free memory) in this scenario, doesn't the DAMON quota
> > auto-tuning feedback loop interpret this as severely under-achieving the free
> > memory goal, causing it to wastefully and drastically increase the reclamation
> > budget (e.g., pageout/demotion) on a node that is already completely free?
> >
> > Should this return 10000 (100% free) instead?
>
> That depends on the perspective. I think 0% makes sense.
>
> >
> > > numerator = i.freeram;
> > > + }
> > > return mult_frac(numerator, 10000, i.totalram);
> > > }
> >
> > [Severity: High]
> > This is a pre-existing issue, but while this patch addresses a potential
> > divide-by-zero when si_meminfo_node() reports totalram = 0, does the same
> > unvalidated division exist in related functions?
> >
> > For example, damos_get_node_memcg_used_bp() divides by i.totalram without a
> > zero-check:
> >
> > mm/damon/core.c:damos_get_node_memcg_used_bp() {
> > ...
> > si_meminfo_node(&i, goal->nid);
> > ...
> > return mult_frac(numerator, 10000, i.totalram);
> > }
> >
> > Similarly, damos_get_in_active_mem_bp() divides by total (active + inactive)
> > via mult_frac() without checking if total is zero:
> >
> > mm/damon/core.c:damos_get_in_active_mem_bp() {
> > ...
> > if (goal->metric = DAMOS_QUOTA_SOME_MEM_USED_BP)
> > return mult_frac(active, 10000, total);
> > return mult_frac(inactive, 10000, total);
> > }
>
> Later patches in this series fix those.
>
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260802162631.90304-1-sj@kernel.org?part=7
> >
>
>
> Thanks,
> SJ
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2026-08-02 17:59 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-02 16:26 [RFC PATCH v1.1 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 1/9] mm/damon/core: skip applying scheme if region split for quota fails SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total SJ Park
2026-08-02 16:43 ` sashiko-bot
2026-08-02 17:41 ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT SJ Park
2026-08-02 16:34 ` sashiko-bot
2026-08-02 17:43 ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 4/9] mm/damon/paddr: respect folio end for DAMOS actions except STAT SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT SJ Park
2026-08-02 16:36 ` sashiko-bot
2026-08-02 17:45 ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 6/9] mm/damon/vaddr: respect folio end for DAMOS_MIGRATE_{HOT,COLD} SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() SJ Park
2026-08-02 16:35 ` sashiko-bot
2026-08-02 17:49 ` SJ Park
2026-08-02 17:59 ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() SJ Park
2026-08-02 16:38 ` sashiko-bot
2026-08-02 17:51 ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() SJ Park
2026-08-02 16:51 ` sashiko-bot
2026-08-02 17:51 ` SJ Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox