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: <6f0f9730-4bbe-7f3c-1b50-690bb77d5d90@huaweicloud.com>
Date: Fri, 19 Sep 2025 09:37:18 +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/18 23:36, Kenta Akagi 写道:
> 
> 
> On 2025/09/18 15:39, Li Nan wrote:
>>
>>
>> 在 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 for the detailed explanation.
> narrow_write_error now returns whether the retry write succeeded.
> If rdev_set_badblock fails, narrow_write_error calls md_error. This looks good to me.
> And after narrow_write_error cleanup, rdev_clear_badblock should be called.
> 
> BTW, I'm not familiar with the patch workflow. Should I submit my
> handle_write_finished patch separately after your cleanup patch merged?
> 

Sure, please send it separately after my cleanup patch. Thanks.

> Thanks,
> Akagi
> 
>> -- 
>> Thanks,
>> Nan
>>
>>
> 
> 
> 
> .

-- 
Thanks,
Nan