btrfs: do not abort transaction on failure to write log tree when syncing log
authorFilipe Manana <fdmanana@suse.com>
Tue, 10 Jan 2023 14:56:37 +0000 (14:56 +0000)
committerDavid Sterba <dsterba@suse.com>
Thu, 12 Jan 2023 14:43:30 +0000 (15:43 +0100)
When syncing the log, if we fail to write log tree extent buffers, we mark
the log for a full commit and abort the transaction. However we don't need
to abort the transaction, all we really need to do is to make sure no one
can commit a superblock pointing to new log tree roots. Just because we
got a failure writing extent buffers for a log tree, it does not mean we
will also fail to do a transaction commit.

One particular case is if due to a bug somewhere, when writing log tree
extent buffers, the tree checker detects some corruption and the writeout
fails because of that. Aborting the transaction can be very disruptive for
a user, specially if the issue happened on a root filesystem. One example
is the scenario in the Link tag below, where an isolated corruption on log
tree leaves was causing transaction aborts when syncing the log.

Link: https://lore.kernel.org/linux-btrfs/ae169fc6-f504-28f0-a098-6fa6a4dfb612@leemhuis.info/
CC: stable@vger.kernel.org # 5.15+
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/disk-io.c
fs/btrfs/tree-log.c

index 8aeaada1fcaec2a9de8128cac61d29fafef5210c..3aa04224315eb383f3b61d8a278b02e68e01bb56 100644 (file)
@@ -367,7 +367,14 @@ error:
        btrfs_print_tree(eb, 0);
        btrfs_err(fs_info, "block=%llu write time tree block corruption detected",
                  eb->start);
-       WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG));
+       /*
+        * Be noisy if this is an extent buffer from a log tree. We don't abort
+        * a transaction in case there's a bad log tree extent buffer, we just
+        * fallback to a transaction commit. Still we want to know when there is
+        * a bad log tree extent buffer, as that may signal a bug somewhere.
+        */
+       WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG) ||
+               btrfs_header_owner(eb) == BTRFS_TREE_LOG_OBJECTID);
        return ret;
 }
 
index afad44a0becfe9e5becea4fbbb63d58c5c147d36..1f70d4ebffae4be3f5487386d9eff12864d95444 100644 (file)
@@ -2980,7 +2980,6 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
                ret = 0;
        if (ret) {
                blk_finish_plug(&plug);
-               btrfs_abort_transaction(trans, ret);
                btrfs_set_log_full_commit(trans);
                mutex_unlock(&root->log_mutex);
                goto out;
@@ -3112,7 +3111,6 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
                goto out_wake_log_root;
        } else if (ret) {
                btrfs_set_log_full_commit(trans);
-               btrfs_abort_transaction(trans, ret);
                mutex_unlock(&log_root_tree->log_mutex);
                goto out_wake_log_root;
        }