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

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

On Thu, Aug 21, 2025 at 2:41 PM SeongJae Park <sj@kernel.org> wrote:
>
> On Thu, 21 Aug 2025 13:29:04 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
>
> > On Thu, Aug 21, 2025 at 11:54 AM SeongJae Park <sj@kernel.org> wrote:
> > >
> > > On Thu, 21 Aug 2025 10:08:03 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> > >
> > > > On Thu, Aug 21, 2025 at 3:27 AM SeongJae Park <sj@kernel.org> wrote:
> > > > >
> > > > > On Wed, 20 Aug 2025 22:18:53 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> > > > >
> > > > > > Hello, SeongJae
> > > > > >
> > > > > > On Wed, Aug 20, 2025 at 2:27 AM SeongJae Park <sj@kernel.org> wrote:
> > > > > > >
> > > > > > > On Wed, 20 Aug 2025 00:01:23 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> > > [...]
> > > > I think that I checked about user impact already but it should be
> > > > insufficient. As you said, I should discuss it first. Anyway, the
> > > > whole thing is my mistake. I'm really so sorry.
> > >
> > > Everyone makes mistakes.  You don't need to apologize.
> > >
> > > >
> > > > So, Would it be better to send an RFC patch even now, instead of
> > > > asking on this email thread? (I'll make next v3 patch with RFC tag,
> > > > it's not question of v3 direction and just about remained question on
> > > > this email thread)
> > >
> > > If you unsure something and there is no reason to send a patch without a
> > > discussion for the point, please discuss first.  To be honest I don't
> > > understand the above question at all.
> >
> > Ah, I just mean that I need to make a new RFC patch instead of
> > replying to this email thread.
>
> Why?
>
> > I'll just keep asking about previous
> > comments on this email thread.
>
> You said you will send a new patch instead of replying here, and then now you
> are saying you will keep aking to this thread.
>
> I'm confused.

I think I miscommunicated something. I'll just follow your suggestion
below. it's the same as my first thought (discussion here, new patch
after discussion finished)

> [...]
>
> Ok, I think this discussion is going unnecessarily deep and only wasting our
> time at clarifying intention of past sentence.  Meanwhile apparently you
> understood my major points.  To repeat, please clarify what is the problem and
> user impacts, when and how it happens, and why the solution solves it.
>
> 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.

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()


>
> Thanks,
> SJ

Best Regards
Sang-Heon Jeon