linux-kernel - Re: [PATCH v4 5/9] md/raid1,raid10: Set R{1,10}BIO_Uptodate when successful retry of a failed bio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8136b746-50c9-51eb-483b-f2661e86d3eb@huaweicloud.com>
Date: Thu, 18 Sep 2025 14:39:45 +0800
From: Li Nan <linan666@...weicloud.com>
To: Kenta Akagi <k@...l.me>, linan666@...weicloud.com, song@...nel.org,
 yukuai3@...wei.com, mtkaczyk@...nel.org, shli@...com, jgq516@...il.com
Cc: linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 5/9] md/raid1,raid10: Set R{1,10}BIO_Uptodate when
 successful retry of a failed bio



在 2025/9/17 21:20, Kenta Akagi 写道:

>>>                if (!narrow_write_error(r1_bio, m))
>>> -                md_error(conf->mddev,
>>> -                     conf->mirrors[m].rdev);
>>> +                md_error(conf->mddev, rdev);
>>>                    /* an I/O failed, we can't clear the bitmap */
>>> -            rdev_dec_pending(conf->mirrors[m].rdev,
>>> -                     conf->mddev);
>>> +            else if (test_bit(In_sync, &rdev->flags) &&
>>> +                 !test_bit(Faulty, &rdev->flags) &&
>>> +                 rdev_has_badblock(rdev,
>>> +                           r1_bio->sector,
>>> +                           r1_bio->sectors) == 0)
>>
>> Clear badblock and set R10BIO_Uptodate if rdev has badblock.
> 
> narrow_write_error returns true when the write succeeds, or when the write
> fails but rdev_set_badblocks succeeds. Here, it determines that the re-write
> succeeded if there is no badblock in the sector to be written by r1_bio.
> So we should not call rdev_clear_badblocks here.
> 

I am trying to cleanup narrow_write_error():

https://lore.kernel.org/linux-raid/20250917093508.456790-3-linan666@huaweicloud.com/T/#u

It may be clearer if narrow_write_error() returns true when all fix IO
succeeds.

```
@@ -2553,11 +2551,17 @@ static bool narrow_write_error(struct r1bio 
*r1_bio, int i)
                 bio_trim(wbio, sector - r1_bio->sector, sectors);
                 wbio->bi_iter.bi_sector += rdev->data_offset;

-               if (submit_bio_wait(wbio) < 0)
-                       /* failure! */
-                       ok = rdev_set_badblocks(rdev, sector,
-                                               sectors, 0)
-                               && ok;
+               if (submit_bio_wait(wbio) < 0) {
+                       ok = false;
+                       if (rdev_set_badblocks(rdev, sector, sectors, 0)) {
+                               /*
+                                * Badblocks set failed, disk marked Faulty.
+                                * No further operations needed.
+                                */
+                               bio_put(wbio);
+                               break;
+                       }
+               }

                 bio_put(wbio);
                 sect_to_write -= sectors;
```

We can clear badblocks and set R10BIO_Uptodate after it. What do you think?
-- 
Thanks,
Nan