Hello, quick question for backporting.. On 09/15/17 23:06, Liu Bo wrote: > commit 4246a0b63bd8 ("block: add a bi_error field to struct bio") > changed the logic of how dio read endio reports errors. [snip]
I've tried to merge this into my 4.9.x++ tree but have a question since the DIO APIs changed recently and itt's hard to tell what is a bug and what is a feature.. :-/ > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -8267,11 +8267,8 @@ static void btrfs_endio_direct_read(struct bio *bio) > struct btrfs_io_bio *io_bio = btrfs_io_bio(bio); > blk_status_t err = bio->bi_status; > > - if (dip->flags & BTRFS_DIO_ORIG_BIO_SUBMITTED) { > + if (dip->flags & BTRFS_DIO_ORIG_BIO_SUBMITTED) > err = btrfs_subio_endio_read(inode, io_bio, err); > - if (!err) > - bio->bi_status = 0; > - } > > unlock_extent(&BTRFS_I(inode)->io_tree, dip->logical_offset, > dip->logical_offset + dip->bytes - 1); This hunk is fairly easy, just reverse bi_status to bi->error. However.. > @@ -8279,7 +8276,7 @@ static void btrfs_endio_direct_read(struct bio *bio) > > kfree(dip); > > - dio_bio->bi_status = bio->bi_status; > + dio_bio->bi_status = err; > dio_end_io(dio_bio); ^^^^^^^^^^^^^^^^^^^ Same here, except that the call to dio_end_io used to take a second parameter (the error code, which has been moved into bi_status in 4.10+) and looked like this: dio_end_io(dio_bio, bio->bi_error); Given that "bio->bi_error" should have been "err" instead, I think err should also be passed to dio_end_io(), so that the whole hunk would look like: .. - dio_bio->bi_error = bio->bi_error; - dio_end_io(dio_bio, bio->bi_error); + dio_bio->bi_error = err; + dio_end_io(dio_bio, err); .. Would this be correct or did I misunderstand some subtle aspect about the DIO error handling? Thanks :) Holger -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html