Re: [PATCH v31 03/13] mm/damon: Adaptively adjust regions

[SeongJae Park]

From: SeongJae Park <sjpark@xxxxxxxxx>

On Tue, 22 Jun 2021 07:59:35 -0700 Shakeel Butt <shakeelb@xxxxxxxxxx> wrote:

> On Mon, Jun 21, 2021 at 1:31 AM SeongJae Park <sj38.park@xxxxxxxxx> wrote:
> >
> > From: SeongJae Park <sjpark@xxxxxxxxx>
> >
> > Even somehow the initial monitoring target regions are well constructed
> > to fulfill the assumption (pages in same region have similar access
> > frequencies), the data access pattern can be dynamically changed.  This
> > will result in low monitoring quality.  To keep the assumption as much
> > as possible, DAMON adaptively merges and splits each region based on
> > their access frequency.
> >
> > For each ``aggregation interval``, it compares the access frequencies of
> > adjacent regions and merges those if the frequency difference is small.
> > Then, after it reports and clears the aggregated access frequency of
> > each region, it splits each region into two or three regions if the
> > total number of regions will not exceed the user-specified maximum
> > number of regions after the split.
> >
> > In this way, DAMON provides its best-effort quality and minimal overhead
> > while keeping the upper-bound overhead that users set.
> >
> > Signed-off-by: SeongJae Park <sjpark@xxxxxxxxx>
> > Reviewed-by: Leonard Foerster <foersleo@xxxxxxxxx>
> > Reviewed-by: Fernand Sieber <sieberf@xxxxxxxxxx>
> [...]
> >
> > +unsigned int damon_nr_regions(struct damon_target *t)
> > +{
> > +       struct damon_region *r;
> > +       unsigned int nr_regions = 0;
> > +
> > +       damon_for_each_region(r, t)
> > +               nr_regions++;
> 
> This bugs me everytime. Please just have nr_regions field in the
> damon_target instead of traversing the list to count the number of
> regions.

Ok, I will make the change in next spin.

> 
> Other than that, it looks good to me.

Thanks,
SeongJae Park