Patch "btrfs: raid56: always verify the P/Q contents for scrub" has been added to the 6.4-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    btrfs: raid56: always verify the P/Q contents for scrub

to the 6.4-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     btrfs-raid56-always-verify-the-p-q-contents-for-scrub.patch
and it can be found in the queue-6.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.


>From 486c737f7fdc0c3f6464cf27ede811daec2769a1 Mon Sep 17 00:00:00 2001
From: Qu Wenruo <wqu@xxxxxxxx>
Date: Fri, 30 Jun 2023 08:56:40 +0800
Subject: btrfs: raid56: always verify the P/Q contents for scrub

From: Qu Wenruo <wqu@xxxxxxxx>

commit 486c737f7fdc0c3f6464cf27ede811daec2769a1 upstream.

[REGRESSION]
Commit 75b470332965 ("btrfs: raid56: migrate recovery and scrub recovery
path to use error_bitmap") changed the behavior of scrub_rbio().

Initially if we have no error reading the raid bio, we will assign
@need_check to true, then finish_parity_scrub() would later verify the
content of P/Q stripes before writeback.

But after that commit we never verify the content of P/Q stripes and
just writeback them.

This can lead to unrepaired P/Q stripes during scrub, or already
corrupted P/Q copied to the dev-replace target.

[FIX]
The situation is more complex than the regression, in fact the initial
behavior is not 100% correct either.

If we have the following rare case, it can still lead to the same
problem using the old behavior:

		0	16K	32K	48K	64K
	Data 1:	|IIIIIII|                       |
	Data 2:	|				|
	Parity:	|	|CCCCCCC|		|

Where "I" means IO error, "C" means corruption.

In the above case, we're scrubbing the parity stripe, then read out all
the contents of Data 1, Data 2, Parity stripes.

But found IO error in Data 1, which leads to rebuild using Data 2 and
Parity and got the correct data.

In that case, we would not verify if the Parity is correct for range
[16K, 32K).

So here we have to always verify the content of Parity no matter if we
did recovery or not.

This patch would remove the @need_check parameter of
finish_parity_scrub() completely, and would always do the P/Q
verification before writeback.

Fixes: 75b470332965 ("btrfs: raid56: migrate recovery and scrub recovery path to use error_bitmap")
CC: stable@xxxxxxxxxxxxxxx # 6.2+
Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
Signed-off-by: David Sterba <dsterba@xxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
 fs/btrfs/raid56.c |   11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -71,7 +71,7 @@ static void rmw_rbio_work_locked(struct
 static void index_rbio_pages(struct btrfs_raid_bio *rbio);
 static int alloc_rbio_pages(struct btrfs_raid_bio *rbio);
 
-static int finish_parity_scrub(struct btrfs_raid_bio *rbio, int need_check);
+static int finish_parity_scrub(struct btrfs_raid_bio *rbio);
 static void scrub_rbio_work_locked(struct work_struct *work);
 
 static void free_raid_bio_pointers(struct btrfs_raid_bio *rbio)
@@ -2404,7 +2404,7 @@ static int alloc_rbio_essential_pages(st
 	return 0;
 }
 
-static int finish_parity_scrub(struct btrfs_raid_bio *rbio, int need_check)
+static int finish_parity_scrub(struct btrfs_raid_bio *rbio)
 {
 	struct btrfs_io_context *bioc = rbio->bioc;
 	const u32 sectorsize = bioc->fs_info->sectorsize;
@@ -2445,9 +2445,6 @@ static int finish_parity_scrub(struct bt
 	 */
 	clear_bit(RBIO_CACHE_READY_BIT, &rbio->flags);
 
-	if (!need_check)
-		goto writeback;
-
 	p_sector.page = alloc_page(GFP_NOFS);
 	if (!p_sector.page)
 		return -ENOMEM;
@@ -2516,7 +2513,6 @@ static int finish_parity_scrub(struct bt
 		q_sector.page = NULL;
 	}
 
-writeback:
 	/*
 	 * time to start writing.  Make bios for everything from the
 	 * higher layers (the bio_list in our rbio) and our p/q.  Ignore
@@ -2699,7 +2695,6 @@ static int scrub_assemble_read_bios(stru
 
 static void scrub_rbio(struct btrfs_raid_bio *rbio)
 {
-	bool need_check = false;
 	int sector_nr;
 	int ret;
 
@@ -2722,7 +2717,7 @@ static void scrub_rbio(struct btrfs_raid
 	 * We have every sector properly prepared. Can finish the scrub
 	 * and writeback the good content.
 	 */
-	ret = finish_parity_scrub(rbio, need_check);
+	ret = finish_parity_scrub(rbio);
 	wait_event(rbio->io_wait, atomic_read(&rbio->stripes_pending) == 0);
 	for (sector_nr = 0; sector_nr < rbio->stripe_nsectors; sector_nr++) {
 		int found_errors;


Patches currently in stable-queue which might be from wqu@xxxxxxxx are

queue-6.4/btrfs-raid56-always-verify-the-p-q-contents-for-scrub.patch
queue-6.4/btrfs-fix-warning-when-putting-transaction-with-qgroups-enabled-after-abort.patch



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux