All of lore.kernel.org
 help / color / mirror / Atom feed
From: SJ Park <sj@kernel.org>
To: Karl Mehltretter <kmehltretter@gmail.com>
Cc: SJ Park <sj@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Lian Wang <lianux.mm@gmail.com>,
	Kunwu Chan <kunwu.chan@gmail.com>,
	damon@lists.linux.dev, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] mm/damon/core: score an unmeasured PSI goal as not achieved for the temporal tuner
Date: Sat, 19 Sep 2026 08:53:41 -0700	[thread overview]
Message-ID: <20260919155342.85699-1-sj@kernel.org> (raw)
In-Reply-To: <aq4g5W58PgWLqJ8p@gmail.com>

On Sat, 19 Sep 2026 07:44:11 +0200 Karl Mehltretter <kmehltretter@gmail.com> wrote:

> On Tue, Sep 15, 2026 at 05:13:11PM +0100, SJ Park wrote:
> > 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?
> > 
> 
> Thanks for explaining the two concerns. I am still new to the DAMON
> code, so I may be missing something, but your suggestion looks good
> to me and it is simpler than what I had. I have prepared a v2 this
> way and will wait for your comments before I send it.

Thank you for accepting my humble suggestion, please feel free to send it when
you are ready!

> 
> One note on the first concern: as far as I can see last_psi_total is
> updated in every tuning round, so commits in between would not make
> the measured time longer than the reset interval. But I am fine with
> your approach either way.

last_psi_total is updated in every tuning round, as you mentioned.  But the
tuning round comes if both the scheme apply interval and the quota reset
interval are passed.  damos_adjust_quota() has the quota reset interval check.
kdamond_apply_schemes() has the apply interval check before calling
damos_adjust_quota().

'''
static void kdamond_apply_schemes(struct damon_ctx *c)
{
[...]
        damon_for_each_scheme(s, c) {
                if (time_before(c->passed_sample_intervals, s->next_apply_sis))
                        continue;
[...]
                damos_adjust_quota(c, s);
'''

'''
static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
{
[...]
        /* New charge window starts */
        if (!time_in_range_open(jiffies, quota->charged_from,
                                quota->charged_from +
                                msecs_to_jiffies(quota->reset_interval))) {
[...]
                damos_set_effective_quota(c, s);
'''

And the apply interval (or, next_apply_sis) is extended for each commit, in
damon_set_attrs().  Hence, the problem can still happen.

> 
> I would also move the last_psi_total update into the new helper, so
> that the helper is the only place that touches the field and the
> unit test can check it.

Sounds good!

> 
> While testing this with damo I found a second problem. A full
> "damo tune" still gave a zero quota for one window, on the unpatched
> kernel too. damon_new_scheme() runs damos_quota_init() on the quota
> of the caller, and since b90408ef1163 damon_commit_ctx() passes the
> quota of each running scheme to it for the test context. So every
> full commit zeroes esz, esz_bp and the charge state of the running
> schemes. Please correct me if I read this wrong. I can send a
> separate patch for that.

I don't really understand the problem.  Could you please further elaborate?

> 
> In a quick QEMU test with your suggestion and that change, the quota
> stays at 1 MiB over "damo tune" and "damo tune --quota_goals_only".

Sounds good.  Looking forward to your next patch!


Thanks,
SJ 

[...]


  reply	other threads:[~2026-09-19 15:53 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-15  6:09 [PATCH 0/2] mm/damon: fix zero quota for PSI goals under the temporal tuner Karl Mehltretter
2026-09-15  6:09 ` [PATCH 1/2] mm/damon/core: score an unmeasured PSI goal as not achieved for " Karl Mehltretter
2026-09-15  6:18   ` sashiko-bot
2026-09-15 14:50   ` SJ Park
2026-09-15 21:19     ` Karl Mehltretter
2026-09-16  0:13       ` SJ Park
2026-09-19  5:44         ` Karl Mehltretter
2026-09-19 15:53           ` SJ Park [this message]
2026-09-21 18:51             ` Karl Mehltretter
2026-09-15  6:09 ` [PATCH 2/2] mm/damon/tests/core-kunit: test PSI goal rounds under " Karl Mehltretter
2026-09-15  6:15   ` sashiko-bot
2026-09-15  7:04   ` KunWu Chan
2026-09-15 21:31     ` Karl Mehltretter
2026-09-16  0:21   ` SJ Park
2026-09-19  5:47     ` Karl Mehltretter
2026-09-19 15:55       ` SJ Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260919155342.85699-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=damon@lists.linux.dev \
    --cc=kmehltretter@gmail.com \
    --cc=kunwu.chan@gmail.com \
    --cc=lianux.mm@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.