Re: [PATCH 02/26] block: Remove req_bio_endio()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/6/24 02:28, Bart Van Assche wrote:
> On 2/1/24 23:30, Damien Le Moal wrote:
>> @@ -916,9 +888,8 @@ bool blk_update_request(struct request *req, blk_status_t
>> error,
>>       if (blk_crypto_rq_has_keyslot(req) && nr_bytes >= blk_rq_bytes(req))
>>           __blk_crypto_rq_put_keyslot(req);
>>   -    if (unlikely(error && !blk_rq_is_passthrough(req) &&
>> -             !(req->rq_flags & RQF_QUIET)) &&
>> -             !test_bit(GD_DEAD, &req->q->disk->state)) {
>> +    if (unlikely(error && !blk_rq_is_passthrough(req) && !quiet) &&
>> +        !test_bit(GD_DEAD, &req->q->disk->state)) {
> 
> The new indentation of !test_bit(GD_DEAD, &req->q->disk->state) looks odd to me

But it is actually correct because that test bit is not part of the unlikely().
Not sure if that is intentional though.

> ...
> 
>>           blk_print_req_error(req, error);
>>           trace_block_rq_error(req, error, nr_bytes);
>>       }
>> @@ -930,12 +901,37 @@ bool blk_update_request(struct request *req,
>> blk_status_t error,
>>           struct bio *bio = req->bio;
>>           unsigned bio_bytes = min(bio->bi_iter.bi_size, nr_bytes);
>>   -        if (bio_bytes == bio->bi_iter.bi_size)
>> +        if (unlikely(error))
>> +            bio->bi_status = error;
>> +
>> +        if (bio_bytes == bio->bi_iter.bi_size) {
>>               req->bio = bio->bi_next;
> 
> The behavior has been changed compared to the original code: the original code
> only tests bio_bytes if error == 0. The new code tests bio_bytes no matter what
> value the 'error' variable has. Is this behavior change intentional?

No change actually. The bio_bytes test was in blk_update_request() already.

> 
> Otherwise this patch looks good to me.
> 
> Thanks,
> 
> Bart.

-- 
Damien Le Moal
Western Digital Research





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux