Re: [PATCH 5/5] md: clarify the resync ETA comment in status_resync()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The newly added comment incorrectly states that the divisor `db` is scaled up by 32, when the code actually scales it down.
--

commit 21672ecf7d2b823245c32b9d2699f7cf1a5c2e7c
Author: Hiroshi Nishida <[email protected]>

md: clarify the resync ETA comment in status_resync()

This commit updates the block comment above the remaining-time computation
in status_resync() to clarify the calculation logic. It is a comment-only
change.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 0cd85cc92ed31..a5c0da0d11334 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8916,17 +8916,11 @@ static int status_resync(struct seq_file *seq, struct mddev *mddev)
>  	 * db: blocks written from mark until now
>  	 * rt: remaining time
>  	 *
> -	 * rt is a sector_t, which is always 64bit now. We are keeping
> -	 * the original algorithm, but it is not really necessary.
> -	 *
> -	 * Original algorithm:
> -	 *   So we divide before multiply in case it is 32bit and close
> -	 *   to the limit.
> -	 *   We scale the divisor (db) by 32 to avoid losing precision
> -	 *   near the end of resync when the number of remaining sectors
> -	 *   is close to 'db'.
> -	 *   We then divide rt by 32 after multiplying by db to compensate.
> -	 *   The '+1' avoids division by zero if db is very small.
> +	 * rt is computed as (remaining sectors) * dt / db.  To keep precision
> +	 * near the end of resync, when the remaining count is close to db, the
> +	 * divisor db is scaled up by 32 before the divide and rt is scaled back

[Severity: Low]
Is this new comment accurate regarding the math applied to db?

The comment states that the divisor db is scaled up by 32, but looking at
the actual code:

    rt = max_sectors - resync;
    rt = div64_u64(rt, db/32+1);

This divides db by 32, effectively scaling it down rather than up (which in
turn scales the resulting quotient up). Does this comment need to be
adjusted to reflect the db/32 downscaling?

> +	 * down by 32 afterwards.  The '+1' avoids division by zero when db is
> +	 * very small.
>  	 */
>  	dt = ((jiffies - mddev->resync_mark) / HZ);
>  	if (!dt) dt++;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5