[PATCH 0/5] md: minor cleanups in md core and raid5/raid1/raid10

[PATCH 0/5] md: minor cleanups in md core and raid5/raid1/raid10

[Hiroshi Nishida]

A handful of small, no-functional-change cleanups noticed while reading
the md code:

 - drop an unused parameter from check_decay_read_errors();
 - use max() in raid5_calc_degraded();
 - make is_mddev_idle() take a bool init flag, matching the helper it
   forwards to;
 - declare a sector count as sector_t rather than int in status_resync();
 - rewrite a self-contradictory comment above the resync ETA math.

No behavioral change in any patch.  Built and functionally sanity-tested
(RAID5/RAID6 create, rebuild, scrub) on x86_64, including a run with KASAN
and lockdep enabled -- with concurrent /proc/mdstat polling to exercise
status_resync() -- and no reports.

Hiroshi Nishida (5):
  md/raid1,raid10: drop unused mddev arg from check_decay_read_errors()
  md/raid5: use max() in raid5_calc_degraded()
  md: make is_mddev_idle() take a bool init flag
  md: use sector_t for recovery_active in status_resync()
  md: clarify the resync ETA comment in status_resync()

 drivers/md/md.c       | 26 ++++++++++----------------
 drivers/md/raid1-10.c |  4 ++--
 drivers/md/raid5.c    |  4 +---
 3 files changed, 13 insertions(+), 21 deletions(-)

base-commit: 55b77337bdd088c77461588e5ec094421b89911b

-- 
2.43.0

[PATCH 1/5] md/raid1,raid10: drop unused mddev arg from check_decay_read_errors()

[Hiroshi Nishida]

check_decay_read_errors() only ever touches the passed rdev; the mddev
parameter has been unused. Remove it and update the sole caller in
exceed_read_errors().

No functional change.

Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
 drivers/md/raid1-10.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
index 56a56a4da4f8..edba52eb3116 100644
--- a/drivers/md/raid1-10.c
+++ b/drivers/md/raid1-10.c
@@ -171,7 +171,7 @@ static inline void raid1_prepare_flush_writes(struct mddev *mddev)
  * We halve the read error count for every hour that has elapsed
  * since the last recorded read error.
  */
-static inline void check_decay_read_errors(struct mddev *mddev, struct md_rdev *rdev)
+static inline void check_decay_read_errors(struct md_rdev *rdev)
 {
 	long cur_time_mon;
 	unsigned long hours_since_last;
@@ -206,7 +206,7 @@ static inline bool exceed_read_errors(struct mddev *mddev, struct md_rdev *rdev)
 	int max_read_errors = atomic_read(&mddev->max_corr_read_errors);
 	int read_errors;
 
-	check_decay_read_errors(mddev, rdev);
+	check_decay_read_errors(rdev);
 	read_errors =  atomic_inc_return(&rdev->read_errors);
 	if (read_errors > max_read_errors) {
 		pr_notice("md/"RAID_1_10_NAME":%s: %pg: Raid device exceeded read_error threshold [cur %d:max %d]\n",
-- 
2.43.0

[PATCH 2/5] md/raid5: use max() in raid5_calc_degraded()

[Hiroshi Nishida]

The tail of raid5_calc_degraded() open-codes a max of the two computed
degraded counts. Use max() for clarity.

No functional change.

Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
 drivers/md/raid5.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 0c5c9fb0606e..2b2aa4f2431c 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -741,9 +741,7 @@ int raid5_calc_degraded(struct r5conf *conf)
 			if (conf->raid_disks <= conf->previous_raid_disks)
 				degraded2++;
 	}
-	if (degraded2 > degraded)
-		return degraded2;
-	return degraded;
+	return max(degraded, degraded2);
 }
 
 static bool has_failed(struct r5conf *conf)
-- 
2.43.0

[PATCH 3/5] md: make is_mddev_idle() take a bool init flag

[Hiroshi Nishida]

is_mddev_idle() took an int init flag but passes it straight through to
is_rdev_holder_idle(), which already takes a bool. Make the types
consistent and update the two callers to pass true/false.

No functional change.

Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
 drivers/md/md.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index d1465bcd86c8..829b4a3f545b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9219,7 +9219,7 @@ static bool is_rdev_holder_idle(struct md_rdev *rdev, bool init)
  *
  * Noted this checking rely on IO accounting is enabled.
  */
-static bool is_mddev_idle(struct mddev *mddev, int init)
+static bool is_mddev_idle(struct mddev *mddev, bool init)
 {
 	unsigned long last_events = mddev->normal_io_events;
 	struct gendisk *disk;
@@ -9771,7 +9771,7 @@ void md_do_sync(struct md_thread *thread)
 	pr_debug("md: using maximum available idle IO bandwidth (but not more than %d KB/sec) for %s.\n",
 		 speed_max(mddev), desc);
 
-	is_mddev_idle(mddev, 1); /* this initializes IO event counters */
+	is_mddev_idle(mddev, true); /* this initializes IO event counters */
 
 	io_sectors = 0;
 	for (m = 0; m < SYNC_MARKS; m++) {
@@ -9920,7 +9920,7 @@ void md_do_sync(struct md_thread *thread)
 				goto repeat;
 			}
 			if (!sync_io_within_limit(mddev) &&
-			    !is_mddev_idle(mddev, 0)) {
+			    !is_mddev_idle(mddev, false)) {
 				/*
 				 * Give other IO more of a chance.
 				 * The faster the devices, the less we wait.
-- 
2.43.0

[PATCH 4/5] md: use sector_t for recovery_active in status_resync()

[Hiroshi Nishida]

recovery_active holds a sector count read from the sector-typed atomic
mddev->recovery_active and is then combined with the sector_t values
curr_mark_cnt and resync_mark_cnt. Declaring it as a plain int needlessly
narrows it and mixes signedness into sector_t arithmetic; declare it
sector_t to match.

No functional change.

Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
 drivers/md/md.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 829b4a3f545b..0cd85cc92ed3 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8816,8 +8816,8 @@ static int status_resync(struct seq_file *seq, struct mddev *mddev)
 {
 	sector_t max_sectors, resync, res;
 	unsigned long dt, db = 0;
-	sector_t rt, curr_mark_cnt, resync_mark_cnt;
-	int scale, recovery_active;
+	sector_t rt, curr_mark_cnt, resync_mark_cnt, recovery_active;
+	int scale;
 	unsigned int per_milli;
 
 	if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery) ||
-- 
2.43.0

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

[Hiroshi Nishida]

The comment above the remaining-time computation was self-contradictory:
it said the original algorithm was being kept but "is not really
necessary", then described that algorithm under an "Original algorithm:"
heading as if it had been replaced. The code still uses it. Rewrite the
comment to simply describe what the code does.

Comment only; no functional change.

Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
 drivers/md/md.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 0cd85cc92ed3..a5c0da0d1133 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
+	 * down by 32 afterwards.  The '+1' avoids division by zero when db is
+	 * very small.
 	 */
 	dt = ((jiffies - mddev->resync_mark) / HZ);
 	if (!dt) dt++;
-- 
2.43.0

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

[sashiko-bot]

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 <nishidafmly@gmail.com>

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/20260624155421.211626-1-nishidafmly@gmail.com?part=5

Re: [PATCH 4/5] md: use sector_t for recovery_active in status_resync()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message falsely claims `mddev->recovery_active` is a 'sector-typed atomic' and that declaring a local variable as `int` 'needlessly narrows it', but the structure field remains a standard 32-bit `atomic_t`.
--

commit 7ec77a6b233ce4d151f7fc723e5f312b20b4ac92
Author: Hiroshi Nishida <nishidafmly@gmail.com>

md: use sector_t for recovery_active in status_resync()

This commit changes the local variable recovery_active in status_resync()
from int to sector_t. The commit message states this is to match the type
of mddev->recovery_active and avoid needlessly narrowing the value.

> recovery_active holds a sector count read from the sector-typed atomic
> mddev->recovery_active and is then combined with the sector_t values
> curr_mark_cnt and resync_mark_cnt. Declaring it as a plain int needlessly
> narrows it and mixes signedness into sector_t arithmetic; declare it
> sector_t to match.

[Severity: Medium]
Is this description accurate? Looking at struct mddev in drivers/md/md.h,
recovery_active is defined as a standard atomic_t:

drivers/md/md.h:struct mddev {
    ...
    atomic_t recovery_active;
    ...
}

Since atomic_t is a 32-bit signed integer, and atomic_read() returns an int,
does changing this local variable to sector_t actually prevent narrowing? The
implicit cast from int to sector_t will now simply happen at assignment
rather than during the arithmetic operations.

Could the commit message be updated to reflect that mddev->recovery_active
is a standard atomic_t, rather than a "sector-typed atomic"?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624155421.211626-1-nishidafmly@gmail.com?part=4

Re: [PATCH 4/5] md: use sector_t for recovery_active in status_resync()

[Hiroshi Nishida]

> [Medium] mddev->recovery_active is a standard atomic_t, not "sector-typed";
> changing the local to sector_t doesn't prevent narrowing.

You're right -- mddev->recovery_active is an atomic_t and
atomic_read() returns int, so there's nothing wider to narrow; the
int->sector_t conversion just moves to the assignment.
The change is only a type-consistency tidy (the local is used solely
with the sector_t curr_mark_cnt/resync_mark_cnt), not a narrowing fix.
I'll reword the commit message in v2 accordingly and drop the
"sector-typed atomic"/"narrows" wording.


2026年6月24日(水) 9:03 <sashiko-bot@kernel.org>:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The commit message falsely claims `mddev->recovery_active` is a 'sector-typed atomic' and that declaring a local variable as `int` 'needlessly narrows it', but the structure field remains a standard 32-bit `atomic_t`.
> --
>
> commit 7ec77a6b233ce4d151f7fc723e5f312b20b4ac92
> Author: Hiroshi Nishida <nishidafmly@gmail.com>
>
> md: use sector_t for recovery_active in status_resync()
>
> This commit changes the local variable recovery_active in status_resync()
> from int to sector_t. The commit message states this is to match the type
> of mddev->recovery_active and avoid needlessly narrowing the value.
>
> > recovery_active holds a sector count read from the sector-typed atomic
> > mddev->recovery_active and is then combined with the sector_t values
> > curr_mark_cnt and resync_mark_cnt. Declaring it as a plain int needlessly
> > narrows it and mixes signedness into sector_t arithmetic; declare it
> > sector_t to match.
>
> [Severity: Medium]
> Is this description accurate? Looking at struct mddev in drivers/md/md.h,
> recovery_active is defined as a standard atomic_t:
>
> drivers/md/md.h:struct mddev {
>     ...
>     atomic_t recovery_active;
>     ...
> }
>
> Since atomic_t is a 32-bit signed integer, and atomic_read() returns an int,
> does changing this local variable to sector_t actually prevent narrowing? The
> implicit cast from int to sector_t will now simply happen at assignment
> rather than during the arithmetic operations.
>
> Could the commit message be updated to reflect that mddev->recovery_active
> is a standard atomic_t, rather than a "sector-typed atomic"?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260624155421.211626-1-nishidafmly@gmail.com?part=4



-- 
Hiroshi Nishida
nishidafmly@gmail.com

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

[Hiroshi Nishida]

> The comment states that the divisor db is scaled up by 32, but the code
> rt = div64_u64(rt, db/32+1); divides db by 32 -- scaling it down.

You're right -- the code divides by db/32, which scales the divisor
*down* by 32 (so the intermediate quotient is 32x larger and keeps
low-order bits a plain divide by db would lose), and rt is then
rescaled by the >> 5.
My rewritten comment had the direction backwards.  I'll fix it in v2:

      rt is, in exact arithmetic, (remaining sectors) * dt / db.  To keep
      precision near the end of resync, when the remaining count is close to db,
      the dividend is divided by db/32 rather than db -- i.e. the divisor is
      scaled down by 32, so the intermediate quotient is 32x larger -- and rt is
      scaled back down by 32 (>> 5) afterwards.

  Thanks,

2026年6月24日(水) 8:58 <sashiko-bot@kernel.org>:
>
> 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 <nishidafmly@gmail.com>
>
> 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/20260624155421.211626-1-nishidafmly@gmail.com?part=5



-- 
Hiroshi Nishida
nishidafmly@gmail.com