Re: (sashiko review) [PATCH 1/3] mm/damon/ops-common: optimize damon_hot_score() using ilog2()

[SeongJae Park <sj@kernel.org>]

TL; DR: sashiko didn't find a blocker for this patch.

Forwarding Sashiko review in a review format with my inline comment, for
sharing more details of my view and doing more discussions via mails if needed.

> # review url: https://sashiko.dev/#/patchset/20260402155733.77050-2-sj@kernel.org
> # start of sashiko.dev inline review
> commit 3d1d9600685998aeba66926091d5a11897567bd0
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/ops-common: optimize damon_hot_score() using ilog2()
> 
> This commit replaces a manual for-loop with an ilog2() based calculation
> to optimize the derivation of age_in_log in damon_hot_score(), improving
> latency during performance tests.
> 
> > diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> > --- a/mm/damon/ops-common.c
> > +++ b/mm/damon/ops-common.c
> [ ... ]
> > @@ -117,9 +117,12 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,
> >  		damon_max_nr_accesses(&c->attrs);
> >  
> >  	age_in_sec = (unsigned long)r->age * c->attrs.aggr_interval / 1000000;
> > -	for (age_in_log = 0; age_in_log < DAMON_MAX_AGE_IN_LOG && age_in_sec;
> > -			age_in_log++, age_in_sec >>= 1)
> > -		;
> > +	if (age_in_sec)
> > +		age_in_log = min_t(int, ilog2(age_in_sec) + 1,
> > +				DAMON_MAX_AGE_IN_LOG);
> > +	else
> > +		age_in_log = 0;
> 
> This isn't a bug, but could this logic be simplified and the branch avoided
> by using fls() instead of ilog2()?
> 
> For non-constant variable arguments, ilog2(x) internally resolves to
> fls(x) - 1. By evaluating ilog2(age_in_sec) + 1, this code essentially
> calculates fls(age_in_sec) - 1 + 1.
> 
> Since fls() natively returns 0 when the input is 0, using fls(age_in_sec)
> directly would remove the need for the conditional branch entirely and
> reduce the block to a single expression.

We decided to use ilog2() since it makes the code easier to read in my taste.
So, this is not a blocker for this patch.

> 
> 
> # end of sashiko.dev inline review
> # review url: https://sashiko.dev/#/patchset/20260402155733.77050-2-sj@kernel.org


Thanks,
SJ

[...]