Skip to contentSkip to conversation

FAILED: patch "[PATCH] btrfs: don't start transaction for scrub if the fs is mounted" failed to apply to 5.10-stable tree

PATCH [email protected] 1 participant 1 message Feb 5, 2022

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 2d192fc4c1abeb0d04d1c8cd54405ff4a0b0255b Mon Sep 17 00:00:00 2001
From: Qu Wenruo <[email protected]>
Date: Thu, 16 Dec 2021 19:47:35 +0800
Subject: [PATCH] btrfs: don't start transaction for scrub if the fs is mounted
read-only

[BUG]
The following super simple script would crash btrfs at unmount time, if CONFIG_BTRFS_ASSERT() is set.

mkfs.btrfs -f $dev
mount $dev $mnt
xfs_io -f -c "pwrite 0 4k" $mnt/file
umount $mnt
mount -r ro $dev $mnt
btrfs scrub start -Br $mnt
umount $mnt

This will trigger the following ASSERT() introduced by commit 0a31daa4b602 ("btrfs: add assertion for empty list of transactions at late stage of umount").

That patch is definitely not the cause, it just makes enough noise for developers.

[CAUSE]
We will start transaction for the following call chain during scrub:

scrub_enumerate_chunks()
|- btrfs_inc_block_group_ro()
|- btrfs_join_transaction()

However for RO mount, there is no running transaction at all, thus btrfs_join_transaction() will start a new transaction.

Furthermore, since it's read-only mount, btrfs_sync_fs() will not call btrfs_commit_super() to commit the new but empty transaction.

And leads to the ASSERT().

The bug has been there for a long time. Only the new ASSERT() makes it noisy enough to be noticed.

[FIX]
For read-only scrub on read-only mount, there is no need to start a transaction nor to allocate new chunks in btrfs_inc_block_group_ro().

Just do extra read-only mount check in btrfs_inc_block_group_ro(), and if it's read-only, skip all chunk allocation and go inc_block_group_ro() directly.

fs/btrfs/block-group.c +13 -0
index 1db24e6d6d90..68feabc83a27 100644
@@ -2544,6 +2544,19 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache,
 	int ret;
 	bool dirty_bg_running;
 
+	/*
+	 * This can only happen when we are doing read-only scrub on read-only
+	 * mount.
+	 * In that case we should not start a new transaction on read-only fs.
+	 * Thus here we skip all chunk allocations.
+	 */
+	if (sb_rdonly(fs_info->sb)) {
+		mutex_lock(&fs_info->ro_block_group_mutex);
+		ret = inc_block_group_ro(cache, 0);
+		mutex_unlock(&fs_info->ro_block_group_mutex);
+		return ret;
+	}
+
 	do {
 		trans = btrfs_join_transaction(root);
 		if (IS_ERR(trans))