FAILED: patch "[PATCH] btrfs: remove BUG() after failure to insert delayed dir index" failed to apply to 5.10-stable tree
Subsystems (2)
[email protected]
maintainerThe 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]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x 2c58c3931ede7cd08cbecf1f1a4acaf0a04a41a9
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2023092027-census-monorail-5614@gregkh' --subject-prefix 'PATCH 5.10.y' HEAD^..
Possible dependencies:
2c58c3931ede ("btrfs: remove BUG() after failure to insert delayed dir index item") 91bfe3104b8d ("btrfs: improve error message after failure to add delayed dir index item") 763748b238ef ("btrfs: reduce amount of reserved metadata for delayed item insertion") c9d02ab4b436 ("btrfs: set delayed item type when initializing it") 3bae13e9d42e ("btrfs: do not BUG_ON() on failure to reserve metadata for delayed item") a176affe547c ("btrfs: assert that delayed item is a dir index item when adding it") b7ef5f3a6f37 ("btrfs: loop only once over data sizes array when inserting an item batch") 086dcbfa50d3 ("btrfs: insert items in batches when logging a directory when possible") eb10d85ee77f ("btrfs: factor out the copying loop of dir items from log_dir_items()") 289cffcb0399 ("btrfs: remove no longer needed checks for NULL log context") 5a656c3628b2 ("btrfs: stop doing GFP_KERNEL memory allocations in the ref verify tool") 506650dcb3a7 ("btrfs: improve the batch insertion of delayed items") bfaa324e9a80 ("btrfs: remove total_data_size variable in btrfs_batch_insert_items()") bb385bedded3 ("btrfs: fix error handling in __btrfs_update_delayed_inode") 64d6b281ba4d ("btrfs: remove unnecessary check_parent_dirs_for_sync()") 3e6a86a193b0 ("btrfs: skip logging directories already logged when logging all parents") ab12313a9f56 ("btrfs: avoid logging new ancestor inodes when logging new inode") 47d3db41e190 ("btrfs: fix race that makes inode logging fallback to transaction commit") 4d6221d7d831 ("btrfs: fix race that causes unnecessary logging of ancestor inodes") 5893dfb98f25 ("btrfs: refactor btrfs_drop_extents() to make it easier to extend")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 2c58c3931ede7cd08cbecf1f1a4acaf0a04a41a9 Mon Sep 17 00:00:00 2001
From: Filipe Manana <[email protected]>
Date: Mon, 28 Aug 2023 09:06:43 +0100
Subject: [PATCH] btrfs: remove BUG() after failure to insert delayed dir index
item
Instead of calling BUG() when we fail to insert a delayed dir index item into the delayed node's tree, we can just release all the resources we have allocated/acquired before and return the error to the caller. This is fine because all existing call chains undo anything they have done before calling btrfs_insert_delayed_dir_index() or BUG_ON (when creating pending snapshots in the transaction commit path).
So remove the BUG() call and do proper error handling.
This relates to a syzbot report linked below, but does not fix it because it only prevents hitting a BUG(), it does not fix the issue where somehow we attempt to use twice the same index number for different index items.
fs/btrfs/delayed-inode.c +47 -27
@@ -1426,7 +1426,29 @@ void btrfs_balance_delayed_items(struct btrfs_fs_info *fs_info)btrfs_wq_run_delayed_node (delayed_root, fs_info,BTRFS_DELAYED_BATCH ); } -/* Will return 0 or -ENOMEM */ +static void btrfs_release_dir_index_item_space (struct btrfs_trans_handle *trans) +{ +struct btrfs_fs_info *fs_info = trans->fs_info; +const u64 bytes =btrfs_calc_insert_metadata_size (fs_info,1 ); + +if (test_bit (BTRFS_FS_LOG_RECOVERING , &fs_info->flags)) +return ; + +/* +* Adding the new dir index item does not require touching another +* leaf, so we can release 1 unit of metadata that was previously +* reserved when starting the transaction. This applies only to +* the case where we had a transaction start and excludes the +* transaction join case (when replaying log trees). +*/ +trace_btrfs_space_reservation (fs_info,"transaction" , + trans->transid, bytes,0 ); +btrfs_block_rsv_release (fs_info, trans->block_rsv, bytes,NULL ); +ASSERT (trans->bytes_reserved >= bytes); + trans->bytes_reserved -= bytes; +} + +/* Will return 0, -ENOMEM or -EEXIST (index number collision, unexpected). */ int btrfs_insert_delayed_dir_index(struct btrfs_trans_handle *trans, const char *name, int name_len, struct btrfs_inode *dir,
@@ -1468,6 +1490,27 @@ int btrfs_insert_delayed_dir_index(struct btrfs_trans_handle *trans,mutex_lock (&delayed_node->mutex); +/* +* First attempt to insert the delayed item. This is to make the error +* handling path simpler in case we fail (-EEXIST). There's no risk of +* any other task coming in and running the delayed item before we do +* the metadata space reservation below, because we are holding the +* delayed node's mutex and that mutex must also be locked before the +* node's delayed items can be run. +*/ + ret =__btrfs_add_delayed_item (delayed_node, delayed_item); +if (unlikely (ret)) { +btrfs_err (trans->fs_info, +"error adding delayed dir index item, name: %.*s, index: %llu, root: %llu, dir: %llu, dir->index_cnt: %llu, delayed_node->index_cnt: %llu, error: %d" , + name_len, name, index,btrfs_root_id (delayed_node->root), + delayed_node->inode_id, dir->index_cnt, + delayed_node->index_cnt, ret); +btrfs_release_delayed_item (delayed_item); +btrfs_release_dir_index_item_space (trans); +mutex_unlock (&delayed_node->mutex); + goto release_node; + } +if (delayed_node->index_item_leaves ==0 || delayed_node->curr_index_batch_size + data_len > leaf_data_size) { delayed_node->curr_index_batch_size = data_len;
@@ -1485,37 +1528,14 @@ int btrfs_insert_delayed_dir_index(struct btrfs_trans_handle *trans,* impossible. */ if (WARN_ON (ret)) { -mutex_unlock (&delayed_node->mutex);btrfs_release_delayed_item (delayed_item); +mutex_unlock (&delayed_node->mutex); goto release_node; } delayed_node->index_item_leaves++; - }else if (!test_bit (BTRFS_FS_LOG_RECOVERING , &fs_info->flags)) { -const u64 bytes =btrfs_calc_insert_metadata_size (fs_info,1 ); - -/* -* Adding the new dir index item does not require touching another -* leaf, so we can release 1 unit of metadata that was previously -* reserved when starting the transaction. This applies only to -* the case where we had a transaction start and excludes the -* transaction join case (when replaying log trees). -*/ -trace_btrfs_space_reservation (fs_info,"transaction" , - trans->transid, bytes,0 ); -btrfs_block_rsv_release (fs_info, trans->block_rsv, bytes,NULL ); -ASSERT (trans->bytes_reserved >= bytes); - trans->bytes_reserved -= bytes; - } - - ret =__btrfs_add_delayed_item (delayed_node, delayed_item); -if (unlikely (ret)) { -btrfs_err (trans->fs_info, -"error adding delayed dir index item, name: %.*s, index: %llu, root: %llu, dir: %llu, dir->index_cnt: %llu, delayed_node->index_cnt: %llu, error: %d" , - name_len, name, index,btrfs_root_id (delayed_node->root), - delayed_node->inode_id, dir->index_cnt, - delayed_node->index_cnt, ret); -BUG (); + }else { +btrfs_release_dir_index_item_space (trans); }mutex_unlock (&delayed_node->mutex);