Skip to content

Commit 2d192fc

Browse files
adam900710kdave
authored andcommitted
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 0a31daa ("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. CC: [email protected] # 5.4+ Signed-off-by: Qu Wenruo <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 27cdfde commit 2d192fc

File tree

1 file changed

+13
-0
lines changed

1 file changed

+13
-0
lines changed

fs/btrfs/block-group.c

+13
Original file line numberDiff line numberDiff line change
@@ -2544,6 +2544,19 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache,
25442544
int ret;
25452545
bool dirty_bg_running;
25462546

2547+
/*
2548+
* This can only happen when we are doing read-only scrub on read-only
2549+
* mount.
2550+
* In that case we should not start a new transaction on read-only fs.
2551+
* Thus here we skip all chunk allocations.
2552+
*/
2553+
if (sb_rdonly(fs_info->sb)) {
2554+
mutex_lock(&fs_info->ro_block_group_mutex);
2555+
ret = inc_block_group_ro(cache, 0);
2556+
mutex_unlock(&fs_info->ro_block_group_mutex);
2557+
return ret;
2558+
}
2559+
25472560
do {
25482561
trans = btrfs_join_transaction(root);
25492562
if (IS_ERR(trans))

0 commit comments

Comments
 (0)