From: SJ Park <sj@kernel.org>
To: Liew Rui Yan <aethernet65535@gmail.com>
Cc: SJ Park <sj@kernel.org>,
akpm@linux-foundation.org, damon@lists.linux.dev,
linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [RFC PATCH] mm/damon: fix damos quota walk-position tracking
Date: Thu, 27 Aug 2026 19:31:11 -0700 [thread overview]
Message-ID: <20260828023112.122652-1-sj@kernel.org> (raw)
In-Reply-To: <20260828015429.131338-1-aethernet65535@gmail.com>
On Fri, 28 Aug 2026 09:54:29 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> On Thu, 27 Aug 2026 17:40:48 -0700 SJ Park <sj@kernel.org> wrote:
>
> > On Fri, 28 Aug 2026 02:08:22 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
[...]
> > > While your approach works, I am curious, why should the cursor be reset
> > > every time the function returns false (does not skip)?
> >
> > It doesn't. It resets charge_{target,addr}_from only once after the regions to
> > skip are all skipped. Am I missing something?
>
> You are right.
Thank you for confirming.
>
> My concern was that the current
> 'return false == reset charge_{target, addr}_from' might be a bit hard
> to understand. However, I realize that my change was quite significant.
>
> To make the existing logic clearer for future readers, I think adding a
> brief comment would be helpful.
Makes sense to me.
> For example:
>
> '''
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2368,6 +2368,15 @@ static bool damos_skip_charged_region(struct damon_target *t,
> damon_split_region_at(t, r, sz_to_skip);
> return true;
> }
> + /*
> + * Reset the charge_{target,addr}_from so that the remaining
> + * regions in this/next target can be processed normally. If
> + * the quota becomes full later during the walk,
> + * damos_apply_scheme() will update the
> + * charge_{target,addr}_from to the correct position.
> + * Otherwise, it implies that all applicable regions in this
> + * target have been processed.
> + */
> quota->charge_target_from = NULL;
> quota->charge_addr_from = 0;
> }
> '''
>
> If this is not necessary or redundant, I am perfectly fine with dropping
> it and just applying your minimal fix for the last-region issue in the
> next revision.
I'd suggest to separate the minimal fix and readability improvement. Could you
please post the minimal fix first, with Cc: stable@? After it is done, we
could revisit this readability improvement.
Thanks,
SJ
[...]
next prev parent reply other threads:[~2026-08-28 2:31 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-25 12:46 [RFC PATCH] mm/damon: fix damos quota walk-position tracking Liew Rui Yan
2026-08-25 12:58 ` sashiko-bot
2026-08-25 13:54 ` SJ Park
2026-08-26 10:24 ` Liew Rui Yan
2026-08-26 14:05 ` SJ Park
2026-08-27 0:44 ` SJ Park
2026-08-27 18:08 ` Liew Rui Yan
2026-08-28 0:40 ` SJ Park
2026-08-28 1:54 ` Liew Rui Yan
2026-08-28 2:31 ` SJ Park [this message]
-- strict thread matches above, loose matches on Subject: below --
2026-08-27 5:54 Liew Rui Yan
2026-08-27 6:02 ` Liew Rui Yan
2026-08-27 13:49 ` 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=20260828023112.122652-1-sj@kernel.org \
--to=sj@kernel.org \
--cc=aethernet65535@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=damon@lists.linux.dev \
--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.