From patchwork Tue Jun 28 20:48:31 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 12898933 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 A57F3C43334 for ; Tue, 28 Jun 2022 20:48:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230102AbiF1Use (ORCPT ); Tue, 28 Jun 2022 16:48:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36246 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229448AbiF1Use (ORCPT ); Tue, 28 Jun 2022 16:48:34 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 641C12A953 for ; Tue, 28 Jun 2022 13:48:33 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 02A626184D for ; Tue, 28 Jun 2022 20:48:33 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5C4D6C341C8; Tue, 28 Jun 2022 20:48:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1656449312; bh=xfkw3eKaNjbFiHH9y+tMHMThhW2gamD4h2DEcFdxeDc=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=JQUiCUw3OdN2nxNmgymn1KCqh3ohGVq4IQ3uOgvZzg/8+pzZWvkpRmzAjimG+GdS1 6/r8BppKOTnHX8RrdSCR1kka4DOHhE+OMfb2HU1NMrf+NKl4/CZOI7IB4db1pS4h/q 3B3GuMdnORdEKPpu0/Y93i4jbg4j0vvQNj6oDqbQ3Rw+s6Pbt3CQ2Jj53bn0L1ekSU Xp+VqA5cqQRO6Gpy3Ubo8o085ner9lmdWG68jDiY0i5Vma71yxLVLZmBU95qoAFSqn fqxrEFEmk8iMHdcDXG1jxtyqh/2xj8krN5dyaHP4YfpbGdLjzAtIt6+KFzvG3Iuw1Z 8nWhQlPHAskyg== Subject: [PATCH 1/8] misc: fix unsigned integer comparison complaints From: "Darrick J. Wong" To: sandeen@sandeen.net, djwong@kernel.org Cc: linux-xfs@vger.kernel.org Date: Tue, 28 Jun 2022 13:48:31 -0700 Message-ID: <165644931191.1089724.14586418293765469096.stgit@magnolia> In-Reply-To: <165644930619.1089724.12201433387040577983.stgit@magnolia> References: <165644930619.1089724.12201433387040577983.stgit@magnolia> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong gcc 11.2 complains about certain variables now that xfs_extnum_t is an unsigned 64-bit integer: dinode.c: In function ‘process_exinode’: dinode.c:960:21: error: comparison of unsigned expression in ‘< 0’ is always false [-Werror=type-limits] 960 | if (numrecs < 0) Since we actually have a function that will tell us the maximum supported extent count for an ondisk dinode structure, use a direct comparison instead of tricky integer math to detect overflows. A more exhaustive audit is probably necessary. IOWS, shut up, gcc... Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- db/check.c | 10 +++++++--- db/metadump.c | 11 +++++++---- repair/dinode.c | 14 ++++++++++---- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/db/check.c b/db/check.c index fb28994d..c9149daa 100644 --- a/db/check.c +++ b/db/check.c @@ -2711,14 +2711,18 @@ process_exinode( int whichfork) { xfs_bmbt_rec_t *rp; + xfs_extnum_t max_nex; rp = (xfs_bmbt_rec_t *)XFS_DFORK_PTR(dip, whichfork); *nex = xfs_dfork_nextents(dip, whichfork); - if (*nex < 0 || *nex > XFS_DFORK_SIZE(dip, mp, whichfork) / + max_nex = xfs_iext_max_nextents( + xfs_dinode_has_large_extent_counts(dip), + whichfork); + if (*nex > max_nex || *nex > XFS_DFORK_SIZE(dip, mp, whichfork) / sizeof(xfs_bmbt_rec_t)) { if (!sflag || id->ilist) - dbprintf(_("bad number of extents %d for inode %lld\n"), - *nex, id->ino); + dbprintf(_("bad number of extents %llu for inode %lld\n"), + (unsigned long long)*nex, id->ino); error++; return; } diff --git a/db/metadump.c b/db/metadump.c index 999c68f7..27d1df43 100644 --- a/db/metadump.c +++ b/db/metadump.c @@ -2278,16 +2278,19 @@ process_exinode( { int whichfork; int used; - xfs_extnum_t nex; + xfs_extnum_t nex, max_nex; whichfork = (itype == TYP_ATTR) ? XFS_ATTR_FORK : XFS_DATA_FORK; nex = xfs_dfork_nextents(dip, whichfork); + max_nex = xfs_iext_max_nextents( + xfs_dinode_has_large_extent_counts(dip), + whichfork); used = nex * sizeof(xfs_bmbt_rec_t); - if (nex < 0 || used > XFS_DFORK_SIZE(dip, mp, whichfork)) { + if (nex > max_nex || used > XFS_DFORK_SIZE(dip, mp, whichfork)) { if (show_warnings) - print_warning("bad number of extents %d in inode %lld", - nex, (long long)cur_ino); + print_warning("bad number of extents %llu in inode %lld", + (unsigned long long)nex, (long long)cur_ino); return 1; } diff --git a/repair/dinode.c b/repair/dinode.c index 04e7f83e..00de31fb 100644 --- a/repair/dinode.c +++ b/repair/dinode.c @@ -942,7 +942,7 @@ process_exinode( xfs_bmbt_rec_t *rp; xfs_fileoff_t first_key; xfs_fileoff_t last_key; - xfs_extnum_t numrecs; + xfs_extnum_t numrecs, max_numrecs; int ret; lino = XFS_AGINO_TO_INO(mp, agno, ino); @@ -956,7 +956,10 @@ process_exinode( * be in the range of valid on-disk numbers, which is: * 0 < numrecs < 2^31 - 1 */ - if (numrecs < 0) + max_numrecs = xfs_iext_max_nextents( + xfs_dinode_has_large_extent_counts(dip), + whichfork); + if (numrecs > max_numrecs) numrecs = *nex; /* @@ -1899,7 +1902,7 @@ process_inode_data_fork( { xfs_ino_t lino = XFS_AGINO_TO_INO(mp, agno, ino); int err = 0; - xfs_extnum_t nex; + xfs_extnum_t nex, max_nex; /* * extent count on disk is only valid for positive values. The kernel @@ -1907,7 +1910,10 @@ process_inode_data_fork( * here, trash it! */ nex = xfs_dfork_data_extents(dino); - if (nex < 0) + max_nex = xfs_iext_max_nextents( + xfs_dinode_has_large_extent_counts(dino), + XFS_DATA_FORK); + if (nex > max_nex) *nextents = 1; else *nextents = nex; From patchwork Tue Jun 28 20:48:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 12898934 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 E4529C43334 for ; Tue, 28 Jun 2022 20:48:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230122AbiF1Usp (ORCPT ); Tue, 28 Jun 2022 16:48:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36508 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229448AbiF1Usn (ORCPT ); Tue, 28 Jun 2022 16:48:43 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 48DAC2FFD1 for ; Tue, 28 Jun 2022 13:48:41 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 46AE1B81E06 for ; Tue, 28 Jun 2022 20:48:39 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E35E9C341C8; Tue, 28 Jun 2022 20:48:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1656449318; bh=567PYF198SZgILpbhROV4h251ujmBnn467Vqm7nAhqo=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=m24/lnZ8an/+Rit5X48QujPirZqjIMWjyhqnqAcl1TAEE1cDuDL6tOvi5wiExB3ht Eb7Og1Pr1wN1Fmpc/TdjA5MVVW9WMl+O2k+zJF9HYWyWxwX/poI3yJulmxjFhtpw7k nt6aPTYbtqsZz+RJTgn4FLzlI2Spsl/ClMo1M7A/jAgKw9o9DRMbn3apDsEW/NZjtJ wCdw7TxYkN0XNdISuli1bfcx05ELI5AI5CeytWUo2XBSQaLTu+a+3YuGveJMyx8mPX 6x7JhRlmVNitiBJSl/GTjjOR0n3m8+Kb8FCc+v9c0a8AypH9jXX27uoA3Q8ic1mLc5 8/x6hcWwuRd+A== Subject: [PATCH 2/8] xfs_logprint: fix formatting specifiers From: "Darrick J. Wong" To: sandeen@sandeen.net, djwong@kernel.org Cc: linux-xfs@vger.kernel.org Date: Tue, 28 Jun 2022 13:48:37 -0700 Message-ID: <165644931754.1089724.16023443761407042271.stgit@magnolia> In-Reply-To: <165644930619.1089724.12201433387040577983.stgit@magnolia> References: <165644930619.1089724.12201433387040577983.stgit@magnolia> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong Fix a missing %u -> %PRIu32 conversion, and add the required '%' in the format specifiers because PRIu{32,64} do not include it on their own. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- logprint/log_print_all.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c index f7c32d6a..8d3ede19 100644 --- a/logprint/log_print_all.c +++ b/logprint/log_print_all.c @@ -267,7 +267,7 @@ xlog_recover_print_inode_core( xlog_extract_dinode_ts(di->di_ctime)); printf(_(" flushiter:%d\n"), di->di_flushiter); printf(_(" size:0x%llx nblks:0x%llx exsize:%d " - "nextents:" PRIu64 " anextents:%u\n"), (unsigned long long) + "nextents:%" PRIu64 " anextents:%" PRIu32 "\n"), (unsigned long long) di->di_size, (unsigned long long)di->di_nblocks, di->di_extsize, nextents, anextents); printf(_(" forkoff:%d dmevmask:0x%x dmstate:%d flags:0x%x " From patchwork Tue Jun 28 20:48:43 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 12898935 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 9AE7BC43334 for ; Tue, 28 Jun 2022 20:48:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229507AbiF1Uss (ORCPT ); Tue, 28 Jun 2022 16:48:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36616 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229448AbiF1Usr (ORCPT ); Tue, 28 Jun 2022 16:48:47 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 536BD2CE03 for ; Tue, 28 Jun 2022 13:48:46 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 08180B81E04 for ; Tue, 28 Jun 2022 20:48:45 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7E9C0C341C8; Tue, 28 Jun 2022 20:48:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1656449323; bh=S1DN/Nh98tVM49PO5BY28WSyoNxcE+vgXSqt7QCVOgk=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=n+qDvvHJy/EPdqWt1GaGxhB1+AxKn6EuYlX+/2wanBOBdKa5hwsn3oaDAd/yfx7kl JPFJvCPO0IH0vLLJcNMDmYZ5/CeTdnakXp8/UHjt0ZMmN3TRRAd/P+kQoUTba6VebJ KGBMEOKyRDRUqgXshwnmC3ryHCQ2pFFL9bbJKMWGBumQ6qfyC5+UaDHYVnVskxs1wR ArSBzmTSCtqLvyAfH7Hc+12HTYjirYwkkLNARXh48eiaKu1m2M82CL1QbvHO0dwyBM GPip051auOIh3dfXYBzPJhTKJzq8r4GccVzbm80dHojyX/ahk1byWj3bJLJnzTfec3 gRtWKCjLqPvVg== Subject: [PATCH 3/8] xfs: fix TOCTOU race involving the new logged xattrs control knob From: "Darrick J. Wong" To: sandeen@sandeen.net, djwong@kernel.org Cc: linux-xfs@vger.kernel.org Date: Tue, 28 Jun 2022 13:48:43 -0700 Message-ID: <165644932309.1089724.2522590493157297421.stgit@magnolia> In-Reply-To: <165644930619.1089724.12201433387040577983.stgit@magnolia> References: <165644930619.1089724.12201433387040577983.stgit@magnolia> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong Source kernel commit: fe5b1eb7fdcb2e5b6dd70da2f17acd8026d93b6a I found a race involving the larp control knob, aka the debugging knob that lets developers enable logging of extended attribute updates: Thread 1 Thread 2 echo 0 > /sys/fs/xfs/debug/larp setxattr(REPLACE) xfs_has_larp (returns false) xfs_attr_set echo 1 > /sys/fs/xfs/debug/larp xfs_attr_defer_replace xfs_attr_init_replace_state xfs_has_larp (returns true) xfs_attr_init_remove_state This isn't a particularly severe problem right now because xattr logging is only enabled when CONFIG_XFS_DEBUG=y, and developers *should* know what they're doing. However, the eventual intent is that callers should be able to ask for the assistance of the log in persisting xattr updates. This capability might not be required for /all/ callers, which means that dynamic control must work correctly. Once an xattr update has decided whether or not to use logged xattrs, it needs to stay in that mode until the end of the operation regardless of what subsequent parallel operations might do. Therefore, it is an error to continue sampling xfs_globals.larp once xfs_attr_change has made a decision about larp, and it was not correct for me to have told Allison that ->create_intent functions can sample the global log incompat feature bitfield to decide to elide a log item. Instead, create a new op flag for the xfs_da_args structure, and convert all other callers of xfs_has_larp and xfs_sb_version_haslogxattrs within the attr update state machine to look for the operations flag. Signed-off-by: Darrick J. Wong --- libxfs/xfs_attr.c | 6 ++++-- libxfs/xfs_attr.h | 12 +----------- libxfs/xfs_attr_leaf.c | 2 +- libxfs/xfs_da_btree.h | 4 +++- 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c index dfd284c6..d2e28a27 100644 --- a/libxfs/xfs_attr.c +++ b/libxfs/xfs_attr.c @@ -995,9 +995,11 @@ xfs_attr_set( /* * We have no control over the attribute names that userspace passes us * to remove, so we have to allow the name lookup prior to attribute - * removal to fail as well. + * removal to fail as well. Preserve the logged flag, since we need + * to pass that through to the logging code. */ - args->op_flags = XFS_DA_OP_OKNOENT; + args->op_flags = XFS_DA_OP_OKNOENT | + (args->op_flags & XFS_DA_OP_LOGGED); if (args->value) { XFS_STATS_INC(mp, xs_attr_set); diff --git a/libxfs/xfs_attr.h b/libxfs/xfs_attr.h index e329da3e..b4a2fc77 100644 --- a/libxfs/xfs_attr.h +++ b/libxfs/xfs_attr.h @@ -28,16 +28,6 @@ struct xfs_attr_list_context; */ #define ATTR_MAX_VALUELEN (64*1024) /* max length of a value */ -static inline bool xfs_has_larp(struct xfs_mount *mp) -{ -#ifdef DEBUG - /* Logged xattrs require a V5 super for log_incompat */ - return xfs_has_crc(mp) && xfs_globals.larp; -#else - return false; -#endif -} - /* * Kernel-internal version of the attrlist cursor. */ @@ -624,7 +614,7 @@ static inline enum xfs_delattr_state xfs_attr_init_replace_state(struct xfs_da_args *args) { args->op_flags |= XFS_DA_OP_ADDNAME | XFS_DA_OP_REPLACE; - if (xfs_has_larp(args->dp->i_mount)) + if (args->op_flags & XFS_DA_OP_LOGGED) return xfs_attr_init_remove_state(args); return xfs_attr_init_add_state(args); } diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c index 75b8f08e..5fea3702 100644 --- a/libxfs/xfs_attr_leaf.c +++ b/libxfs/xfs_attr_leaf.c @@ -1527,7 +1527,7 @@ xfs_attr3_leaf_add_work( if (tmp) entry->flags |= XFS_ATTR_LOCAL; if (args->op_flags & XFS_DA_OP_REPLACE) { - if (!xfs_has_larp(mp)) + if (!(args->op_flags & XFS_DA_OP_LOGGED)) entry->flags |= XFS_ATTR_INCOMPLETE; if ((args->blkno2 == args->blkno) && (args->index2 <= args->index)) { diff --git a/libxfs/xfs_da_btree.h b/libxfs/xfs_da_btree.h index d33b7686..ffa3df5b 100644 --- a/libxfs/xfs_da_btree.h +++ b/libxfs/xfs_da_btree.h @@ -92,6 +92,7 @@ typedef struct xfs_da_args { #define XFS_DA_OP_NOTIME (1u << 5) /* don't update inode timestamps */ #define XFS_DA_OP_REMOVE (1u << 6) /* this is a remove operation */ #define XFS_DA_OP_RECOVERY (1u << 7) /* Log recovery operation */ +#define XFS_DA_OP_LOGGED (1u << 8) /* Use intent items to track op */ #define XFS_DA_OP_FLAGS \ { XFS_DA_OP_JUSTCHECK, "JUSTCHECK" }, \ @@ -101,7 +102,8 @@ typedef struct xfs_da_args { { XFS_DA_OP_CILOOKUP, "CILOOKUP" }, \ { XFS_DA_OP_NOTIME, "NOTIME" }, \ { XFS_DA_OP_REMOVE, "REMOVE" }, \ - { XFS_DA_OP_RECOVERY, "RECOVERY" } + { XFS_DA_OP_RECOVERY, "RECOVERY" }, \ + { XFS_DA_OP_LOGGED, "LOGGED" } /* * Storage for holding state during Btree searches and split/join ops. From patchwork Tue Jun 28 20:48:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 12898936 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 39871C43334 for ; Tue, 28 Jun 2022 20:48:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230217AbiF1Usx (ORCPT ); Tue, 28 Jun 2022 16:48:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36740 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230419AbiF1Usw (ORCPT ); Tue, 28 Jun 2022 16:48:52 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CFA132FFC4 for ; Tue, 28 Jun 2022 13:48:50 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 9A22F6182E for ; Tue, 28 Jun 2022 20:48:49 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 094B5C341C8; Tue, 28 Jun 2022 20:48:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1656449329; bh=ce+nJnYsIb8gdisTpza82ipvThPyNEptPwvmdp/MZGU=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=LR4Ztfjlp59Pk2kWvTomO3JyFqqM5jD1oNjjkfTX4+hcwO7B4hjCQ88yglc+TugaF xcq6rf7maVgmnB+shzu1DKFEcdUpFuJzHD/J9auiMhlMThdmNZ6Z7lahcg3ry/rTAj r0c0OSsg2HVa/9QCXff7QpT0OSzmMPbjzQK5hfzPFpz4eDhozCpYMuUv0ygHRgK0Wb s4ALZ7SV5LU2BQFBzfLsQDlc1dAo6jy1sJEQas2kWxW5bSxNQ4yc9tQ9kzO61zrjGi HJcFMH5He0oDrZTCRWJcpNCxVWUBk6ZG8QOWWKUjYdvKaYjf5GkAuc87CBzpYIo0os pBRDOCqJDqacA== Subject: [PATCH 4/8] libxfs: remove xfs_globals.larp From: "Darrick J. Wong" To: sandeen@sandeen.net, djwong@kernel.org Cc: linux-xfs@vger.kernel.org Date: Tue, 28 Jun 2022 13:48:48 -0700 Message-ID: <165644932867.1089724.4941357024120616371.stgit@magnolia> In-Reply-To: <165644930619.1089724.12201433387040577983.stgit@magnolia> References: <165644930619.1089724.12201433387040577983.stgit@magnolia> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong This dummy debugging knob isn't necessary anymore, so get rid of it. Signed-off-by: Darrick J. Wong --- include/xfs_mount.h | 7 ------- libxfs/util.c | 6 ------ 2 files changed, 13 deletions(-) diff --git a/include/xfs_mount.h b/include/xfs_mount.h index 7935e7ea..ba80aa79 100644 --- a/include/xfs_mount.h +++ b/include/xfs_mount.h @@ -270,11 +270,4 @@ struct xfs_dquot { int q_type; }; -struct xfs_globals { -#ifdef DEBUG - bool larp; /* log attribute replay */ -#endif -}; -extern struct xfs_globals xfs_globals; - #endif /* __XFS_MOUNT_H__ */ diff --git a/libxfs/util.c b/libxfs/util.c index e5e49477..ef01fcf8 100644 --- a/libxfs/util.c +++ b/libxfs/util.c @@ -720,10 +720,4 @@ xfs_fs_mark_healthy( spin_unlock(&mp->m_sb_lock); } -struct xfs_globals xfs_globals = { -#ifdef DEBUG - .larp = false, /* log attribute replay */ -#endif -}; - void xfs_ag_geom_health(struct xfs_perag *pag, struct xfs_ag_geometry *ageo) { } From patchwork Tue Jun 28 20:48:54 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 12898937 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 EC8C5C43334 for ; Tue, 28 Jun 2022 20:48:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229448AbiF1Us5 (ORCPT ); Tue, 28 Jun 2022 16:48:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36760 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231138AbiF1Us4 (ORCPT ); Tue, 28 Jun 2022 16:48:56 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 95BF12F39E for ; Tue, 28 Jun 2022 13:48:55 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 325B061851 for ; Tue, 28 Jun 2022 20:48:55 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 905AFC341C8; Tue, 28 Jun 2022 20:48:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1656449334; bh=k0NLvDtWRCasBJGV+FCCK4c6B0wMXMNwlpkQK/TLa2s=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=gRUgMwSTMqNUpF9ndvy6Ve8RHgEXBOQINTPWBsj9N0Oy3JjWqadm/4HsDtVdsh53/ abl+pwSxNTP0MP63uURuQUFRgQs8+f9A+4txnBOFR4h99TgGjATqnHIU7lkBh19bph o+M70NZpAA/iNnERLUE0IZquV5dZcrOuoDGNArU5pjYDimhz/bb08SHDK59XG4dbbt Jmv+kObHypexSvtY50TkeVe9kLDsNgzMHGMivtwSlnxkXPPs9+lfKbYufGBLnpGmEc 462fEjIX+kAVNpR29G5HcfnuGEYqobK02cwgq+gckCP5IrP/1U9iKS3PV4DiYQ68HQ 4rRKwkpFNZV6Q== Subject: [PATCH 5/8] xfs: fix variable state usage From: "Darrick J. Wong" To: sandeen@sandeen.net, djwong@kernel.org Cc: linux-xfs@vger.kernel.org Date: Tue, 28 Jun 2022 13:48:54 -0700 Message-ID: <165644933419.1089724.10811377112251235277.stgit@magnolia> In-Reply-To: <165644930619.1089724.12201433387040577983.stgit@magnolia> References: <165644930619.1089724.12201433387040577983.stgit@magnolia> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong Source kernel commit: e387f3c8beb10f2f557d6fb1d31a0c0252a2b65d The variable @args is fed to a tracepoint, and that's the only place it's used. This is fine for the kernel, but for userspace, tracepoints are #define'd out of existence, which results in this warning on gcc 11.2: xfs_attr.c: In function ‘xfs_attr_node_try_addname’: xfs_attr.c:1440:42: warning: unused variable ‘args’ [-Wunused-variable] 1440 | struct xfs_da_args *args = attr->xattri_da_args; | ^~~~ Clean this up. Signed-off-by: Darrick J. Wong --- libxfs/xfs_attr.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c index d2e28a27..dba528e6 100644 --- a/libxfs/xfs_attr.c +++ b/libxfs/xfs_attr.c @@ -1439,12 +1439,11 @@ static int xfs_attr_node_try_addname( struct xfs_attr_intent *attr) { - struct xfs_da_args *args = attr->xattri_da_args; struct xfs_da_state *state = attr->xattri_da_state; struct xfs_da_state_blk *blk; int error; - trace_xfs_attr_node_addname(args); + trace_xfs_attr_node_addname(state->args); blk = &state->path.blk[state->path.active-1]; ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC); From patchwork Tue Jun 28 20:48:59 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 12898938 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 CB31DC43334 for ; Tue, 28 Jun 2022 20:49:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229634AbiF1UtC (ORCPT ); Tue, 28 Jun 2022 16:49:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36908 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229707AbiF1UtC (ORCPT ); Tue, 28 Jun 2022 16:49:02 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3E05F2F01F for ; Tue, 28 Jun 2022 13:49:01 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id C05406182E for ; Tue, 28 Jun 2022 20:49:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 27BCDC341C8; Tue, 28 Jun 2022 20:49:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1656449340; bh=2ojqSZNoi156RvIwkWDKbxBHfXib3jdx3HrTOupgH1s=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=Z1SwhqNPazoQt11+DZPv3iJqo0zFIVUX0aeD/MI0uZ+ADKpwkYyoL5PhAKQeRLDW8 jOK15QT9PZB/y6Ea/xGSid0WJB7QY/LpilT8X82V1Be1quuu0LAw+A6AYCY20mtx9K 3j9DvmEGsFWl8KQgkG/hLro+n9EhxXColdJlrE2ZnzfBJdWV8paKEeVLHsXDS5e+Tb hEjqKWetwDqnaeouWm2QjjKAyFBnsGcDVCCAUOv1BcYDfNG+xDguSTppNNYQmN4HvB wUykvH9LogqR4urpri0JFhJabHoDQHG09mxK8sNPXZH7srs6G7y0epdPHagvUQbAhb lycRsOcYXLm4g== Subject: [PATCH 6/8] xfs: empty xattr leaf header blocks are not corruption From: "Darrick J. Wong" To: sandeen@sandeen.net, djwong@kernel.org Cc: linux-xfs@vger.kernel.org Date: Tue, 28 Jun 2022 13:48:59 -0700 Message-ID: <165644933974.1089724.15159085443668536774.stgit@magnolia> In-Reply-To: <165644930619.1089724.12201433387040577983.stgit@magnolia> References: <165644930619.1089724.12201433387040577983.stgit@magnolia> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong Source kernel commit: 1ddd6c9a04ae0fed6790e1ba8294b0a2e6ed8066 TLDR: Revert commit 51e6104fdb95 ("xfs: detect empty attr leaf blocks in xfs_attr3_leaf_verify") because it was wrong. Every now and then we get a corruption report from the kernel or xfs_repair about empty leaf blocks in the extended attribute structure. We've long thought that these shouldn't be possible, but prior to 5.18 one would shake loose in the recoveryloop fstests about once a month. A new addition to the xattr leaf block verifier in 5.19-rc1 makes this happen every 7 minutes on my testing cloud. I added a ton of logging to detect any time we set the header count on an xattr leaf block to zero. This produced the following dmesg output on generic/388: XFS (sda4): ino 0x21fcbaf leaf 0x129bf78 hdcount==0! Call Trace: dump_stack_lvl+0x34/0x44 xfs_attr3_leaf_create+0x187/0x230 xfs_attr_shortform_to_leaf+0xd1/0x2f0 xfs_attr_set_iter+0x73e/0xa90 xfs_xattri_finish_update+0x45/0x80 xfs_attr_finish_item+0x1b/0xd0 xfs_defer_finish_noroll+0x19c/0x770 __xfs_trans_commit+0x153/0x3e0 xfs_attr_set+0x36b/0x740 xfs_xattr_set+0x89/0xd0 __vfs_setxattr+0x67/0x80 __vfs_setxattr_noperm+0x6e/0x120 vfs_setxattr+0x97/0x180 setxattr+0x88/0xa0 path_setxattr+0xc3/0xe0 __x64_sys_setxattr+0x27/0x30 do_syscall_64+0x35/0x80 entry_SYSCALL_64_after_hwframe+0x46/0xb0 So now we know that someone is creating empty xattr leaf blocks as part of converting a sf xattr structure into a leaf xattr structure. The conversion routine logs any existing sf attributes in the same transaction that creates the leaf block, so we know this is a setxattr to a file that has no attributes at all. Next, g/388 calls the shutdown ioctl and cycles the mount to trigger log recovery. I also augmented buffer item recovery to call ->verify_struct on any attr leaf blocks and complain if it finds a failure: XFS (sda4): Unmounting Filesystem XFS (sda4): Mounting V5 Filesystem XFS (sda4): Starting recovery (logdev: internal) XFS (sda4): xattr leaf daddr 0x129bf78 hdrcount == 0! Call Trace: dump_stack_lvl+0x34/0x44 xfs_attr3_leaf_verify+0x3b8/0x420 xlog_recover_buf_commit_pass2+0x60a/0x6c0 xlog_recover_items_pass2+0x4e/0xc0 xlog_recover_commit_trans+0x33c/0x350 xlog_recovery_process_trans+0xa5/0xe0 xlog_recover_process_data+0x8d/0x140 xlog_do_recovery_pass+0x19b/0x720 xlog_do_log_recovery+0x62/0xc0 xlog_do_recover+0x33/0x1d0 xlog_recover+0xda/0x190 xfs_log_mount+0x14c/0x360 xfs_mountfs+0x517/0xa60 xfs_fs_fill_super+0x6bc/0x950 get_tree_bdev+0x175/0x280 vfs_get_tree+0x1a/0x80 path_mount+0x6f5/0xaa0 __x64_sys_mount+0x103/0x140 do_syscall_64+0x35/0x80 entry_SYSCALL_64_after_hwframe+0x46/0xb0 RIP: 0033:0x7fc61e241eae And a moment later, the _delwri_submit of the recovered buffers trips the same verifier and recovery fails: XFS (sda4): Metadata corruption detected at xfs_attr3_leaf_verify+0x393/0x420 [xfs], xfs_attr3_leaf block 0x129bf78 XFS (sda4): Unmount and run xfs_repair XFS (sda4): First 128 bytes of corrupted metadata buffer: 00000000: 00 00 00 00 00 00 00 00 3b ee 00 00 00 00 00 00 ........;....... 00000010: 00 00 00 00 01 29 bf 78 00 00 00 00 00 00 00 00 .....).x........ 00000020: a5 1b d0 02 b2 9a 49 df 8e 9c fb 8d f8 31 3e 9d ......I......1>. 00000030: 00 00 00 00 02 1f cb af 00 00 00 00 10 00 00 00 ................ 00000040: 00 50 0f b0 00 00 00 00 00 00 00 00 00 00 00 00 .P.............. 00000050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00000060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00000070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ XFS (sda4): Corruption of in-memory data (0x8) detected at _xfs_buf_ioapply+0x37f/0x3b0 [xfs] (fs/xfs/xfs_buf.c:1518). Shutting down filesystem. XFS (sda4): Please unmount the filesystem and rectify the problem(s) XFS (sda4): log mount/recovery failed: error -117 XFS (sda4): log mount failed I think I see what's going on here -- setxattr is racing with something that shuts down the filesystem: Thread 1 Thread 2 -------- -------- xfs_attr_sf_addname xfs_attr_shortform_to_leaf xfs_trans_bhold(leaf) xattri_dela_state = XFS_DAS_LEAF_ADD xfs_trans_bhold_release(leaf) Thread 3 -------- xlog_recover_buf_commit_pass2 xlog_recover_do_reg_buffer xfs_buf_delwri_queue(leaf) xfs_buf_delwri_submit _xfs_buf_ioapply(leaf) xfs_attr3_leaf_write_verify As you can see, the bhold keeps the leaf buffer locked and thus prevents the *AIL* from tripping over the ichdr.count==0 check in the write verifier. Unfortunately, it doesn't prevent the log from getting flushed to disk, which sets up log recovery to fail. So. It's clear that the kernel has always had the ability to persist attr leaf blocks with ichdr.count==0, which means that it's part of the ondisk format now. Unfortunately, this check has been added and removed multiple times throughout history. It first appeared in[1] kernel 3.10 as part of the early V5 format patches. The check was later discovered to break log recovery and hence disabled[2] during log recovery in kernel 4.10. Simultaneously, the check was added[3] to xfs_repair 4.9.0 to try to weed out the empty leaf blocks. This was still not correct because log recovery would recover an empty attr leaf block successfully only for regular xattr operations to trip over the empty block during of the block during regular operation. Therefore, the check was removed entirely[4] in kernel 5.7 but removal of the xfs_repair check was forgotten. The continued complaints from xfs_repair lead to us mistakenly re-adding[5] the verifier check for kernel 5.19. Remove it once again. [1] 517c22207b04 ("xfs: add CRCs to attr leaf blocks") [2] 2e1d23370e75 ("xfs: ignore leaf attr ichdr.count in verifier during log replay") [3] f7140161 ("xfs_repair: junk leaf attribute if count == 0") [4] f28cef9e4dac ("xfs: don't fail verifier on empty attr3 leaf block") [5] 51e6104fdb95 ("xfs: detect empty attr leaf blocks in xfs_attr3_leaf_verify") Looking at the rest of the xattr code, it seems that files with empty leaf blocks behave as expected -- listxattr reports no attributes; getxattr on any xattr returns nothing as expected; removexattr does nothing; and setxattr can add attributes just fine. Original-bug: 517c22207b04 ("xfs: add CRCs to attr leaf blocks") Still-not-fixed-by: 2e1d23370e75 ("xfs: ignore leaf attr ichdr.count in verifier during log replay") Removed-in: f28cef9e4dac ("xfs: don't fail verifier on empty attr3 leaf block") Fixes: 51e6104fdb95 ("xfs: detect empty attr leaf blocks in xfs_attr3_leaf_verify") Signed-off-by: Darrick J. Wong --- libxfs/xfs_attr_leaf.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c index 5fea3702..047ab01b 100644 --- a/libxfs/xfs_attr_leaf.c +++ b/libxfs/xfs_attr_leaf.c @@ -286,6 +286,23 @@ xfs_attr3_leaf_verify_entry( return NULL; } +/* + * Validate an attribute leaf block. + * + * Empty leaf blocks can occur under the following circumstances: + * + * 1. setxattr adds a new extended attribute to a file; + * 2. The file has zero existing attributes; + * 3. The attribute is too large to fit in the attribute fork; + * 4. The attribute is small enough to fit in a leaf block; + * 5. A log flush occurs after committing the transaction that creates + * the (empty) leaf block; and + * 6. The filesystem goes down after the log flush but before the new + * attribute can be committed to the leaf block. + * + * Hence we need to ensure that we don't fail the validation purely + * because the leaf is empty. + */ static xfs_failaddr_t xfs_attr3_leaf_verify( struct xfs_buf *bp) @@ -307,15 +324,6 @@ xfs_attr3_leaf_verify( if (fa) return fa; - /* - * Empty leaf blocks should never occur; they imply the existence of a - * software bug that needs fixing. xfs_repair also flags them as a - * corruption that needs fixing, so we should never let these go to - * disk. - */ - if (ichdr.count == 0) - return __this_address; - /* * firstused is the block offset of the first name info structure. * Make sure it doesn't go off the block or crash into the header. From patchwork Tue Jun 28 20:49:05 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 12898939 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 7DB9ACCA47E for ; Tue, 28 Jun 2022 20:49:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230397AbiF1UtM (ORCPT ); Tue, 28 Jun 2022 16:49:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37124 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231172AbiF1UtJ (ORCPT ); Tue, 28 Jun 2022 16:49:09 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 590B32FFC4 for ; Tue, 28 Jun 2022 13:49:08 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 06353B82013 for ; Tue, 28 Jun 2022 20:49:07 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B87E2C341C8; Tue, 28 Jun 2022 20:49:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1656449345; bh=zHgt8M39DaBHfbVsCo9H4gFPg6hQaSk5pclu/PWwmeQ=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=lSRE9OwEaR70ZfW9H71S8gQ6G47KFpNEuL/jswlvuGR+mrVMEuVMBUYzJDc1RrJSj /GdaU1Xxott6F2FTEYByjXgD51InvIs1ivxb/KY5EBNkiLpCY5PjMoBm3c09j7CcBZ QZ6X5OnN2iJasMu0zXaCmfHiznaw+7WOm4bTZlcP6jq3NRWYowr3irBSmqIz6U23Uv 6XMNCNQurnPG/QXaTk7I4jxDqlgYFPtfNzwyvNllbwBMMgYxBJanAwTuK4YmF5N07Y UZ49+zXo9GA6RFBaDMVOhq3snFXJfO8QltntphhjWR9Msh6G2toSIp7qyio2aKuUDB 9PhXewjIUMYNA== Subject: [PATCH 7/8] xfs_repair: ignore empty xattr leaf blocks From: "Darrick J. Wong" To: sandeen@sandeen.net, djwong@kernel.org Cc: linux-xfs@vger.kernel.org Date: Tue, 28 Jun 2022 13:49:05 -0700 Message-ID: <165644934532.1089724.4998920056841721528.stgit@magnolia> In-Reply-To: <165644930619.1089724.12201433387040577983.stgit@magnolia> References: <165644930619.1089724.12201433387040577983.stgit@magnolia> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong As detailed in the previous commit, empty xattr leaf blocks can be the benign byproduct of the system going down during the multi-step process of adding a large xattr to a file that has no xattrs. If we find one at attr fork offset 0, we should clear it, but this isn't a corruption. Signed-off-by: Darrick J. Wong --- repair/attr_repair.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/repair/attr_repair.c b/repair/attr_repair.c index 2055d96e..c3a6d502 100644 --- a/repair/attr_repair.c +++ b/repair/attr_repair.c @@ -579,6 +579,26 @@ process_leaf_attr_block( firstb = mp->m_sb.sb_blocksize; stop = xfs_attr3_leaf_hdr_size(leaf); + /* + * Empty leaf blocks at offset zero can occur as a race between + * setxattr and the system going down, so we only take action if we're + * running in modify mode. See xfs_attr3_leaf_verify for details of + * how we've screwed this up many times. + */ + if (!leafhdr.count && da_bno == 0) { + if (no_modify) { + do_log( + _("would clear empty leaf attr block 0, inode %" PRIu64 "\n"), + ino); + return 0; + } + + do_warn( + _("will clear empty leaf attr block 0, inode %" PRIu64 "\n"), + ino); + return 1; + } + /* does the count look sorta valid? */ if (!leafhdr.count || leafhdr.count * sizeof(xfs_attr_leaf_entry_t) + stop > From patchwork Tue Jun 28 20:49:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 12898940 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 0C436C43334 for ; Tue, 28 Jun 2022 20:49:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229712AbiF1UtY (ORCPT ); Tue, 28 Jun 2022 16:49:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37360 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231211AbiF1UtV (ORCPT ); Tue, 28 Jun 2022 16:49:21 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 79B262F01F for ; Tue, 28 Jun 2022 13:49:12 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id ECD626182E for ; Tue, 28 Jun 2022 20:49:11 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 53A53C341C8; Tue, 28 Jun 2022 20:49:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1656449351; bh=4IbRFRZ+pEppSjRMzR0jkY27Zjm6u+2PiR1dgdqsxIo=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=fqwSMdgqMj0IqmQm57Q1HLLPgAmQTjWSWgEm1qSxyB7cNF0xGyYs1Kj/zgXM4jfL3 xu0Te8Bb4bmzm9qhKJX7NMyO1TWnIv8RY6CHtzKBBzam0PUEY++cH/6RnCrLXSn3jH YunFLxrZ6RZnNAtPyUZ9Bv8ZEsgw0G9A1J5fC/YQ/ACw+M9b+nRZeDvAoCAI+itKbh YFdEntvyQlyIaeKmr/a+Gcy2CAfmc+zcvijpE2GW1bLAvIiFRzh+PZyvaRK71fdVlJ Z7zuNmPa0PfDS8jhZBZ6amvA4GE6A+rKM48h/uJI4i+HYbtTM+KqvqtdGxz9Hx2ykJ baUEeL4MvbeOQ== Subject: [PATCH 8/8] xfs: don't hold xattr leaf buffers across transaction rolls From: "Darrick J. Wong" To: sandeen@sandeen.net, djwong@kernel.org Cc: linux-xfs@vger.kernel.org Date: Tue, 28 Jun 2022 13:49:10 -0700 Message-ID: <165644935091.1089724.6853487454654769399.stgit@magnolia> In-Reply-To: <165644930619.1089724.12201433387040577983.stgit@magnolia> References: <165644930619.1089724.12201433387040577983.stgit@magnolia> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong Source kernel commit: 8b2fe54075e0d04a126ecfbeb714fea2f77fb8e4 Now that we've established (again!) that empty xattr leaf buffers are ok, we no longer need to bhold them to transactions when we're creating new leaf blocks. Get rid of the entire mechanism, which should simplify the xattr code quite a bit. Signed-off-by: Darrick J. Wong --- libxfs/xfs_attr.c | 38 +++++++++----------------------------- libxfs/xfs_attr.h | 5 ----- libxfs/xfs_attr_leaf.c | 9 ++------- libxfs/xfs_attr_leaf.h | 3 +-- 4 files changed, 12 insertions(+), 43 deletions(-) diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c index dba528e6..08973934 100644 --- a/libxfs/xfs_attr.c +++ b/libxfs/xfs_attr.c @@ -48,7 +48,7 @@ STATIC int xfs_attr_shortform_addname(xfs_da_args_t *args); STATIC int xfs_attr_leaf_get(xfs_da_args_t *args); STATIC int xfs_attr_leaf_removename(xfs_da_args_t *args); STATIC int xfs_attr_leaf_hasname(struct xfs_da_args *args, struct xfs_buf **bp); -STATIC int xfs_attr_leaf_try_add(struct xfs_da_args *args, struct xfs_buf *bp); +STATIC int xfs_attr_leaf_try_add(struct xfs_da_args *args); /* * Internal routines when attribute list is more than one block. @@ -391,16 +391,10 @@ xfs_attr_sf_addname( * It won't fit in the shortform, transform to a leaf block. GROT: * another possible req'mt for a double-split btree op. */ - error = xfs_attr_shortform_to_leaf(args, &attr->xattri_leaf_bp); + error = xfs_attr_shortform_to_leaf(args); if (error) return error; - /* - * Prevent the leaf buffer from being unlocked so that a concurrent AIL - * push cannot grab the half-baked leaf buffer and run into problems - * with the write verifier. - */ - xfs_trans_bhold(args->trans, attr->xattri_leaf_bp); attr->xattri_dela_state = XFS_DAS_LEAF_ADD; out: trace_xfs_attr_sf_addname_return(attr->xattri_dela_state, args->dp); @@ -445,11 +439,9 @@ xfs_attr_leaf_addname( /* * Use the leaf buffer we may already hold locked as a result of - * a sf-to-leaf conversion. The held buffer is no longer valid - * after this call, regardless of the result. + * a sf-to-leaf conversion. */ - error = xfs_attr_leaf_try_add(args, attr->xattri_leaf_bp); - attr->xattri_leaf_bp = NULL; + error = xfs_attr_leaf_try_add(args); if (error == -ENOSPC) { error = xfs_attr3_leaf_to_node(args); @@ -495,8 +487,6 @@ xfs_attr_node_addname( struct xfs_da_args *args = attr->xattri_da_args; int error; - ASSERT(!attr->xattri_leaf_bp); - error = xfs_attr_node_addname_find_attr(attr); if (error) return error; @@ -1213,24 +1203,14 @@ xfs_attr_restore_rmt_blk( */ STATIC int xfs_attr_leaf_try_add( - struct xfs_da_args *args, - struct xfs_buf *bp) + struct xfs_da_args *args) { + struct xfs_buf *bp; int error; - /* - * If the caller provided a buffer to us, it is locked and held in - * the transaction because it just did a shortform to leaf conversion. - * Hence we don't need to read it again. Otherwise read in the leaf - * buffer. - */ - if (bp) { - xfs_trans_bhold_release(args->trans, bp); - } else { - error = xfs_attr3_leaf_read(args->trans, args->dp, 0, &bp); - if (error) - return error; - } + error = xfs_attr3_leaf_read(args->trans, args->dp, 0, &bp); + if (error) + return error; /* * Look up the xattr name to set the insertion point for the new xattr. diff --git a/libxfs/xfs_attr.h b/libxfs/xfs_attr.h index b4a2fc77..dfb47fa6 100644 --- a/libxfs/xfs_attr.h +++ b/libxfs/xfs_attr.h @@ -515,11 +515,6 @@ struct xfs_attr_intent { */ struct xfs_attri_log_nameval *xattri_nameval; - /* - * Used by xfs_attr_set to hold a leaf buffer across a transaction roll - */ - struct xfs_buf *xattri_leaf_bp; - /* Used to keep track of current state of delayed operation */ enum xfs_delattr_state xattri_dela_state; diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c index 047ab01b..75d36141 100644 --- a/libxfs/xfs_attr_leaf.c +++ b/libxfs/xfs_attr_leaf.c @@ -927,14 +927,10 @@ xfs_attr_shortform_getvalue( return -ENOATTR; } -/* - * Convert from using the shortform to the leaf. On success, return the - * buffer so that we can keep it locked until we're totally done with it. - */ +/* Convert from using the shortform to the leaf format. */ int xfs_attr_shortform_to_leaf( - struct xfs_da_args *args, - struct xfs_buf **leaf_bp) + struct xfs_da_args *args) { struct xfs_inode *dp; struct xfs_attr_shortform *sf; @@ -996,7 +992,6 @@ xfs_attr_shortform_to_leaf( sfe = xfs_attr_sf_nextentry(sfe); } error = 0; - *leaf_bp = bp; out: kmem_free(tmpbuffer); return error; diff --git a/libxfs/xfs_attr_leaf.h b/libxfs/xfs_attr_leaf.h index efa757f1..368f4d9f 100644 --- a/libxfs/xfs_attr_leaf.h +++ b/libxfs/xfs_attr_leaf.h @@ -49,8 +49,7 @@ void xfs_attr_shortform_create(struct xfs_da_args *args); void xfs_attr_shortform_add(struct xfs_da_args *args, int forkoff); int xfs_attr_shortform_lookup(struct xfs_da_args *args); int xfs_attr_shortform_getvalue(struct xfs_da_args *args); -int xfs_attr_shortform_to_leaf(struct xfs_da_args *args, - struct xfs_buf **leaf_bp); +int xfs_attr_shortform_to_leaf(struct xfs_da_args *args); int xfs_attr_sf_removename(struct xfs_da_args *args); int xfs_attr_sf_findname(struct xfs_da_args *args, struct xfs_attr_sf_entry **sfep,