Re: [PATCH v3] mm/damon/core: set quota->charged_from to jiffies at first charge window

[Sang-Heon Jeon <ekffu200098@gmail.com>]

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