On Mon, Sep 18, 2017 at 03:49:34PM +0200, Holger Hoffstätte wrote: > > 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.. :-/ >
Good point, hopefully it's the last change on those API. > > --- 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? Yes, the diff looks good to me. Thanks, -liubo -- 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