Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1188191 > unrolled thread

add a bi_error field to struct bio V3

Back to article view | Back to linux.kernel

Contents

  add a bi_error field to struct bio V3 Christoph Hellwig <hch@lst.de> - 2015-07-20 15:40 +0200
    Re: [PATCH] block: add a bi_error field to struct bio NeilBrown <neilb@suse.com> - 2015-07-22 07:10 +0200
    Re: [PATCH] block: add a bi_error field to struct bio Christoph Hellwig <hch@lst.de> - 2015-07-24 13:00 +0200
      Re: [PATCH] block: add a bi_error field to struct bio Christoph Hellwig <hch@lst.de> - 2015-07-28 13:20 +0200

#1188191 — add a bi_error field to struct bio V3

Bio error reporting has been a mess for a while, and the increasing
use of chained bios makes it worse.  Add a bi_error field to struct
bio to fix this.

Note that the rebase to 4.2-rc means a lot of context changes, so I've
dropped the Reviewed-by tags from V2 as it will need a re-review.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1189651 — Re: [PATCH] block: add a bi_error field to struct bio

On Mon, 20 Jul 2015 15:29:37 +0200 Christoph Hellwig <hch@lst.de> wrote:

> Currently we have two different ways to signal an I/O error on a BIO:
> 
>  (1) by clearing the BIO_UPTODATE flag
>  (2) by returning a Linux errno value to the bi_end_io callback
> 
> The first one has the drawback of only communicating a single possible
> error (-EIO), and the second one has the drawback of not beeing persistent
> when bios are queued up, and are not passed along from child to parent
> bio in the ever more popular chaining scenario.  Having both mechanisms
> available has the additional drawback of utterly confusing driver authors
> and introducing bugs where various I/O submitters only deal with one of
> them, and the others have to add boilerplate code to deal with both kinds
> of error returns.
> 
> So add a new bi_error field to store an errno value directly in struct
> bio and remove the existing mechanisms to clean all this up.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: NeilBrown <neilb@suse.com> (umem and md/raid).

i.e. these files.
>  drivers/block/umem.c                |  4 +--
>  drivers/md/faulty.c                 |  4 +--
>  drivers/md/linear.c                 |  2 +-
>  drivers/md/md.c                     | 18 +++++------
>  drivers/md/multipath.c              | 12 +++----
>  drivers/md/raid0.c                  |  2 +-
>  drivers/md/raid1.c                  | 53 ++++++++++++++++---------------
>  drivers/md/raid10.c                 | 55 +++++++++++++++-----------------
>  drivers/md/raid5.c                  | 52 +++++++++++++++----------------


Thanks,
NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1191716 — Re: [PATCH] block: add a bi_error field to struct bio

On Wed, Jul 22, 2015 at 03:59:46PM -0600, Jens Axboe wrote:
> One possible solution would be to shrink bi_flags to an unsigned int, no 
> problems fitting that in. Then we could stuff bi_error in that (new) hole, 
> and we would end up having the same size again.

As long as we use set/test/clear_bt on bi_flags that won't work unfortunately.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1194125 — Re: [PATCH] block: add a bi_error field to struct bio

On Fri, Jul 24, 2015 at 10:36:45AM -0600, Jens Axboe wrote:
> Right, I don't think we need to do that though. If you look at the flags 
> usage, it's all over the map. Some use test/set_bit, some set it just by 
> OR'ing the mask. There's no reason we can't make this work without relying 
> on set/test_bit, and then shrink it to an unsigned int.

Yes, the current mess doesn't look kosher.  The bvec pool bits don't
really make it better.

But do we really need the cmpxchg hack? Seems like most flags aren't
exposed to concurrency at all, althugh this would need a careful audit.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web