mm/damon/stat: set last_refresh_jiffies to jiffies at startup
Subsystems (2)
PATCH 1/1 mm/damon/stat: set last_refresh_jiffies to jiffies at startup
In DAMON_STAT's damon_stat_damon_call_fn(), time_before_eq() is used to avoid unnecessarily frequent stat update.
On 32-bit systems, the kernel initializes jiffies to "-5 minutes" to make jiffies wrap bugs appear earlier. However, this causes time_before_eq() in DAMON_STAT to unexpectedly return true during the first 5 minutes after boot on 32-bit systems (see [1] for more explanation, which fixes another jiffies-related issue in DAMON). As a result, DAMON_STAT does not update any monitoring results during that period, which can be more confusing when DAMON_STAT_ENABLED_DEFAULT is enabled.
Fix it by setting last_refresh_jiffies to jiffies at startup.
[1] https://lkml.kernel.org/r/[email protected]
--- mm/damon/stat.c | 3 +++ 1 file changed, 3 insertions(+)
mm/damon/stat.c +3 -0
@@ -132,6 +132,9 @@ static int damon_stat_damon_call_fn(void *data)struct damon_ctx *c = data;static unsigned long last_refresh_jiffies; +if (unlikely (!last_refresh_jiffies)) + last_refresh_jiffies = jiffies; +/* avoid unnecessarily frequent stat update */ if (time_before_eq (jiffies, last_refresh_jiffies +msecs_to_jiffies (5 *MSEC_PER_SEC )))
Signature · 1 line
2.43.0
SeongJae Park
maintainerQuoted 11 lines · On Tue, 28 Oct 2025 14:19:27 +0800 Quanmin Yan <[email protected]> wrote:
In DAMON_STAT's
damon_stat_damon_call_fn(),time_before_eq()is used to avoid unnecessarily frequent stat update.On 32-bit systems, the kernel initializes jiffies to "-5 minutes" to make jiffies wrap bugs appear earlier. However, this causes
time_before_eq()inDAMON_STATto unexpectedly return true during the first 5 minutes after boot on 32-bit systems (see [1] for more explanation, which fixes another jiffies-related issue in DAMON). As a result,DAMON_STATdoes not update any monitoring results during that period, which can be more confusing whenDAMON_STAT_ENABLED_DEFAULTis enabled.
Quoted 1 line
Fix it by setting
last_refresh_jiffiesto jiffies at startup.
Nice catch, thank you for this patch!
Quoted 14 lines
[1] https://lkml.kernel.org/r/[email protected]
Fixes fabdd1e911da ("mm/damon/stat: calculate and expose estimated memory bandwidth")Signed-off-by Quanmin Yan <[email protected]>--- mm/damon/stat.c | 3 +++ 1 file changed, 3 insertions(+)mm/damon/stat.c +0
-0
mm/damon/stat.c static int damon_stat_damon_call_fn(void *data) · line 137
Quoted 6 lines
mm/damon/stat.c +3
-0@@ -132,6 +132,9 @@ static int damon_stat_damon_call_fn(void *data)struct damon_ctx *c = data;static unsigned long last_refresh_jiffies; +if (unlikely (!last_refresh_jiffies)) + last_refresh_jiffies = jiffies; +
How about doing the initialization together with the declaration? E.g.,
static int damon_stat_damon_call_fn(void *data)
{
struct damon_ctx *c = data;- static unsigned long last_refresh_jiffies;
+ static unsigned long last_refresh_jiffies = jiffies;
Thanks,
SJ
[...]
SeongJae Park
maintainerQuoted 43 lines · On Tue, 28 Oct 2025 07:19:14 -0700 SeongJae Park <[email protected]> wrote:
Quoted 11 lines · On Tue, 28 Oct 2025 14:19:27 +0800 Quanmin Yan <[email protected]> wrote:
In DAMON_STAT's
damon_stat_damon_call_fn(),time_before_eq()is used to avoid unnecessarily frequent stat update.On 32-bit systems, the kernel initializes jiffies to "-5 minutes" to make jiffies wrap bugs appear earlier. However, this causes
time_before_eq()inDAMON_STATto unexpectedly return true during the first 5 minutes after boot on 32-bit systems (see [1] for more explanation, which fixes another jiffies-related issue in DAMON). As a result,DAMON_STATdoes not update any monitoring results during that period, which can be more confusing whenDAMON_STAT_ENABLED_DEFAULTis enabled.Quoted 1 line
Fix it by setting
last_refresh_jiffiesto jiffies at startup.Nice catch, thank you for this patch!
Quoted 14 lines
[1] https://lkml.kernel.org/r/[email protected]
Fixes fabdd1e911da ("mm/damon/stat: calculate and expose estimated memory bandwidth")Signed-off-by Quanmin Yan <[email protected]>--- mm/damon/stat.c | 3 +++ 1 file changed, 3 insertions(+)mm/damon/stat.c +0
-0mm/damon/stat.c static int damon_stat_damon_call_fn(void *data) · line 137
Quoted 6 lines
mm/damon/stat.c +3
-0@@ -132,6 +132,9 @@ static int damon_stat_damon_call_fn(void *data)struct damon_ctx *c = data;static unsigned long last_refresh_jiffies; +if (unlikely (!last_refresh_jiffies)) + last_refresh_jiffies = jiffies; +How about doing the initialization together with the declaration? E.g.,
static int
damon_stat_damon_call_fn(void *data){ struct damon_ctx *c = data;
Review
Quoted 2 lines
(quoted diff) +1
-1- static unsigned long last_refresh_jiffies; + static unsigned long last_refresh_jiffies = jiffies;
Actually, a similar issue can happen again if DAMON_STAT is stopped and restarted by user. That is, if user stops DAMON_STAT just after last_refresh_jiffies is updated, and restart it after 5 seconds or more, the time_before_eq() on damon_call_fn() will return true, so stat updates will happen earlier than expected. Shouldn't be a real problem, but better to avoid if possible.
How about making last_refresh_jiffies a global variable and initialize it on damon_stat_start()? To avoid unnecessary name conflicts, the variable name would also better to be changed, e.g., damon_stat_last_refresh_jiffies.
Thanks,
SJ
[...]
SeongJae Park
maintainerQuoted 42 lines · On Tue, 28 Oct 2025 07:32:49 -0700 SeongJae Park <[email protected]> wrote:
Quoted 40 lines · On Tue, 28 Oct 2025 07:19:14 -0700 SeongJae Park <[email protected]> wrote:
Quoted 11 lines · On Tue, 28 Oct 2025 14:19:27 +0800 Quanmin Yan <[email protected]> wrote:
In DAMON_STAT's
damon_stat_damon_call_fn(),time_before_eq()is used to avoid unnecessarily frequent stat update.On 32-bit systems, the kernel initializes jiffies to "-5 minutes" to make jiffies wrap bugs appear earlier. However, this causes
time_before_eq()inDAMON_STATto unexpectedly return true during the first 5 minutes after boot on 32-bit systems (see [1] for more explanation, which fixes another jiffies-related issue in DAMON). As a result,DAMON_STATdoes not update any monitoring results during that period, which can be more confusing whenDAMON_STAT_ENABLED_DEFAULTis enabled.Quoted 1 line
Fix it by setting
last_refresh_jiffiesto jiffies at startup.Nice catch, thank you for this patch!
Quoted 14 lines
[1] https://lkml.kernel.org/r/[email protected]
Fixes fabdd1e911da ("mm/damon/stat: calculate and expose estimated memory bandwidth")Signed-off-by Quanmin Yan <[email protected]>--- mm/damon/stat.c | 3 +++ 1 file changed, 3 insertions(+)mm/damon/stat.c +0
-0mm/damon/stat.c static int damon_stat_damon_call_fn(void *data) · line 137
Quoted 6 lines
mm/damon/stat.c +3
-0@@ -132,6 +132,9 @@ static int damon_stat_damon_call_fn(void *data)struct damon_ctx *c = data;static unsigned long last_refresh_jiffies; +if (unlikely (!last_refresh_jiffies)) + last_refresh_jiffies = jiffies; +How about doing the initialization together with the declaration? E.g.,
Quoted 5 lines
Review
Quoted 5 lines
static int
damon_stat_damon_call_fn(void *data){ struct damon_ctx *c = data;(quoted diff) +1
-1- static unsigned long last_refresh_jiffies; + static unsigned long last_refresh_jiffies = jiffies;
Please ignore the above suggestion. It will even not build, like below...
.../mm/damon/stat.c: In function ‘damon_stat_damon_call_fn’:.../mm/damon/stat.c:133:53: error: initializer element is not constant
133 | static unsigned long last_refresh_jiffies = jiffies;
| ^~~~~~~
Quoted 11 lines
Actually, a similar issue can happen again if
DAMON_STATis stopped and restarted by user. That is, if user stopsDAMON_STATjust afterlast_refresh_jiffiesis updated, and restart it after 5 seconds or more, thetime_before_eq()ondamon_call_fn()will return true, so stat updates will happen earlier than expected. Shouldn't be a real problem, but better to avoid if possible.How about making
last_refresh_jiffiesa global variable and initialize it ondamon_stat_start()? To avoid unnecessary name conflicts, the variable name would also better to be changed, e.g.,damon_stat_last_refresh_jiffies.
But, please consider the above one.
And I just realized a similar issue exist for next_update_jiffies in mm/damon/sysfs.c file. Please feel free to send a patch for that if you willing to.
Thanks,
SJ
[...]
Quoted 60 lines · SeongJae Park
Quoted 39 lines · On Tue, 28 Oct 2025 07:32:49 -0700 SeongJae Park <[email protected]> wrote:
Quoted 37 lines · On Tue, 28 Oct 2025 07:19:14 -0700 SeongJae Park <[email protected]> wrote:
Quoted 11 lines · On Tue, 28 Oct 2025 14:19:27 +0800 Quanmin Yan <[email protected]> wrote:
In DAMON_STAT's
damon_stat_damon_call_fn(),time_before_eq()is used to avoid unnecessarily frequent stat update.On 32-bit systems, the kernel initializes jiffies to "-5 minutes" to make jiffies wrap bugs appear earlier. However, this causes
time_before_eq()inDAMON_STATto unexpectedly return true during the first 5 minutes after boot on 32-bit systems (see [1] for more explanation, which fixes another jiffies-related issue in DAMON). As a result,DAMON_STATdoes not update any monitoring results during that period, which can be more confusing whenDAMON_STAT_ENABLED_DEFAULTis enabled.Quoted 1 line
Fix it by setting
last_refresh_jiffiesto jiffies at startup.Nice catch, thank you for this patch!
Quoted 13 lines
[1] https://lkml.kernel.org/r/[email protected]
Fixes fabdd1e911da ("mm/damon/stat: calculate and expose estimated memory bandwidth")Signed-off-by Quanmin Yan <[email protected]>--- mm/damon/stat.c | 3 +++ 1 file changed, 3 insertions(+)mm/damon/stat.c +0
-0mm/damon/stat.c static int damon_stat_damon_call_fn(void *data) · line 137
Quoted 6 lines
mm/damon/stat.c +3
-0@@ -132,6 +132,9 @@ static int damon_stat_damon_call_fn(void *data)struct damon_ctx *c = data;static unsigned long last_refresh_jiffies; +if (unlikely (!last_refresh_jiffies)) + last_refresh_jiffies = jiffies; +How about doing the initialization together with the declaration? E.g.,
Quoted 5 lines
Review
Quoted 5 lines
static int damon_stat_damon_call_fn(void *data) { struct damon_ctx *c = data;(quoted diff) +1
-1- static unsigned long last_refresh_jiffies; + static unsigned long last_refresh_jiffies = jiffies;Please ignore the above suggestion. It will even not build, like below...
.../mm/damon/stat.c: In function ‘damon_stat_damon_call_fn’:.../mm/damon/stat.c:133:53: error: initializer element is not constant
133 | static unsigned longlast_refresh_jiffies= jiffies;
| ^~~~~~~Quoted 7 lines
Actually, a similar issue can happen again if
DAMON_STATis stopped and restarted by user. That is, if user stopsDAMON_STATjust afterlast_refresh_jiffiesis updated, and restart it after 5 seconds or more, thetime_before_eq()ondamon_call_fn()will return true, so stat updates will happen earlier than expected. Shouldn't be a real problem, but better to avoid if possible.
Quoted 9 lines
Quoted 3 lines
How about making
last_refresh_jiffiesa global variable and initialize it ondamon_stat_start()? To avoid unnecessary name conflicts, the variable name would also better to be changed, e.g.,damon_stat_last_refresh_jiffies.But, please consider the above one.
And I just realized a similar issue exist for
next_update_jiffiesinmm/damon/sysfs.cfile. Please feel free to send a patch for that if you willing to.
OK, I’ll review all these issues and send a new patch set once everything is ready.🙂
Thanks,
Quanmin Yan
[...]
Quoted 41 lines · SeongJae Park
Quoted 11 lines · On Tue, 28 Oct 2025 14:19:27 +0800 Quanmin Yan <[email protected]> wrote:
In DAMON_STAT's
damon_stat_damon_call_fn(),time_before_eq()is used to avoid unnecessarily frequent stat update.On 32-bit systems, the kernel initializes jiffies to "-5 minutes" to make jiffies wrap bugs appear earlier. However, this causes
time_before_eq()inDAMON_STATto unexpectedly return true during the first 5 minutes after boot on 32-bit systems (see [1] for more explanation, which fixes another jiffies-related issue in DAMON). As a result,DAMON_STATdoes not update any monitoring results during that period, which can be more confusing whenDAMON_STAT_ENABLED_DEFAULTis enabled.Quoted 1 line
Fix it by setting
last_refresh_jiffiesto jiffies at startup.Nice catch, thank you for this patch!
Quoted 13 lines
[1] https://lkml.kernel.org/r/[email protected]
Fixes fabdd1e911da ("mm/damon/stat: calculate and expose estimated memory bandwidth")Signed-off-by Quanmin Yan <[email protected]>--- mm/damon/stat.c | 3 +++ 1 file changed, 3 insertions(+)mm/damon/stat.c +0
-0mm/damon/stat.c static int damon_stat_damon_call_fn(void *data) · line 137
Quoted 6 lines
mm/damon/stat.c +3
-0@@ -132,6 +132,9 @@ static int damon_stat_damon_call_fn(void *data)struct damon_ctx *c = data;static unsigned long last_refresh_jiffies; +if (unlikely (!last_refresh_jiffies)) + last_refresh_jiffies = jiffies; +How about doing the initialization together with the declaration? E.g.,
static int damon_stat_damon_call_fn(void *data) { struct damon_ctx *c = data;
Review
Quoted 2 lines
(quoted diff) +1
-1- static unsigned long last_refresh_jiffies; + static unsigned long last_refresh_jiffies = jiffies;
Thank you for your suggestion. Well, I actually tried that before, but found that jiffies is not a compile-time constant,|it| is set at runtime, while static initializers must use constant expressions, that's why the current patch is modified this way.
Thanks,
Quanmin Yan
[...]