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


Reply via email to