Kernel initialize "jiffies" timer as 5 minutes below zero, as shown in include/linux/jiffies.h
/* * Have the 32 bit jiffies value wrap 5 minutes after boot * so jiffies wrap bugs show up earlier. */ #define INITIAL_JIFFIES ((unsigned long)(unsigned int) (-300*HZ))
And jiffies comparison help functions cast unsigned value to signed to cover wraparound
#define time_after_eq(a,b) \ (typecheck(unsigned long, a) && \ typecheck(unsigned long, b) && \ ((long)((a) - (b)) >= 0))
When quota->charged_from is initialized to 0, time_after_eq() can incorrectly return FALSE even after reset_interval has elapsed. This occurs when (jiffies - reset_interval) produces a value with MSB=1, which is interpreted as negative in signed arithmetic.
This issue primarily affects 32-bit systems because: On 64-bit systems: MSB=1 values occur after ~292 million years from boot (assuming HZ=1000), almost impossible.
On 32-bit systems: MSB=1 values occur during the first 5 minutes after boot, and the second half of every jiffies wraparound cycle, starting from day 25 (assuming HZ=1000)
When above unexpected FALSE return from time_after_eq() occurs, the charging window will not reset. The user impact depends on esz value at that time.
If esz is 0, scheme ignores configured quotas and runs without any limits.
If esz is not 0, scheme stops working once the quota is exhausted. It remains until the charging window finally resets.
So, change quota->charged_from to jiffies at damos_adjust_quota() when it is considered as the first charge window. By this change, we can avoid unexpected FALSE return from time_after_eq()
Fixes: 2b8a248d5873 ("mm/damon/schemes: implement size quota for schemes application speed control") # 5.16 Cc: stable@vger.kernel.org Signed-off-by: Sang-Heon Jeon ekffu200098@gmail.com --- Changes from v2 [2] - remove unnecessary example about time_after_eq() - remove description of unexpected reset of quota->charged_from - clarify user impacts and when bug happens
Changes from v1 [1] - not change current default value of quota->charged_from - set quota->charged_from when it is consider first charge below - add more description of jiffies and wraparound example to commit messages
[1] https://lore.kernel.org/damon/20250818183803.1450539-1-ekffu200098@gmail.com... [2] https://lore.kernel.org/damon/20250819150123.1532458-1-ekffu200098@gmail.com... --- mm/damon/core.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/mm/damon/core.c b/mm/damon/core.c index cb41fddca78c..93bad6d0da5b 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -2130,6 +2130,10 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s) if (!quota->ms && !quota->sz && list_empty("a->goals)) return;
+ /* First charge window */ + if (!quota->total_charged_sz && !quota->charged_from) + quota->charged_from = jiffies; + /* New charge window starts */ if (time_after_eq(jiffies, quota->charged_from + msecs_to_jiffies(quota->reset_interval))) {
On Fri, 22 Aug 2025 01:33:46 +0900 Sang-Heon Jeon ekffu200098@gmail.com wrote:
Kernel initialize "jiffies" timer as 5 minutes below zero, as shown in include/linux/jiffies.h
/*
- Have the 32 bit jiffies value wrap 5 minutes after boot
- so jiffies wrap bugs show up earlier.
*/ #define INITIAL_JIFFIES ((unsigned long)(unsigned int) (-300*HZ))
And jiffies comparison help functions cast unsigned value to signed to cover wraparound
#define time_after_eq(a,b) \ (typecheck(unsigned long, a) && \ typecheck(unsigned long, b) && \ ((long)((a) - (b)) >= 0))
When quota->charged_from is initialized to 0, time_after_eq() can incorrectly return FALSE even after reset_interval has elapsed. This occurs when (jiffies - reset_interval) produces a value with MSB=1, which is interpreted as negative in signed arithmetic.
This issue primarily affects 32-bit systems because: On 64-bit systems: MSB=1 values occur after ~292 million years from boot (assuming HZ=1000), almost impossible.
On 32-bit systems: MSB=1 values occur during the first 5 minutes after boot, and the second half of every jiffies wraparound cycle, starting from day 25 (assuming HZ=1000)
When above unexpected FALSE return from time_after_eq() occurs, the charging window will not reset. The user impact depends on esz value at that time.
If esz is 0, scheme ignores configured quotas and runs without any limits.
If esz is not 0, scheme stops working once the quota is exhausted. It remains until the charging window finally resets.
So, change quota->charged_from to jiffies at damos_adjust_quota() when it is considered as the first charge window. By this change, we can avoid unexpected FALSE return from time_after_eq()
Thank you for this patch, Sang-Heon! But, checkpatch.pl raises below three warnings. Could you please fix those and send yet another version?
WARNING: Commit log lines starting with '#' are dropped by git as comments #16: #define INITIAL_JIFFIES ((unsigned long)(unsigned int) (-300*HZ))
WARNING: Commit log lines starting with '#' are dropped by git as comments #21: #define time_after_eq(a,b) \
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #26: When quota->charged_from is initialized to 0, time_after_eq() can incorrectly
Thanks, SJ
[...]
On Fri, Aug 22, 2025 at 2:53 AM SeongJae Park sj@kernel.org wrote:
On Fri, 22 Aug 2025 01:33:46 +0900 Sang-Heon Jeon ekffu200098@gmail.com wrote:
Kernel initialize "jiffies" timer as 5 minutes below zero, as shown in include/linux/jiffies.h
/*
- Have the 32 bit jiffies value wrap 5 minutes after boot
- so jiffies wrap bugs show up earlier.
*/ #define INITIAL_JIFFIES ((unsigned long)(unsigned int) (-300*HZ))
And jiffies comparison help functions cast unsigned value to signed to cover wraparound
#define time_after_eq(a,b) \ (typecheck(unsigned long, a) && \ typecheck(unsigned long, b) && \ ((long)((a) - (b)) >= 0))
When quota->charged_from is initialized to 0, time_after_eq() can incorrectly return FALSE even after reset_interval has elapsed. This occurs when (jiffies - reset_interval) produces a value with MSB=1, which is interpreted as negative in signed arithmetic.
This issue primarily affects 32-bit systems because: On 64-bit systems: MSB=1 values occur after ~292 million years from boot (assuming HZ=1000), almost impossible.
On 32-bit systems: MSB=1 values occur during the first 5 minutes after boot, and the second half of every jiffies wraparound cycle, starting from day 25 (assuming HZ=1000)
When above unexpected FALSE return from time_after_eq() occurs, the charging window will not reset. The user impact depends on esz value at that time.
If esz is 0, scheme ignores configured quotas and runs without any limits.
If esz is not 0, scheme stops working once the quota is exhausted. It remains until the charging window finally resets.
So, change quota->charged_from to jiffies at damos_adjust_quota() when it is considered as the first charge window. By this change, we can avoid unexpected FALSE return from time_after_eq()
Thank you for this patch, Sang-Heon! But, checkpatch.pl raises below three warnings. Could you please fix those and send yet another version?
WARNING: Commit log lines starting with '#' are dropped by git as comments #16: #define INITIAL_JIFFIES ((unsigned long)(unsigned int) (-300*HZ)) WARNING: Commit log lines starting with '#' are dropped by git as comments #21: #define time_after_eq(a,b) \ WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #26: When quota->charged_from is initialized to 0, time_after_eq() can incorrectly
I will fix it. Also, I came up with a way to prevent these minor mistakes. Thank you so much.
Thanks, SJ
[...]
Best Regards. Sang-Heon Jeon
linux-stable-mirror@lists.linaro.org
-
Sang-Heon Jeon -
SeongJae Park