On 11/02/2017 08:54 AM, Liu Bo wrote:
With raid6 profile, btrfs can end up with data corruption by the
following steps.
Say we have a 5 disks that are set up with raid6 profile,
1) mount this btrfs
2) one disk gets pulled out
3) write something to btrfs and sync
4) another disk gets pulled out
5) write something to btrfs and sync
6) umount this btrfs
7) bring the two disk back
8) reboot
9) mount this btrfs
Chances are mount will fail (even with -odegraded) because of failure
on reading metadata blocks, IOW, our raid6 setup is not able to
protect two disk failures in some cases.
So it turns out that there is a bug in raid6's recover code, that is,
if we have one of stripes in the raid6 layout as shown here,
| D1(*) | D2(*) | D3 | D4 | D5 |
-------------------------------------
| data1 | data2 | P | Q | data0 |
D1 and D2 are the two disks which got pulled out and brought back.
When mount reads data1 and finds out that data1 doesn't match its crc,
btrfs goes to recover data1 from other stripes, what it'll be doing is
1) read data2, parity P, parity Q and data0
2) as we have a valid parity P and two data stripes, it goes to
recover in raid5 style.
(However, since disk D2 was pulled out and data2 on D2 could be stale,
data2 should end up crc error, if not then raid5 recover is as
expected OR this example is confusing to explain the context of
two data stipe missing.
Thanks, Anand
we still get the wrong data1 from this reconstruction.)
3) btrfs continues to try to reconstruct data1 from parity Q, data2
and data0, we still get the wrong one for the same reason.
The fix here is to take advantage of the device flag, ie. 'In_sync',
all data on a device might be stale if 'In_sync' has not been set.
>
With this, we can build the correct data2 from parity P, Q and data1.
Signed-off-by: Liu Bo <bo.li.liu@xxxxxxxxxx>
---
fs/btrfs/raid56.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index 67262f8..3c0ce61 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -1076,7 +1076,8 @@ static int rbio_add_io_page(struct btrfs_raid_bio *rbio,
disk_start = stripe->physical + (page_index << PAGE_SHIFT);
/* if the device is missing, just fail this stripe */
- if (!stripe->dev->bdev)
+ if (!stripe->dev->bdev ||
+ !test_bit(In_sync, &stripe->dev->flags))
return fail_rbio_index(rbio, stripe_nr);
/* see if we can add this page onto our existing bio */
--
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
