Re: [PATCH 1/2] mm/damon/core: score an unmeasured PSI goal as not achieved for the temporal tuner

[SJ Park <sj@kernel.org>]

On Tue, 15 Sep 2026 23:19:27 +0200 Karl Mehltretter <kmehltretter@gmail.com> wrote:

> On Tue, Sep 15, 2026 at 07:50:54AM +0100, SJ Park wrote:
> > 
> > The intention is to make no effect this round.  Setting current_value to zero
> > to temporal tuner means it will now have highest quota it could have.  If the
> > effective quota before this was zero, it gets an effect.
> > 
> > Ideally, we should somehow remember what was the last esz and keep it.  I have
> > no good idea for doing that with minimum change.  This is a corner case in my
> > opinion (correct me if I'm wrong) so I want to keep the change as simple as
> > possible.
> > 
> > If there is not easy way to do that, I think just keeping the behavior but
> > making it explicitly explained might be better.
> > 
> 
> Hello SJ,
> 
> I believe I have a version that addresses your concern without adding
> too much complexity.

Thank you for keep pursuing on improving DAMON, Karl!

> 
> In short, it leaves the tuners as they are and instead keeps
> last_psi_total across a commit of an existing PSI goal, as the code
> did before a68878f83ae6. The U64_MAX round then happens only once per
> new goal.

I actually considered this option when working on commit a68878f83ae6.  I had
two following concerns though.  First, if the user commits multiple times
before the next quota reset interval, the next tuning round will work with
pressure times that cumulated for longer than the quota reset interval.  This
is a quite rare corner case, but I didn't feel that comfortable.  Second, the
behavior for new commit and update commit is different.

So I'd still prefer to just making no effect this round.  And my commit was
failed at doing that, because it didn't aware of temporal tuner.  Maybe we
could show whether the goal was achieved or not, using esz and let the tuner
show same achieveness?  What about something like below?

'''
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3187,6 +3187,25 @@ static inline u64 damos_get_some_mem_psi_total(void)

 #endif /* CONFIG_PSI */

+static void damos_set_psi_current_val(u64 now_psi_total, struct
+               damos_quota_goal *goal, struct damos *s)
+{
+       if (goal->last_psi_total != U64_MAX) {
+               goal->current_value = now_psi_total - goal->last_psi_total;
+               return;
+       }
+       /* Uninitialized last_psi_total; make no effect this round */
+       if (s->quota.goal_tuner == DAMOS_QUOTA_GOAL_TUNER_CONSIST) {
+               goal->current_value = goal->target_value;
+               return;
+       }
+       /* Let temporal tuner show goal achieveness same to the last round */
+       if (!s->quota.esz)
+               goal->current_value = goal->target_value;
+       else
+               goal->current_value = 0;
+}
+
 #ifdef CONFIG_NUMA
 static bool invalid_mem_node(int nid)
 {
@@ -3439,12 +3458,7 @@ static void damos_set_quota_goal_current_value(struct damon_ctx *c,
                break;
        case DAMOS_QUOTA_SOME_MEM_PSI_US:
                now_psi_total = damos_get_some_mem_psi_total();
-               /* uninitialized last_psi_total; make no effect this round */
-               if (goal->last_psi_total == U64_MAX)
-                       goal->current_value = goal->target_value;
-               else
-                       goal->current_value = now_psi_total -
-                               goal->last_psi_total;
+               damos_set_psi_current_val(now_psi_total, goal, s);
                goal->last_psi_total = now_psi_total;
                break;
        case DAMOS_QUOTA_NODE_MEM_USED_BP:
'''

The code could further cleaned and optimized, but hopefully that will give you
my rough idea.

What do you think?


Thanks,
SJ

[...]