From: sashiko-bot@kernel.org
To: "Hiroshi Nishida" <nishidafmly@gmail.com>
Cc: yukuai@fygo.io, linux-raid@vger.kernel.org
Subject: Re: [PATCH 4/5] md: use sector_t for recovery_active in status_resync()
Date: Wed, 24 Jun 2026 16:03:31 +0000 [thread overview]
Message-ID: <20260624160331.9C5F51F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260624155421.211626-5-nishidafmly@gmail.com>
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
next prev parent reply other threads:[~2026-06-24 16:03 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-24 15:54 [PATCH 0/5] md: minor cleanups in md core and raid5/raid1/raid10 Hiroshi Nishida
2026-06-24 15:54 ` [PATCH 1/5] md/raid1,raid10: drop unused mddev arg from check_decay_read_errors() Hiroshi Nishida
2026-06-24 15:54 ` [PATCH 2/5] md/raid5: use max() in raid5_calc_degraded() Hiroshi Nishida
2026-06-24 15:54 ` [PATCH 3/5] md: make is_mddev_idle() take a bool init flag Hiroshi Nishida
2026-06-24 15:54 ` [PATCH 4/5] md: use sector_t for recovery_active in status_resync() Hiroshi Nishida
2026-06-24 16:03 ` sashiko-bot [this message]
2026-06-24 16:48 ` Hiroshi Nishida
2026-06-24 15:54 ` [PATCH 5/5] md: clarify the resync ETA comment " Hiroshi Nishida
2026-06-24 15:58 ` sashiko-bot
2026-06-24 16:50 ` Hiroshi Nishida
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260624160331.9C5F51F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=linux-raid@vger.kernel.org \
--cc=nishidafmly@gmail.com \
--cc=sashiko-reviews@lists.linux.dev \
--cc=yukuai@fygo.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.