Re: [RFC PATCH] mm/damon: fix damos quota walk-position tracking

From: SJ Park

Date: Thu Aug 27 2026 - 22:31:26 EST


On Fri, 28 Aug 2026 09:54:29 +0800 Liew Rui Yan <aethernet65535@xxxxxxxxx> wrote:

> On Thu, 27 Aug 2026 17:40:48 -0700 SJ Park <sj@xxxxxxxxxx> wrote:
>
> > On Fri, 28 Aug 2026 02:08:22 +0800 Liew Rui Yan <aethernet65535@xxxxxxxxx> 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

[...]