* [PATCH] Btrfs: tolerate errors if we have retried successfully
@ 2017-04-14 1:11 Liu Bo
2017-05-05 16:52 ` David Sterba
2017-05-17 21:42 ` [PATCH v2] " Liu Bo
0 siblings, 2 replies; 6+ messages in thread
From: Liu Bo @ 2017-04-14 1:11 UTC (permalink / raw)
To: linux-btrfs
With raid1 profile, dio read isn't tolerating IO errors if read length is
less than the stripe length (64K).
This fixes the problem by setting bio's error to 0 if a good copy has been
found.
Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
fs/btrfs/inode.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 632b616..4e1398e 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8113,8 +8113,11 @@ static void btrfs_endio_direct_read(struct bio *bio)
struct btrfs_io_bio *io_bio = btrfs_io_bio(bio);
int err = bio->bi_error;
- 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_error = 0;
+ }
unlock_extent(&BTRFS_I(inode)->io_tree, dip->logical_offset,
dip->logical_offset + dip->bytes - 1);
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Btrfs: tolerate errors if we have retried successfully
2017-04-14 1:11 [PATCH] Btrfs: tolerate errors if we have retried successfully Liu Bo
@ 2017-05-05 16:52 ` David Sterba
2017-05-09 19:40 ` Liu Bo
2017-05-17 21:42 ` [PATCH v2] " Liu Bo
1 sibling, 1 reply; 6+ messages in thread
From: David Sterba @ 2017-05-05 16:52 UTC (permalink / raw)
To: Liu Bo; +Cc: linux-btrfs
On Thu, Apr 13, 2017 at 06:11:56PM -0700, Liu Bo wrote:
> With raid1 profile, dio read isn't tolerating IO errors if read length is
> less than the stripe length (64K).
Can you please write more details why this is true? Some pointers to
code etc, I'm lost. Eg. where the errors is tolerated. Thanks.
> This fixes the problem by setting bio's error to 0 if a good copy has been
> found.
>
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ---
> fs/btrfs/inode.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 632b616..4e1398e 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -8113,8 +8113,11 @@ static void btrfs_endio_direct_read(struct bio *bio)
> struct btrfs_io_bio *io_bio = btrfs_io_bio(bio);
> int err = bio->bi_error;
>
> - 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_error = 0;
> + }
>
> unlock_extent(&BTRFS_I(inode)->io_tree, dip->logical_offset,
> dip->logical_offset + dip->bytes - 1);
> --
> 2.5.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Btrfs: tolerate errors if we have retried successfully
2017-05-05 16:52 ` David Sterba
@ 2017-05-09 19:40 ` Liu Bo
2017-05-15 15:57 ` David Sterba
0 siblings, 1 reply; 6+ messages in thread
From: Liu Bo @ 2017-05-09 19:40 UTC (permalink / raw)
To: dsterba, linux-btrfs
On Fri, May 05, 2017 at 06:52:45PM +0200, David Sterba wrote:
> On Thu, Apr 13, 2017 at 06:11:56PM -0700, Liu Bo wrote:
> > With raid1 profile, dio read isn't tolerating IO errors if read length is
> > less than the stripe length (64K).
>
> Can you please write more details why this is true? Some pointers to
> code etc, I'm lost. Eg. where the errors is tolerated. Thanks.
Sure.
Our bio didn't get split in btrfs_submit_direct_hook() if (dip->flags &
BTRFS_DIO_ORIG_BIO_SUBMITTED) is true. If the underlying device returns error
somehow, bio->bi_error has recorded that error.
If we could recover the correct data from another copy in profile raid1/10/5/6,
with btrfs_subio_endio_read() returning 0, bio would have the correct data in
its vector, but bio->bi_error is not updated accordingly so that the following
dio_end_io(dio_bio, bio->bi_error) makes directIO think this read has failed.
Thanks,
-liubo
>
> > This fixes the problem by setting bio's error to 0 if a good copy has been
> > found.
> >
> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > ---
> > fs/btrfs/inode.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index 632b616..4e1398e 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -8113,8 +8113,11 @@ static void btrfs_endio_direct_read(struct bio *bio)
> > struct btrfs_io_bio *io_bio = btrfs_io_bio(bio);
> > int err = bio->bi_error;
> >
> > - 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_error = 0;
> > + }
> >
> > unlock_extent(&BTRFS_I(inode)->io_tree, dip->logical_offset,
> > dip->logical_offset + dip->bytes - 1);
> > --
> > 2.5.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Btrfs: tolerate errors if we have retried successfully
2017-05-09 19:40 ` Liu Bo
@ 2017-05-15 15:57 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2017-05-15 15:57 UTC (permalink / raw)
To: Liu Bo; +Cc: dsterba, linux-btrfs
On Tue, May 09, 2017 at 12:40:53PM -0700, Liu Bo wrote:
> On Fri, May 05, 2017 at 06:52:45PM +0200, David Sterba wrote:
> > On Thu, Apr 13, 2017 at 06:11:56PM -0700, Liu Bo wrote:
> > > With raid1 profile, dio read isn't tolerating IO errors if read length is
> > > less than the stripe length (64K).
> >
> > Can you please write more details why this is true? Some pointers to
> > code etc, I'm lost. Eg. where the errors is tolerated. Thanks.
>
> Sure.
>
> Our bio didn't get split in btrfs_submit_direct_hook() if (dip->flags &
> BTRFS_DIO_ORIG_BIO_SUBMITTED) is true. If the underlying device returns error
> somehow, bio->bi_error has recorded that error.
>
> If we could recover the correct data from another copy in profile raid1/10/5/6,
> with btrfs_subio_endio_read() returning 0, bio would have the correct data in
> its vector, but bio->bi_error is not updated accordingly so that the following
> dio_end_io(dio_bio, bio->bi_error) makes directIO think this read has failed.
Great, thanks. Please update the patch and resend.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] Btrfs: tolerate errors if we have retried successfully
2017-04-14 1:11 [PATCH] Btrfs: tolerate errors if we have retried successfully Liu Bo
2017-05-05 16:52 ` David Sterba
@ 2017-05-17 21:42 ` Liu Bo
2017-06-06 13:42 ` David Sterba
1 sibling, 1 reply; 6+ messages in thread
From: Liu Bo @ 2017-05-17 21:42 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
With raid1 profile, dio read isn't tolerating IO errors if read length is
less than the stripe length (64K).
Our bio didn't get split in btrfs_submit_direct_hook() if (dip->flags &
BTRFS_DIO_ORIG_BIO_SUBMITTED) is true and that happens when the read
length is less than 64k. In this case, if the underlying device returns
error somehow, bio->bi_error has recorded that error.
If we could recover the correct data from another copy in profile raid1/10/5/6,
with btrfs_subio_endio_read() returning 0, bio would have the correct data in
its vector, but bio->bi_error is not updated accordingly so that the following
dio_end_io(dio_bio, bio->bi_error) makes directIO think this read has failed.
This fixes the problem by setting bio's error to 0 if a good copy has been
found.
Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
v2: Add more details to changelog.
fs/btrfs/inode.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 632b616..4e1398e 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8113,8 +8113,11 @@ static void btrfs_endio_direct_read(struct bio *bio)
struct btrfs_io_bio *io_bio = btrfs_io_bio(bio);
int err = bio->bi_error;
- 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_error = 0;
+ }
unlock_extent(&BTRFS_I(inode)->io_tree, dip->logical_offset,
dip->logical_offset + dip->bytes - 1);
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Btrfs: tolerate errors if we have retried successfully
2017-05-17 21:42 ` [PATCH v2] " Liu Bo
@ 2017-06-06 13:42 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2017-06-06 13:42 UTC (permalink / raw)
To: Liu Bo; +Cc: linux-btrfs, David Sterba
On Wed, May 17, 2017 at 03:42:00PM -0600, Liu Bo wrote:
> With raid1 profile, dio read isn't tolerating IO errors if read length is
> less than the stripe length (64K).
>
> Our bio didn't get split in btrfs_submit_direct_hook() if (dip->flags &
> BTRFS_DIO_ORIG_BIO_SUBMITTED) is true and that happens when the read
> length is less than 64k. In this case, if the underlying device returns
> error somehow, bio->bi_error has recorded that error.
>
> If we could recover the correct data from another copy in profile raid1/10/5/6,
> with btrfs_subio_endio_read() returning 0, bio would have the correct data in
> its vector, but bio->bi_error is not updated accordingly so that the following
> dio_end_io(dio_bio, bio->bi_error) makes directIO think this read has failed.
>
> This fixes the problem by setting bio's error to 0 if a good copy has been
> found.
>
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ---
>
> v2: Add more details to changelog.
Thanks.
Reviewed-by: David Sterba <dsterba@suse.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-06-06 13:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-14 1:11 [PATCH] Btrfs: tolerate errors if we have retried successfully Liu Bo
2017-05-05 16:52 ` David Sterba
2017-05-09 19:40 ` Liu Bo
2017-05-15 15:57 ` David Sterba
2017-05-17 21:42 ` [PATCH v2] " Liu Bo
2017-06-06 13:42 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).