Re: [PATCH] mm/damon/core: set initial quota->charged_from to INITIAL_JIFFIES

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

Hello, SeongJae

On Tue, Aug 19, 2025 at 8:01 AM SeongJae Park <sj@kernel.org> wrote:
>
> Hello Sang-Heon,
>
> On Tue, 19 Aug 2025 03:38:03 +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))
> >
> > In 32bit system, if quota->charged_from is initialized to 0 as it now,
> > it will not adjust event if reset_interval_ms passes for the first 5
> > minutes.
>
> jiffies is unsigned.  Hence the initial invocation of time_after_eq() in
> damos_adjust_quota(), which is the only place reading quota->charged_from, will
> return 'true' and the quota adjustment would be done without delay, unless the
> scheme apply interval is unrealistically big.

Yeah, jiffies is unsigned but to cover wraparound in 32bit,
`time_after_eq` casts intermediate value to signed value.

#define time_after_eq(a,b)    \
    (typecheck(unsigned long, a) && \
     typecheck(unsigned long, b) && \
     ((long)((a) - (b)) >= 0))

Here is an example with INITIAL_JIFFIES(HZ is assumed 1000); a =
INITIAL_JIFFIES = 0xFFFB6C20, b = charged_from + interval = 0 + 1000
(just assumed value) = 0x000003E8 and a-b = 0xFFFB6C20 - 0x00003E8 =
0xFFFB6838; (long)((a) - (b)) is interpreted negative value in 32 bit
system.

> In 32bit system, if quota->charged_from is initialized to 0 as it now,
> it will not adjust event if reset_interval_ms passes for the first 5
> minutes.

Also, you can easily reproduce the above situation on a 32bit system.
If my explanation is insufficient, please let me know. I'll try to
find another explanation.

> Please let me know if I'm missing something.
>
>
> Thanks,
> SJ
>
> [...]