Revert "btrfs: scrub: use larger block size for data extent scrub"
authorQu Wenruo <wqu@suse.com>
Sun, 6 Nov 2022 23:23:26 +0000 (07:23 +0800)
committerDavid Sterba <dsterba@suse.com>
Mon, 7 Nov 2022 13:35:14 +0000 (14:35 +0100)
This reverts commit 786672e9e1a39a231806313e3c445c236588ceef.

[BUG]
Since commit 786672e9e1a3 ("btrfs: scrub: use larger block size for data
extent scrub"), btrfs scrub no longer reports errors if the corruption
is not in the first sector of a STRIPE_LEN.

The following script can expose the problem:

  mkfs.btrfs -f $dev
  mount $dev $mnt
  xfs_io -f -c "pwrite -S 0xff 0 8k" $mnt/foobar
  umount $mnt

  # 13631488 is the logical bytenr of above 8K extent
  btrfs-map-logical -l 13631488 -b 4096 $dev
  mirror 1 logical 13631488 physical 13631488 device /dev/test/scratch1

  # Corrupt the 2nd sector of that extent
  xfs_io -f -c "pwrite -S 0x00 13635584 4k" $dev

  mount $dev $mnt
  btrfs scrub start -B $mnt
  scrub done for 54e63f9f-0c30-4c84-a33b-5c56014629b7
  Scrub started:    Mon Nov  7 07:18:27 2022
  Status:           finished
  Duration:         0:00:00
  Total to scrub:   536.00MiB
  Rate:             0.00B/s
  Error summary:    no errors found <<<

[CAUSE]
That offending commit enlarges the data extent scrub size from sector
size to BTRFS_STRIPE_LEN, to avoid extra scrub_block to be allocated.

But unfortunately the data extent scrub is still heavily relying on the
fact that there is only one scrub_sector per scrub_block.

Thus it will only check the first sector, and ignoring the remaining
sectors.

Furthermore the error reporting is not able to handle multiple sectors
either.

[FIX]
For now just revert the offending commit.

The consequence is just extra memory usage during scrub.
We will need a proper change to make the remaining data scrub path to
handle multiple sectors before we enlarging the data scrub size.

Reported-by: Li Zhang <zhanglikernel@gmail.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/scrub.c

index f260c53..b659c67 100644 (file)
@@ -2672,17 +2672,11 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
        u8 csum[BTRFS_CSUM_SIZE];
        u32 blocksize;
 
-       /*
-        * Block size determines how many scrub_block will be allocated.  Here
-        * we use BTRFS_STRIPE_LEN (64KiB) as default limit, so we won't
-        * allocate too many scrub_block, while still won't cause too large
-        * bios for large extents.
-        */
        if (flags & BTRFS_EXTENT_FLAG_DATA) {
                if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK)
                        blocksize = map->stripe_len;
                else
-                       blocksize = BTRFS_STRIPE_LEN;
+                       blocksize = sctx->fs_info->sectorsize;
                spin_lock(&sctx->stat_lock);
                sctx->stat.data_extents_scrubbed++;
                sctx->stat.data_bytes_scrubbed += len;