linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Nikolay Borisov <nborisov@suse.com>,
	Johannes Thumshirn <johannes.thumshirn@wdc.com>,
	Filipe Manana <fdmanana@suse.com>,
	David Sterba <dsterba@suse.com>
Subject: [PATCH 5.4 01/57] btrfs: fix filesystem corruption after a device replace
Date: Mon,  5 Oct 2020 17:26:13 +0200	[thread overview]
Message-ID: <20201005142109.863987507@linuxfoundation.org> (raw)
In-Reply-To: <20201005142109.796046410@linuxfoundation.org>

From: Filipe Manana <fdmanana@suse.com>

commit 4c8f353272dd1262013873990c0fafd0e3c8f274 upstream.

We use a device's allocation state tree to track ranges in a device used
for allocated chunks, and we set ranges in this tree when allocating a new
chunk. However after a device replace operation, we were not setting the
allocated ranges in the new device's allocation state tree, so that tree
is empty after a device replace.

This means that a fitrim operation after a device replace will trim the
device ranges that have allocated chunks and extents, as we trim every
range for which there is not a range marked in the device's allocation
state tree. It is also important during chunk allocation, since the
device's allocation state is used to determine if a range is already
allocated when allocating a new chunk.

This is trivial to reproduce and the following script triggers the bug:

  $ cat reproducer.sh
  #!/bin/bash

  DEV1="/dev/sdg"
  DEV2="/dev/sdh"
  DEV3="/dev/sdi"

  wipefs -a $DEV1 $DEV2 $DEV3 &> /dev/null

  # Create a raid1 test fs on 2 devices.
  mkfs.btrfs -f -m raid1 -d raid1 $DEV1 $DEV2 > /dev/null
  mount $DEV1 /mnt/btrfs

  xfs_io -f -c "pwrite -S 0xab 0 10M" /mnt/btrfs/foo

  echo "Starting to replace $DEV1 with $DEV3"
  btrfs replace start -B $DEV1 $DEV3 /mnt/btrfs
  echo

  echo "Running fstrim"
  fstrim /mnt/btrfs
  echo

  echo "Unmounting filesystem"
  umount /mnt/btrfs

  echo "Mounting filesystem in degraded mode using $DEV3 only"
  wipefs -a $DEV1 $DEV2 &> /dev/null
  mount -o degraded $DEV3 /mnt/btrfs
  if [ $? -ne 0 ]; then
          dmesg | tail
          echo
          echo "Failed to mount in degraded mode"
          exit 1
  fi

  echo
  echo "File foo data (expected all bytes = 0xab):"
  od -A d -t x1 /mnt/btrfs/foo

  umount /mnt/btrfs

When running the reproducer:

  $ ./replace-test.sh
  wrote 10485760/10485760 bytes at offset 0
  10 MiB, 2560 ops; 0.0901 sec (110.877 MiB/sec and 28384.5216 ops/sec)
  Starting to replace /dev/sdg with /dev/sdi

  Running fstrim

  Unmounting filesystem
  Mounting filesystem in degraded mode using /dev/sdi only
  mount: /mnt/btrfs: wrong fs type, bad option, bad superblock on /dev/sdi, missing codepage or helper program, or other error.
  [19581.748641] BTRFS info (device sdg): dev_replace from /dev/sdg (devid 1) to /dev/sdi started
  [19581.803842] BTRFS info (device sdg): dev_replace from /dev/sdg (devid 1) to /dev/sdi finished
  [19582.208293] BTRFS info (device sdi): allowing degraded mounts
  [19582.208298] BTRFS info (device sdi): disk space caching is enabled
  [19582.208301] BTRFS info (device sdi): has skinny extents
  [19582.212853] BTRFS warning (device sdi): devid 2 uuid 1f731f47-e1bb-4f00-bfbb-9e5a0cb4ba9f is missing
  [19582.213904] btree_readpage_end_io_hook: 25839 callbacks suppressed
  [19582.213907] BTRFS error (device sdi): bad tree block start, want 30490624 have 0
  [19582.214780] BTRFS warning (device sdi): failed to read root (objectid=7): -5
  [19582.231576] BTRFS error (device sdi): open_ctree failed

  Failed to mount in degraded mode

So fix by setting all allocated ranges in the replace target device when
the replace operation is finishing, when we are holding the chunk mutex
and we can not race with new chunk allocations.

A test case for fstests follows soon.

Fixes: 1c11b63eff2a67 ("btrfs: replace pending/pinned chunks lists with io tree")
CC: stable@vger.kernel.org # 5.2+
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 fs/btrfs/dev-replace.c |   40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -562,6 +562,37 @@ static void btrfs_rm_dev_replace_unblock
 	wake_up(&fs_info->dev_replace.replace_wait);
 }
 
+/*
+ * When finishing the device replace, before swapping the source device with the
+ * target device we must update the chunk allocation state in the target device,
+ * as it is empty because replace works by directly copying the chunks and not
+ * through the normal chunk allocation path.
+ */
+static int btrfs_set_target_alloc_state(struct btrfs_device *srcdev,
+					struct btrfs_device *tgtdev)
+{
+	struct extent_state *cached_state = NULL;
+	u64 start = 0;
+	u64 found_start;
+	u64 found_end;
+	int ret = 0;
+
+	lockdep_assert_held(&srcdev->fs_info->chunk_mutex);
+
+	while (!find_first_extent_bit(&srcdev->alloc_state, start,
+				      &found_start, &found_end,
+				      CHUNK_ALLOCATED, &cached_state)) {
+		ret = set_extent_bits(&tgtdev->alloc_state, found_start,
+				      found_end, CHUNK_ALLOCATED);
+		if (ret)
+			break;
+		start = found_end + 1;
+	}
+
+	free_extent_state(cached_state);
+	return ret;
+}
+
 static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
 				       int scrub_ret)
 {
@@ -636,8 +667,14 @@ static int btrfs_dev_replace_finishing(s
 	dev_replace->time_stopped = ktime_get_real_seconds();
 	dev_replace->item_needs_writeback = 1;
 
-	/* replace old device with new one in mapping tree */
+	/*
+	 * Update allocation state in the new device and replace the old device
+	 * with the new one in the mapping tree.
+	 */
 	if (!scrub_ret) {
+		scrub_ret = btrfs_set_target_alloc_state(src_device, tgt_device);
+		if (scrub_ret)
+			goto error;
 		btrfs_dev_replace_update_device_in_mapping_tree(fs_info,
 								src_device,
 								tgt_device);
@@ -648,6 +685,7 @@ static int btrfs_dev_replace_finishing(s
 				 btrfs_dev_name(src_device),
 				 src_device->devid,
 				 rcu_str_deref(tgt_device->name), scrub_ret);
+error:
 		up_write(&dev_replace->rwsem);
 		mutex_unlock(&fs_info->chunk_mutex);
 		mutex_unlock(&fs_info->fs_devices->device_list_mutex);



  reply	other threads:[~2020-10-05 15:28 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-05 15:26 [PATCH 5.4 00/57] 5.4.70-rc1 review Greg Kroah-Hartman
2020-10-05 15:26 ` Greg Kroah-Hartman [this message]
2020-10-05 15:26 ` [PATCH 5.4 02/57] mmc: sdhci: Workaround broken command queuing on Intel GLK based IRBIS models Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 03/57] USB: gadget: f_ncm: Fix NDP16 datagram validation Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 04/57] gpio: siox: explicitly support only threaded irqs Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 05/57] gpio: mockup: fix resource leak in error path Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 06/57] gpio: tc35894: fix up tc35894 interrupt configuration Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 07/57] clk: socfpga: stratix10: fix the divider for the emac_ptp_free_clk Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 08/57] vsock/virtio: add transport parameter to the virtio_transport_reset_no_sock() Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 09/57] net: virtio_vsock: Enhance connection semantics Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 10/57] xfs: trim IO to found COW extent limit Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 11/57] Input: i8042 - add nopnp quirk for Acer Aspire 5 A515 Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 12/57] iio: adc: qcom-spmi-adc5: fix driver name Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 13/57] ftrace: Move RCU is watching check after recursion check Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 14/57] memstick: Skip allocating card when removing host Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 15/57] drm/amdgpu: restore proper ref count in amdgpu_display_crtc_set_config Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 16/57] clocksource/drivers/timer-gx6605s: Fixup counter reload Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 17/57] libbpf: Remove arch-specific include path in Makefile Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 18/57] drivers/net/wan/hdlc_fr: Add needed_headroom for PVC devices Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 19/57] drm/sun4i: mixer: Extend regmap max_register Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 20/57] net: dec: de2104x: Increase receive ring size for Tulip Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 21/57] rndis_host: increase sleep time in the query-response loop Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 22/57] nvme-core: get/put ctrl and transport module in nvme_dev_open/release() Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 23/57] fuse: fix the ->direct_IO() treatment of iov_iter Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 24/57] drivers/net/wan/lapbether: Make skb->protocol consistent with the header Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 25/57] drivers/net/wan/hdlc: Set skb->protocol before transmitting Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 26/57] mac80211: Fix radiotap header channel flag for 6GHz band Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 27/57] mac80211: do not allow bigger VHT MPDUs than the hardware supports Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 28/57] tracing: Make the space reserved for the pid wider Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 29/57] tools/io_uring: fix compile breakage Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 30/57] spi: fsl-espi: Only process interrupts for expected events Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 31/57] nvme-pci: fix NULL req in completion handler Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 32/57] nvme-fc: fail new connections to a deleted host or remote port Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 33/57] gpio: sprd: Clear interrupt when setting the type as edge Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 34/57] phy: ti: am654: Fix a leak in serdes_am654_probe() Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 35/57] pinctrl: mvebu: Fix i2c sda definition for 98DX3236 Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 36/57] nfs: Fix security label length not being reset Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 37/57] clk: tegra: Always program PLL_E when enabled Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 38/57] clk: samsung: exynos4: mark chipid clock as CLK_IGNORE_UNUSED Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 39/57] iommu/exynos: add missing put_device() call in exynos_iommu_of_xlate() Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 40/57] gpio/aspeed-sgpio: enable access to all 80 input & output sgpios Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 41/57] gpio/aspeed-sgpio: dont enable all interrupts by default Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 42/57] gpio: aspeed: fix ast2600 bank properties Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 43/57] i2c: cpm: Fix i2c_ram structure Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 44/57] Input: trackpoint - enable Synaptics trackpoints Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 45/57] scripts/dtc: only append to HOST_EXTRACFLAGS instead of overwriting Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 46/57] random32: Restore __latent_entropy attribute on net_rand_state Greg Kroah-Hartman
2020-10-05 15:26 ` [PATCH 5.4 47/57] block/diskstats: more accurate approximation of io_ticks for slow disks Greg Kroah-Hartman
2020-10-05 15:27 ` [PATCH 5.4 48/57] mm: replace memmap_context by meminit_context Greg Kroah-Hartman
2020-10-05 15:27 ` [PATCH 5.4 49/57] mm: dont rely on system state to detect hot-plug operations Greg Kroah-Hartman
2020-10-05 15:27 ` [PATCH 5.4 50/57] nvme: Cleanup and rename nvme_block_nr() Greg Kroah-Hartman
2020-10-05 15:27 ` [PATCH 5.4 51/57] nvme: Introduce nvme_lba_to_sect() Greg Kroah-Hartman
2020-10-05 15:27 ` [PATCH 5.4 52/57] nvme: consolidate chunk_sectors settings Greg Kroah-Hartman
2020-10-05 15:27 ` [PATCH 5.4 53/57] epoll: do not insert into poll queues until all sanity checks are done Greg Kroah-Hartman
2020-10-05 15:27 ` [PATCH 5.4 54/57] epoll: replace ->visited/visited_list with generation count Greg Kroah-Hartman
2020-10-05 15:27 ` [PATCH 5.4 55/57] epoll: EPOLL_CTL_ADD: close the race in decision to take fast path Greg Kroah-Hartman
2020-10-05 15:27 ` [PATCH 5.4 56/57] ep_create_wakeup_source(): dentry name can change under you Greg Kroah-Hartman
2020-10-05 15:27 ` [PATCH 5.4 57/57] netfilter: ctnetlink: add a range check for l3/l4 protonum Greg Kroah-Hartman
2020-10-06  0:22 ` [PATCH 5.4 00/57] 5.4.70-rc1 review Shuah Khan
2020-10-06  5:54 ` Naresh Kamboju
2020-10-06  8:25   ` Naresh Kamboju
2020-10-06 14:49     ` Ben Hutchings
2020-10-06 18:17 ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201005142109.863987507@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=dsterba@suse.com \
    --cc=fdmanana@suse.com \
    --cc=johannes.thumshirn@wdc.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nborisov@suse.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).