Re: [RFC PATCH v3 07/37] mm/damon/core: apply access reports to high level snapshot

[SeongJae Park <sj@kernel.org>]

On Fri, 12 Dec 2025 22:20:04 +0900 JaeJoon Jung <rgbi3307@gmail.com> wrote:

> On Mon, 8 Dec 2025 at 15:35, SeongJae Park <sj@kernel.org> wrote:
> >
> > Now any DAMON API callers can report their observed access information.
> > The DAMON core layer is just ignoring those, though.  Update the core to
> > use the reported information at building the high level access pattern
> > snapshot.
> 
> It seems inefficient to repeatedly access the damon_access_reports[1000] array
> using a for loop in the kdamond_check_reported_accesses() function.
> It is inefficient to for loop through the entire
> damon_access_reports[1000] array.
> When CONFIG_HZ and jiffies are increased as follows and
> damond sample_interval is 5000us (5ms), the time flow diagram is as follows.
> 
> CONFIG_HZ 1000, jiffies == 1ms
> damond sample_interval == 5000us (5ms)
> 
> reports_len(==): [0 ... 5]
>                                                      [*]
>         0    1    2    3    4    5     6    7    8    9        997    998     999
>         [====|====|====|====|====]-----|----|----|----|  ....  |------|-------|
> jiffies++    1    2    3    4    5     0    0    0    0        0      0       0
> damond_fn(sample interval)      -5[0<]
> 
> reports_len(==): [997 ... 2]
>                                   [*]
>         0      1      2    3    4    5     6    7    8    9        997   998   999
>         [======|======]----|----|----|-----|----|----|----|  ....  [=====|=====]
> jiffies++   1001      1002 3    4    5     6    7    8    9        997   998   999
> damond_fn(sample interval)
>                             -5[997<]

Please use fixed-length fonts for something like above, from next time.  I
fixed the diagram with my best effort, as above.  But I still fail at
understanding your point.  More clarification about what the diagram means
would be nice.

> 
> It seems that only the section corresponding to the sample interval ([==|==])
> can be cycled as follows.  And, how about enjoying damon_access_reports[1000]
> as damon_access_reports[500]?  Even if it reduce the 1000ms to 500ms
> array space, it seems that it can sufficiently report and process within
> the sample interval of 5ms.

Are you assuming the the reports can be made only once per 1 millisecond?  That
is not true.  The design assumes any kernel API caller could make the report,
so more than one report can be made within one millisecond.  Am I
missingsomething?

> 
>  static unsigned int kdamond_check_reported_accesses(struct damon_ctx *ctx)
>  {
> -       int i;
> +       int i = damon_access_reports_len;
> +       unsigned int nr = 0;
>         struct damon_access_report *report;
>         struct damon_target *t;
> 
> @@ -2904,16 +2905,18 @@ static unsigned int
> kdamond_check_reported_accesses(struct damon_ctx *ctx)
>                 return 0;
> 
>         mutex_lock(&damon_access_reports_lock);
> -       for (i = 0; i < damon_access_reports_len; i++) {
> -               report = &damon_access_reports[i];
> -               if (time_before(report->report_jiffies,
> -                                       jiffies -
> -                                       usecs_to_jiffies(
> -                                               ctx->attrs.sample_interval)))
> -                       continue;
> +       report = &damon_access_reports[i];
> +       while (time_after(report->report_jiffies,
> +               jiffies - usecs_to_jiffies(ctx->attrs.sample_interval))) {
>                 damon_for_each_target(t, ctx)
>                         kdamond_apply_access_report(report, t, ctx);
> +               if (++nr >= DAMON_ACCESS_REPORTS_CAP)
> +                       break;
> +
> +               i = (i == 0) ? (DAMON_ACCESS_REPORTS_CAP - 1) : (i - 1);
> +               report = &damon_access_reports[i];
>         }
> +
>         mutex_unlock(&damon_access_reports_lock);
>         /* For nr_accesses_bp, absence of access should also be reported. */
>         return kdamond_apply_zero_access_report(ctx);
> }

So I still don't get your points before the above code diff, but I understand
this code diff.

I agree this is more efficient.  I will consider doing something like this in
the next spin.


Thanks,
SJ

[...]