* [PATCH] Btrfs: enable repair during read for raid56 profile
@ 2017-03-24 19:13 Liu Bo
2017-03-27 16:59 ` David Sterba
2017-03-29 17:53 ` [PATCH v2] " Liu Bo
0 siblings, 2 replies; 6+ messages in thread
From: Liu Bo @ 2017-03-24 19:13 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
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(-)
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);
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] Btrfs: enable repair during read for raid56 profile
2017-03-24 19:13 [PATCH] Btrfs: enable repair during read for raid56 profile Liu Bo
@ 2017-03-27 16:59 ` David Sterba
2017-03-29 4:36 ` Liu Bo
2017-03-29 17:53 ` [PATCH v2] " Liu Bo
1 sibling, 1 reply; 6+ messages in thread
From: David Sterba @ 2017-03-27 16:59 UTC (permalink / raw)
To: Liu Bo; +Cc: linux-btrfs, David Sterba
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);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Btrfs: enable repair during read for raid56 profile
2017-03-27 16:59 ` David Sterba
@ 2017-03-29 4:36 ` Liu Bo
0 siblings, 0 replies; 6+ messages in thread
From: Liu Bo @ 2017-03-29 4:36 UTC (permalink / raw)
To: dsterba, linux-btrfs
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);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] Btrfs: enable repair during read for raid56 profile
2017-03-24 19:13 [PATCH] Btrfs: enable repair during read for raid56 profile Liu Bo
2017-03-27 16:59 ` David Sterba
@ 2017-03-29 17:53 ` Liu Bo
2017-03-31 1:14 ` [PATCH v2] Btrfs: fix wrong failed mirror_num of read-repair on raid56 Qu Wenruo
1 sibling, 1 reply; 6+ messages in thread
From: Liu Bo @ 2017-03-29 17:53 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
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 scenario 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>
---
v2: Fix typo and add comments for why BTRFS_MAP_READ is used.
fs/btrfs/extent_io.c | 40 +++++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index d39a11c..7241f94 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,35 @@ 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)) {
+ /*
+ * Note that we don't use BTRFS_MAP_WRITE because it's supposed
+ * to update all raid stripes, but here we just want to correct
+ * bad stripe, thus BTRFS_MAP_READ is abused to only get the bad
+ * stripe's 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);
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] Btrfs: fix wrong failed mirror_num of read-repair on raid56
2017-03-29 17:53 ` [PATCH v2] " Liu Bo
@ 2017-03-31 1:14 ` Qu Wenruo
2017-03-31 1:21 ` Qu Wenruo
0 siblings, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2017-03-31 1:14 UTC (permalink / raw)
To: Liu Bo, linux-btrfs; +Cc: David Sterba
At 03/30/2017 01:54 AM, Liu Bo wrote:
> In raid56 scenario, after trying parity recovery, we didn't set
> mirror_num for btrfs_bio with failed mirror_num, hence
> end_bio_extent_readpage() will report a random mirror_num in dmesg
> log.
>
> Cc: David Sterba <dsterba@suse.cz>
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
Tested-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
The test case is btrfs/125, with compress=lzo.
Even with ordered_extent error handler enhancement, compress=lzo mount
option will cause test case to fail due to mismatch csum.
With these patches applied, btrfs/125 passws without error.
Thanks,
Qu
> ---
> v2: Set mirror_num inside raid56_parity_recover so that all callers
> can get a correct mirror_num if they need.
>
> fs/btrfs/raid56.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index 1571bf2..a333acc 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -2118,6 +2118,11 @@ int raid56_parity_recover(struct btrfs_fs_info *fs_info, struct bio *bio,
> struct btrfs_raid_bio *rbio;
> int ret;
>
> + if (generic_io) {
> + ASSERT(bbio->mirror_num == mirror_num);
> + btrfs_io_bio(bio)->mirror_num = mirror_num;
> + }
> +
> rbio = alloc_rbio(fs_info, bbio, stripe_len);
> if (IS_ERR(rbio)) {
> if (generic_io)
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] Btrfs: fix wrong failed mirror_num of read-repair on raid56
2017-03-31 1:14 ` [PATCH v2] Btrfs: fix wrong failed mirror_num of read-repair on raid56 Qu Wenruo
@ 2017-03-31 1:21 ` Qu Wenruo
0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2017-03-31 1:21 UTC (permalink / raw)
To: Liu Bo, linux-btrfs; +Cc: David Sterba
At 03/31/2017 09:14 AM, Qu Wenruo wrote:
>
>
> At 03/30/2017 01:54 AM, Liu Bo wrote:
>> In raid56 scenario, after trying parity recovery, we didn't set
>> mirror_num for btrfs_bio with failed mirror_num, hence
>> end_bio_extent_readpage() will report a random mirror_num in dmesg
>> log.
>>
>> Cc: David Sterba <dsterba@suse.cz>
>> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
>
> Tested-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>
> The test case is btrfs/125, with compress=lzo.
>
> Even with ordered_extent error handler enhancement, compress=lzo mount
> option will cause test case to fail due to mismatch csum.
>
> With these patches applied, btrfs/125 passws without error.
>
> Thanks,
> Qu
Errr, sorry wrong patch, it should be previous patch "Btrfs: enable
repair during read for raid56 profile".
Thanks,
Qu
>> ---
>> v2: Set mirror_num inside raid56_parity_recover so that all callers
>> can get a correct mirror_num if they need.
>>
>> fs/btrfs/raid56.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
>> index 1571bf2..a333acc 100644
>> --- a/fs/btrfs/raid56.c
>> +++ b/fs/btrfs/raid56.c
>> @@ -2118,6 +2118,11 @@ int raid56_parity_recover(struct btrfs_fs_info
>> *fs_info, struct bio *bio,
>> struct btrfs_raid_bio *rbio;
>> int ret;
>>
>> + if (generic_io) {
>> + ASSERT(bbio->mirror_num == mirror_num);
>> + btrfs_io_bio(bio)->mirror_num = mirror_num;
>> + }
>> +
>> rbio = alloc_rbio(fs_info, bbio, stripe_len);
>> if (IS_ERR(rbio)) {
>> if (generic_io)
>>
>
>
> --
> 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
end of thread, other threads:[~2017-03-31 1:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-24 19:13 [PATCH] Btrfs: enable repair during read for raid56 profile Liu Bo
2017-03-27 16:59 ` David Sterba
2017-03-29 4:36 ` Liu Bo
2017-03-29 17:53 ` [PATCH v2] " Liu Bo
2017-03-31 1:14 ` [PATCH v2] Btrfs: fix wrong failed mirror_num of read-repair on raid56 Qu Wenruo
2017-03-31 1:21 ` Qu Wenruo
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).