Btrfs: enable repair during read for raid56 profile
diff mbox

Message ID 1490382815-25248-1-git-send-email-bo.li.liu@oracle.com
State New
Headers show

Commit Message

Liu Bo March 24, 2017, 7:13 p.m. UTC
Now that scrub can fix data errors with the help of parity for raid56
profile, repair during read is able to as well.

Although the mirror num in raid56 senario has different meanings, i.e.
0 or 1: read data directly
> 1:    do recover with parity,
it could be fit into how we repair bad block during read.

The trick is to use BTRFS_MAP_READ instead of BTRFS_MAP_WRITE to get the
device and position on it.

Cc: David Sterba <dsterba@suse.cz>
Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/extent_io.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

Comments

David Sterba March 27, 2017, 4:59 p.m. UTC | #1
On Fri, Mar 24, 2017 at 12:13:35PM -0700, Liu Bo wrote:
> Now that scrub can fix data errors with the help of parity for raid56
> profile, repair during read is able to as well.
> 
> Although the mirror num in raid56 senario has different meanings, i.e.

(typo: scenario)

> 0 or 1: read data directly
> > 1:    do recover with parity,
> it could be fit into how we repair bad block during read.

Could we possibly add some symbolic names for the RAID56 case?

> 
> The trick is to use BTRFS_MAP_READ instead of BTRFS_MAP_WRITE to get the
> device and position on it.

Please also document the trick in the code before the following.

> +	if (btrfs_is_parity_mirror(map_tree, logical, length, mirror_num)) {
> +		/* use BTRFS_MAP_READ to get the phy dev and sector */
> +		ret = btrfs_map_block(fs_info, BTRFS_MAP_READ, logical,
> +				      &map_length, &bbio, 0);
> +		if (ret) {
> +			btrfs_bio_counter_dec(fs_info);
> +			bio_put(bio);
> +			return -EIO;
> +		}
> +		ASSERT(bbio->mirror_num == 1);
> +	} else {
> +		ret = btrfs_map_block(fs_info, BTRFS_MAP_WRITE, logical,
> +				      &map_length, &bbio, mirror_num);
> +		if (ret) {
> +			btrfs_bio_counter_dec(fs_info);
> +			bio_put(bio);
> +			return -EIO;
> +		}
> +		BUG_ON(mirror_num != bbio->mirror_num);
>  	}
> -	BUG_ON(mirror_num != bbio->mirror_num);
> -	sector = bbio->stripes[mirror_num-1].physical >> 9;
> +
> +	sector = bbio->stripes[bbio->mirror_num - 1].physical >> 9;
>  	bio->bi_iter.bi_sector = sector;
> -	dev = bbio->stripes[mirror_num-1].dev;
> +	dev = bbio->stripes[bbio->mirror_num - 1].dev;
>  	btrfs_put_bbio(bbio);
>  	if (!dev || !dev->bdev || !dev->writeable) {
>  		btrfs_bio_counter_dec(fs_info);
--
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
Liu Bo March 29, 2017, 4:36 a.m. UTC | #2
On Mon, Mar 27, 2017 at 06:59:44PM +0200, David Sterba wrote:
> On Fri, Mar 24, 2017 at 12:13:35PM -0700, Liu Bo wrote:
> > Now that scrub can fix data errors with the help of parity for raid56
> > profile, repair during read is able to as well.
> > 
> > Although the mirror num in raid56 senario has different meanings, i.e.
> 
> (typo: scenario)
>

Thanks!

> > 0 or 1: read data directly
> > > 1:    do recover with parity,
> > it could be fit into how we repair bad block during read.
> 
> Could we possibly add some symbolic names for the RAID56 case?
>

Yes, we could do that here, but I'm afraid it might not be helpful
because most of places still use mirror or mirror_num, such as
btrfs_submit_bio_hook and btrfs_map_bio.

> > 
> > The trick is to use BTRFS_MAP_READ instead of BTRFS_MAP_WRITE to get the
> > device and position on it.
> 
> Please also document the trick in the code before the following.
>

Good point.

Thanks,

-liubo
> > +	if (btrfs_is_parity_mirror(map_tree, logical, length, mirror_num)) {
> > +		/* use BTRFS_MAP_READ to get the phy dev and sector */
> > +		ret = btrfs_map_block(fs_info, BTRFS_MAP_READ, logical,
> > +				      &map_length, &bbio, 0);
> > +		if (ret) {
> > +			btrfs_bio_counter_dec(fs_info);
> > +			bio_put(bio);
> > +			return -EIO;
> > +		}
> > +		ASSERT(bbio->mirror_num == 1);
> > +	} else {
> > +		ret = btrfs_map_block(fs_info, BTRFS_MAP_WRITE, logical,
> > +				      &map_length, &bbio, mirror_num);
> > +		if (ret) {
> > +			btrfs_bio_counter_dec(fs_info);
> > +			bio_put(bio);
> > +			return -EIO;
> > +		}
> > +		BUG_ON(mirror_num != bbio->mirror_num);
> >  	}
> > -	BUG_ON(mirror_num != bbio->mirror_num);
> > -	sector = bbio->stripes[mirror_num-1].physical >> 9;
> > +
> > +	sector = bbio->stripes[bbio->mirror_num - 1].physical >> 9;
> >  	bio->bi_iter.bi_sector = sector;
> > -	dev = bbio->stripes[mirror_num-1].dev;
> > +	dev = bbio->stripes[bbio->mirror_num - 1].dev;
> >  	btrfs_put_bbio(bbio);
> >  	if (!dev || !dev->bdev || !dev->writeable) {
> >  		btrfs_bio_counter_dec(fs_info);
--
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

Patch
diff mbox

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 1edb55d..0a44a43 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2009,10 +2009,6 @@  int repair_io_failure(struct btrfs_inode *inode, u64 start, u64 length,
 	ASSERT(!(fs_info->sb->s_flags & MS_RDONLY));
 	BUG_ON(!mirror_num);
 
-	/* we can't repair anything in raid56 yet */
-	if (btrfs_is_parity_mirror(map_tree, logical, length, mirror_num))
-		return 0;
-
 	bio = btrfs_io_bio_alloc(GFP_NOFS, 1);
 	if (!bio)
 		return -EIO;
@@ -2025,17 +2021,30 @@  int repair_io_failure(struct btrfs_inode *inode, u64 start, u64 length,
 	 * read repair operation.
 	 */
 	btrfs_bio_counter_inc_blocked(fs_info);
-	ret = btrfs_map_block(fs_info, BTRFS_MAP_WRITE, logical,
-			      &map_length, &bbio, mirror_num);
-	if (ret) {
-		btrfs_bio_counter_dec(fs_info);
-		bio_put(bio);
-		return -EIO;
+	if (btrfs_is_parity_mirror(map_tree, logical, length, mirror_num)) {
+		/* use BTRFS_MAP_READ to get the phy dev and sector */
+		ret = btrfs_map_block(fs_info, BTRFS_MAP_READ, logical,
+				      &map_length, &bbio, 0);
+		if (ret) {
+			btrfs_bio_counter_dec(fs_info);
+			bio_put(bio);
+			return -EIO;
+		}
+		ASSERT(bbio->mirror_num == 1);
+	} else {
+		ret = btrfs_map_block(fs_info, BTRFS_MAP_WRITE, logical,
+				      &map_length, &bbio, mirror_num);
+		if (ret) {
+			btrfs_bio_counter_dec(fs_info);
+			bio_put(bio);
+			return -EIO;
+		}
+		BUG_ON(mirror_num != bbio->mirror_num);
 	}
-	BUG_ON(mirror_num != bbio->mirror_num);
-	sector = bbio->stripes[mirror_num-1].physical >> 9;
+
+	sector = bbio->stripes[bbio->mirror_num - 1].physical >> 9;
 	bio->bi_iter.bi_sector = sector;
-	dev = bbio->stripes[mirror_num-1].dev;
+	dev = bbio->stripes[bbio->mirror_num - 1].dev;
 	btrfs_put_bbio(bbio);
 	if (!dev || !dev->bdev || !dev->writeable) {
 		btrfs_bio_counter_dec(fs_info);