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

Damien Le Moal
2024-03-26T01:54:45Z

On 3/26/24 04:39, Bart Van Assche wrote:
> On 3/24/24 21:44, Damien Le Moal wrote:
>> - if (bio_bytes == bio->bi_iter.bi_size)
>> + if (unlikely(error))
>> + bio->bi_status = error;
>> +
>> + if (bio_bytes == bio->bi_iter.bi_size) {
>
> Why no "else" in front of the above if? I think this patch introduces a
> behavior change. With the current code, if a zone append operation
> fails, bio->bi_status is set to 'error'. With this patch applied, this
> becomes BLK_STS_IOERR.

Adding an else would be very wrong since we still need to go to the next bio of
the request if the current BIO failed. But I get your point. Will fix it.

>
>> req->bio = bio->bi_next;
>> + } else if (req_op(req) == REQ_OP_ZONE_APPEND) {
>> + /*
>> + * Partial zone append completions cannot be supported
>> + * as the BIO fragments may end up not being written
>> + * sequentially. For such case, force the completed
>> + * nbytes to be equal to the BIO size so that
>> + * bio_advance() sets the BIO remaining size to 0 and
>> + * we end up calling bio_endio() before returning.
>> + */
>> + bio->bi_status = BLK_STS_IOERR;
>> + bio_bytes = bio->bi_iter.bi_size;
>> + }
>
> Thanks,
>
> Bart.
>
>

--
Damien Le Moal
Western Digital Research