Re: [PATCH 1/1] mm/damon/core: simplify minimum watermark wait time calculation

[Huang Adrian]

Hi SJ,

On Tue, Nov 10, 2026 at 11:39 PM SJ Park <sj@xxxxxxxxxx> wrote:
>
> Hello Adrian,
>
> On Tue, 10 Nov 2026 16:51:32 +0800 "Adrian Huang (Lenovo)" <adrianhuang0701@xxxxxxxxx> wrote:
>
> > The minimum watermark wait time calculation uses an additional boolean
> > variable, init_wait_time, to determine whether min_wait_time has been
> > initialized.
> >
> > Simplify the logic by using list_is_first() to identify the first
> > scheme and initialize min_wait_time accordingly. Subsequent schemes
> > update min_wait_time only when their waiting intervals are shorter.
> >
> > This removes the unnecessary boolean variable while preserving the
> > existing behavior, including the case where no schemes are configured.
> >
> > Signed-off-by: Adrian Huang (Lenovo) <adrianhuang0701@xxxxxxxxx>
> > ---
> >  mm/damon/core.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index d36823f4d092..93ec684a113c 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3985,14 +3985,12 @@ static int kdamond_wait_activation(struct damon_ctx *ctx)
> >
> >       while (!kdamond_need_stop(ctx)) {
> >               unsigned long min_wait_time = 0;
> > -             bool init_wait_time = false;
> >
> >               damon_for_each_scheme(s, ctx) {
> >                       wait_time = damos_wmark_wait_us(s);
> > -                     if (!init_wait_time || wait_time < min_wait_time) {
> > -                             init_wait_time = true;
> > +                     if (list_is_first(&s->list, &ctx->schemes) ||
> > +                         wait_time < min_wait_time)
> >                               min_wait_time = wait_time;
> > -                     }
>
> I implemented and used list handling wrappers like damon_for_each_scheme(), to
> hide implementation details of data structures.  This change exposes some of
> such details.  If we agree we have to remove init_wait_time, I'd prefer adding
> a sort of damon_first_scheme() like wrapper.

Ah, sorry. I didn't realize you intentionally hid the implementation
details. That makes sense.

> But, init_wait_time really need to be removed?  I cannot show clear benefit of
> removing it that justifies adding a new wrapper.  To someone or on some
> evironments, init_wait_time might making the code too difficult to read or
> inefficient (system having so small amount of memory?), though.  If not, I'd
> suggest dropping this.
>
> What do you think, Adrian?

I agree with your point. Let's drop this patch. Thanks for the quick
feedback anyway. ;-)

-- Adrian