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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
