From: Qu Wenruo wqu@suse.com
commit 2d192fc4c1abeb0d04d1c8cd54405ff4a0b0255b upstream.
[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.
CC: stable@vger.kernel.org # 5.4+ Signed-off-by: Qu Wenruo wqu@suse.com Signed-off-by: David Sterba dsterba@suse.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- fs/btrfs/block-group.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
--- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c @@ -2547,6 +2547,19 @@ int btrfs_inc_block_group_ro(struct btrf 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(fs_info->extent_root); if (IS_ERR(trans))