mm/damon/stat: monitor all System RAM resources
Subsystems (3)
SeongJae Park
maintainerPATCH 1/1 mm/damon/stat: monitor all System RAM resources
DAMON_STAT usage document (Documentation/admin-guide/mm/damon/stat.rst) says it monitors the system's entire physical memory. But, it is monitoring only the biggest System RAM resource of the system. When there are multiple System RAM resources, this results in monitoring only an unexpectedly small fraction of the physical memory. For example, suppose the system has a 500 GiB System RAM, 10 MiB non-System RAM, and 500 GiB System RAM resources in order on the physical address space. DAMON_STAT will monitor only the first 500 GiB System RAM. This situation is particularly common on NUMA systems.
Select a physical address range that covers all System RAM areas of the system, to fix this issue and make it work as documented.
--- mm/damon/stat.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-)
mm/damon/stat.c +48 -3
@@ -145,12 +145,57 @@ static int damon_stat_damon_call_fn(void *data)return 0 ; } +struct damon_stat_system_ram_range_walk_arg { +bool walked; +struct resource res; +}; + +static int damon_stat_system_ram_walk_fn (struct resource *res,void *arg) +{ +struct damon_stat_system_ram_range_walk_arg *a = arg; + +if (!a->walked) { + a->walked = true; + a->res.start = res->start; + } + a->res.end = res->end; +return 0 ; +} + +static unsigned long damon_stat_res_to_core_addr (resource_size_t ra, +unsigned long addr_unit) +{ +/* +* Use div_u64() for avoiding linking errors related with __udivdi3, +* __aeabi_uldivmod, or similar problems. This should also improve the +* performance optimization (read div_u64() comment for the detail). +*/ +if (sizeof (ra) ==8 &&sizeof (addr_unit) ==4 ) +return div_u64 (ra, addr_unit); +return ra / addr_unit; +} + +static int damon_stat_set_monitoring_region (struct damon_target *t, +unsigned long addr_unit,unsigned long min_region_sz) +{ +struct damon_addr_range addr_range; +struct damon_stat_system_ram_range_walk_arg arg = {}; + +walk_system_ram_res (0 ,-1 , &arg, damon_stat_system_ram_walk_fn); +if (!arg.walked) +return -EINVAL ; + addr_range.start =damon_stat_res_to_core_addr ( + arg.res.start, addr_unit); + addr_range.end =damon_stat_res_to_core_addr ( + arg.res.end +1 , addr_unit); +return damon_set_regions (t, &addr_range, addr_unit, min_region_sz); +} +static struct damon_ctx *damon_stat_build_ctx (void ) {struct damon_ctx *ctx;struct damon_attrs attrs;struct damon_target *target; -unsigned long start =0 , end =0 ; ctx =damon_new_ctx ();if (!ctx)
@@ -180,8 +225,8 @@ static struct damon_ctx *damon_stat_build_ctx(void)if (!target) goto free_out;damon_add_target (ctx, target); -if (damon_set_region_biggest_system_ram_default (target, &start, &end, - ctx->addr_unit, ctx->min_region_sz)) +if (damon_stat_set_monitoring_region (target, ctx->addr_unit, + ctx->min_region_sz)) goto free_out;return ctx; free_out:
base-commit: ef772f765091af645985f2bd70390143a9820970
Signature · 1 line
2.47.3
SeongJae Park
maintainerQuoted 62 lines · On Thu, 12 Mar 2026 21:44:47 -0700 SeongJae Park <[email protected]> wrote:
DAMON_STATusage document (Documentation/admin-guide/mm/damon/stat.rst) says it monitors the system's entire physical memory. But, it is monitoring only the biggest System RAM resource of the system. When there are multiple System RAM resources, this results in monitoring only an unexpectedly small fraction of the physical memory. For example, suppose the system has a 500 GiB System RAM, 10 MiB non-System RAM, and 500 GiB System RAM resources in order on the physical address space.DAMON_STATwill monitor only the first 500 GiB System RAM. This situation is particularly common on NUMA systems.Select a physical address range that covers all System RAM areas of the system, to fix this issue and make it work as documented.
Fixes 369c415e6073 ("mm/damon: introduce DAMON_STAT module")Cc <[email protected]> # 6.17.xSigned-off-by SeongJae Park <[email protected]>--- mm/damon/stat.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-)mm/damon/stat.c +34
-0@@ -145,12 +145,57 @@ static int damon_stat_damon_call_fn(void *data)return 0 ; } +struct damon_stat_system_ram_range_walk_arg { +bool walked; +struct resource res; +}; + +static int damon_stat_system_ram_walk_fn (struct resource *res,void *arg) +{ +struct damon_stat_system_ram_range_walk_arg *a = arg; + +if (!a->walked) { + a->walked = true; + a->res.start = res->start; + } + a->res.end = res->end; +return 0 ; +} + +static unsigned long damon_stat_res_to_core_addr (resource_size_t ra, +unsigned long addr_unit) +{ +/* +* Use div_u64() for avoiding linking errors related with __udivdi3, +* __aeabi_uldivmod, or similar problems. This should also improve the +* performance optimization (read div_u64() comment for the detail). +*/ +if (sizeof (ra) ==8 &&sizeof (addr_unit) ==4 ) +return div_u64 (ra, addr_unit); +return ra / addr_unit; +} + +static int damon_stat_set_monitoring_region (struct damon_target *t, +unsigned long addr_unit,unsigned long min_region_sz) +{ +struct damon_addr_range addr_range;
mm/damon/stat.c static int damon_stat_damon_call_fn(void *data) · line 191
Quoted 10 lines
mm/damon/stat.c +10
-0+struct damon_stat_system_ram_range_walk_arg arg = {}; + +walk_system_ram_res (0 ,-1 , &arg, damon_stat_system_ram_walk_fn); +if (!arg.walked) +return -EINVAL ; + addr_range.start =damon_stat_res_to_core_addr ( + arg.res.start, addr_unit); + addr_range.end =damon_stat_res_to_core_addr ( + arg.res.end +1 , addr_unit); +return damon_set_regions (t, &addr_range, addr_unit, min_region_sz);
The third argument of damon_set_regions() is the number of ranges (length of the array passed by the second argument), so '1' should be passed. But the patch is passing 'addr_unit'. It will not cause a real issue since the value is set to '1' in this case. But it is an obvious bug. I will fix this in the v2.
FYI, I found this issue from an AI code review result [1] that published on the internet. The review also added two more comments, but those seem irrelevant to me, so ignoring. I don't know who made the web site. I only got the url from a social.kernel.org post [2]. Whoever made the web site, thanks for the review.
[1] https://sashiko.dev/#/patchset/20260313044449.4038-1-sj%40kernel.org
[2] https://social.kernel.org/notice/B4E8DdeZY07PseemfI
Thanks,
SJ
[...]
SeongJae Park
maintainerQuoted 72 lines · On Fri, 13 Mar 2026 16:40:22 -0700 SeongJae Park <[email protected]> wrote:
Quoted 70 lines · On Thu, 12 Mar 2026 21:44:47 -0700 SeongJae Park <[email protected]> wrote:
DAMON_STATusage document (Documentation/admin-guide/mm/damon/stat.rst) says it monitors the system's entire physical memory. But, it is monitoring only the biggest System RAM resource of the system. When there are multiple System RAM resources, this results in monitoring only an unexpectedly small fraction of the physical memory. For example, suppose the system has a 500 GiB System RAM, 10 MiB non-System RAM, and 500 GiB System RAM resources in order on the physical address space.DAMON_STATwill monitor only the first 500 GiB System RAM. This situation is particularly common on NUMA systems.Select a physical address range that covers all System RAM areas of the system, to fix this issue and make it work as documented.
Fixes 369c415e6073 ("mm/damon: introduce DAMON_STAT module")Cc <[email protected]> # 6.17.xSigned-off-by SeongJae Park <[email protected]>--- mm/damon/stat.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-)mm/damon/stat.c +42
-0@@ -145,12 +145,57 @@ static int damon_stat_damon_call_fn(void *data)return 0 ; } +struct damon_stat_system_ram_range_walk_arg { +bool walked; +struct resource res; +}; + +static int damon_stat_system_ram_walk_fn (struct resource *res,void *arg) +{ +struct damon_stat_system_ram_range_walk_arg *a = arg; + +if (!a->walked) { + a->walked = true; + a->res.start = res->start; + } + a->res.end = res->end; +return 0 ; +} + +static unsigned long damon_stat_res_to_core_addr (resource_size_t ra, +unsigned long addr_unit) +{ +/* +* Use div_u64() for avoiding linking errors related with __udivdi3, +* __aeabi_uldivmod, or similar problems. This should also improve the +* performance optimization (read div_u64() comment for the detail). +*/ +if (sizeof (ra) ==8 &&sizeof (addr_unit) ==4 ) +return div_u64 (ra, addr_unit); +return ra / addr_unit; +} + +static int damon_stat_set_monitoring_region (struct damon_target *t, +unsigned long addr_unit,unsigned long min_region_sz) +{ +struct damon_addr_range addr_range; +struct damon_stat_system_ram_range_walk_arg arg = {}; + +walk_system_ram_res (0 ,-1 , &arg, damon_stat_system_ram_walk_fn); +if (!arg.walked) +return -EINVAL ; + addr_range.start =damon_stat_res_to_core_addr ( + arg.res.start, addr_unit); + addr_range.end = damon_stat_res_to_core_addr(
Quoted 8 lines
mm/damon/stat.c static int damon_stat_damon_call_fn(void *data) · line 191
Quoted 2 lines
mm/damon/stat.c +2
-0+ arg.res.end + 1, addr_unit); +return damon_set_regions (t, &addr_range, addr_unit, min_region_sz);The third argument of
damon_set_regions()is the number of ranges (length of the array passed by the second argument), so '1' should be passed. But the patch is passing 'addr_unit'. It will not cause a real issue since the value is set to '1' in this case. But it is an obvious bug. I will fix this in the v2.
Or, Andrew, if you prefer to, please pick below attaching fixup together. I will post v2 on Sunday if this is not picked up with the below fixup by tomorrow.
Thanks,
SJ
[...]
=== >8 ===
From e5f029441de038289320a7cb99a249800d99471f Mon Sep 17 00:00:00 2001
From: SeongJae Park <[email protected]>
Date: Fri, 13 Mar 2026 16:41:00 -0700
Subject: [PATCH] mm/damon/stat: fix wrong argument to damon_set_regions()
The third argument for damon_set_regions() is the number of ranges in the second argument, but the code is wrongly passing addr_unit. Fix it.
--- mm/damon/stat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/damon/stat.c +1 -1
@@ -188,7 +188,7 @@ static int damon_stat_set_monitoring_region(struct damon_target *t, arg.res.start, addr_unit); addr_range.end =damon_stat_res_to_core_addr ( arg.res.end +1 , addr_unit); -return damon_set_regions (t, &addr_range, addr_unit, min_region_sz); +return damon_set_regions (t, &addr_range,1 , min_region_sz); }static struct damon_ctx *damon_stat_build_ctx (void )
Signature · 1 line
2.47.3
Andrew Morton
maintainerQuoted 4 lines · On Fri, 13 Mar 2026 16:40:22 -0700 SeongJae Park <[email protected]> wrote:
FYI, I found this issue from an AI code review result [1] that published on the internet. The review also added two more comments, but those seem irrelevant to me, so ignoring. I don't know who made the web site. I only got the url from a social.kernel.org post [2]. Whoever made the web site,
Sashiko is being led by Roman at Google.
Quoted 1 line
thanks for the review.
yop, it's good.
SeongJae Park
maintainerQuoted 8 lines · On Sat, 14 Mar 2026 15:51:09 -0700 Andrew Morton <[email protected]>…
Quoted 4 lines · On Fri, 13 Mar 2026 16:40:22 -0700 SeongJae Park <[email protected]> wrote:
FYI, I found this issue from an AI code review result [1] that published on the internet. The review also added two more comments, but those seem irrelevant to me, so ignoring. I don't know who made the web site. I only got the url from a social.kernel.org post [2]. Whoever made the web site,
Sashiko is being led by Roman at Google.
Thank you for letting me know this, Anderw!
Quoted 4 lines
Quoted 2 lines
thanks for the review.
yop, it's good.
I'm now a big fan of it. It is really good, except the lack of the dark mode ;) I therefore added features [1] for showing the AI reviews on the terminal to my mailing tool (hkml).
Anyway, this is really awesome and helpful. Thank you for developing this, Roman and the team!
Quoted 4 lines
[1] https://github.com/sjp38/hackermail/blob/master/USAGE.md#reading-sashikodev-ai-review
Thanks,
SJ
Quoted 16 lines · SeongJae Park
Quoted 8 lines · On Sat, 14 Mar 2026 15:51:09 -0700 Andrew Morton <[email protected]>…
Quoted 4 lines · On Fri, 13 Mar 2026 16:40:22 -0700 SeongJae Park <[email protected]> wrote:
FYI, I found this issue from an AI code review result [1] that published on the internet. The review also added two more comments, but those seem irrelevant to me, so ignoring. I don't know who made the web site. I only got the url from a social.kernel.org post [2]. Whoever made the web site,
Sashiko is being led by Roman at Google.
Thank you for letting me know this, Anderw!
Quoted 3 lines
Quoted 2 lines
thanks for the review.
Quoted 8 lines
Quoted 1 line
yop, it's good.
I'm now a big fan of it. It is really good, except the lack of the dark mode ;) I therefore added features [1] for showing the AI reviews on the terminal to my mailing tool (hkml).
Anyway, this is really awesome and helpful. Thank you for developing this, Roman and the team!
Thanks, SJ and Andrew for kind words!
We just launched it late on Thursday and my plan was to keep it under the radar for a week or two to make sure everything works stable before announcing it publicly... But it feels like it's too late :) I hope we can keep it running well, but, please, don't expect it to be fully reliable from the first day :)
Good point re the dark mode - I'll add it. And Sashiko itself is 100% open-source and available here:
https://github.com/sashiko-dev/sashiko/
PR's are really welcome!
Thanks,
Roman