Re: [PATCH v2] 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 12:58 AM SeongJae Park <sj@kernel.org> wrote:
>
> On Thu, 21 Aug 2025 20:06:58 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
>
> > On Thu, Aug 21, 2025 at 2:41 PM SeongJae Park <sj@kernel.org> wrote:
> [...]
> > > Let's restart.  Could you please rewrite the commit log for this patch and send
> > > the draft as a reply to this?
> > >
> > > We can further discuss on the new draft if it has more things to improve.  And
> > > once the discussion is finalized, you can post v4 of this patch with the
> > > updated commit message.
> >
> > Good Idea. This is the draft for commit message. Also, Thank you for
> > your patience and understanding.
>
> Thank you for accepting my humble suggestion.
>
> >
> > 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()
>
> This new draft looks good to me.  I find nothing to further modify.  Could you
> please send v3 of this patch with the above commit log?

I'm really glad to hear that! Your thoughtful comments really brought
this together.

>
> Thanks,
> SJ
>
> [...]

Best Regards
Sang-Heon Jeon