From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0FD8ECE79AF for ; Wed, 20 Sep 2023 09:53:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234129AbjITJxp (ORCPT ); Wed, 20 Sep 2023 05:53:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49856 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234114AbjITJxo (ORCPT ); Wed, 20 Sep 2023 05:53:44 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5F0CA8F for ; Wed, 20 Sep 2023 02:53:38 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id ABA41C433C8; Wed, 20 Sep 2023 09:53:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1695203618; bh=QsrqGCNyS1wj22NRN9E8jEymZVd+IX069LQckqlgu5k=; h=Subject:To:Cc:From:Date:From; b=fbQNP2wcJavnOHCj28HaR4YHp+Tua7YXCPcSsbwA0x9XNA9Sq28isC1130jsFxijV GS+FZNPK9YSW+1lQjj3Cuzf8IoMMvUpAXvHxcdKNS/EFIYeMOY5no+SGce+zry0ne3 8Mb8LwOtEOHOp0XVVvaHVPK6euVrpjg54nCapQbc= Subject: FAILED: patch "[PATCH] btrfs: remove BUG() after failure to insert delayed dir index" failed to apply to 5.10-stable tree To: fdmanana@suse.com, dsterba@suse.com, wqu@suse.com Cc: From: Date: Wed, 20 Sep 2023 11:53:27 +0200 Message-ID: <2023092027-census-monorail-5614@gregkh> MIME-Version: 1.0 Content-Type: text/plain; charset=ANSI_X3.4-1968 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org The 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 . 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 # git commit -s git send-email --to '' --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 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. Link: https://lore.kernel.org/linux-btrfs/00000000000036e1290603e097e0@google.com/ CC: stable@vger.kernel.org # 5.4+ Reviewed-by: Qu Wenruo Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c index bb9908cbabfe..6e779bc16340 100644 --- a/fs/btrfs/delayed-inode.c +++ b/fs/btrfs/delayed-inode.c @@ -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);