Re: [PATCH v5 07/29] md: avoid stale clone I/O accounting timestamps
[email protected] Sun, 02 Aug 2026 20:45:17 +0000
| Newsgroups | org.kernel.vger.linux-raid |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: Pre-existing issues: - [High] Permanent in-flight I/O counter leak when jiffies wraps to 0. -- --- Patch [7]: [PATCH v5 07/29] md: avoid stale clone I/O accounting timest= amps --- commit 3831c24d41ef696f06d6f215b39197db739a0831 Author: Yu Kuai <[email protected]> md: avoid stale clone I/O accounting timestamps =20 This commit avoids stale clone I/O accounting timestamps by explicitly setting start_time to 0 when queue I/O stats are disabled. This prevents the completion path in md_end_clone_io() from using a stale non-zero value= =20 from a pooled bio and performing accounting that was never started. =20 > diff --git a/drivers/md/md.c b/drivers/md/md.c > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -9448,6 +9448,8 @@ static void md_clone_bio(struct mddev *mddev, struc= t bio **bio) > md_io_clone->mddev =3D mddev; > if (blk_queue_io_stat(bdev->bd_disk->queue)) > md_io_clone->start_time =3D bio_start_io_acct(*bio); > + else > + md_io_clone->start_time =3D 0; [Severity: High] This is a pre-existing issue, but does using 0 as a sentinel value here create a permanent in-flight I/O counter leak when jiffies wraps to 0? In bio_start_io_acct(), the returned value is the current jiffies. On 32-bit systems, jiffies is initialized to -300*HZ and naturally wraps to 0 exactly five minutes after boot. If a bio is cloned exactly at the tick where jiffies is 0, start_time will be recorded as 0 even though accounting was started. When completion occurs in md_end_clone_io(), the check sees 0: if (md_io_clone->start_time) bio_end_io_acct(orig_bio, md_io_clone->start_time); This evaluates to false, bypassing the end accounting and leaving the in-flight counter permanently incremented. This can cause operations that wait for a queue freeze, such as suspend or device removal, to hang indefinitely. Would it be safer to use a separate boolean flag to track if accounting was started, rather than overloading start_time? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260802195038.1642= [email protected]?part=3D7