| From 16199ad9eb6db60a6b10794a09fc1ac6d09312ff Mon Sep 17 00:00:00 2001 |
| From: Filipe Manana <fdmanana@suse.com> |
| Date: Tue, 10 Jan 2023 14:56:37 +0000 |
| Subject: btrfs: do not abort transaction on failure to write log tree when syncing log |
| |
| From: Filipe Manana <fdmanana@suse.com> |
| |
| commit 16199ad9eb6db60a6b10794a09fc1ac6d09312ff upstream. |
| |
| 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> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| fs/btrfs/disk-io.c | 9 ++++++++- |
| fs/btrfs/tree-log.c | 2 -- |
| 2 files changed, 8 insertions(+), 3 deletions(-) |
| |
| --- a/fs/btrfs/disk-io.c |
| +++ b/fs/btrfs/disk-io.c |
| @@ -344,7 +344,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; |
| } |
| |
| --- a/fs/btrfs/tree-log.c |
| +++ b/fs/btrfs/tree-log.c |
| @@ -3011,7 +3011,6 @@ int btrfs_sync_log(struct btrfs_trans_ha |
| 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; |
| @@ -3143,7 +3142,6 @@ int btrfs_sync_log(struct btrfs_trans_ha |
| 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; |
| } |