From patchwork Tue Apr 12 04:25:34 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 12810014 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 3F244C433FE for ; Tue, 12 Apr 2022 04:26:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345806AbiDLE2R (ORCPT ); Tue, 12 Apr 2022 00:28:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51522 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345813AbiDLE2K (ORCPT ); Tue, 12 Apr 2022 00:28:10 -0400 Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 2C95A329A0 for ; Mon, 11 Apr 2022 21:25:53 -0700 (PDT) Received: from dread.disaster.area (pa49-186-233-190.pa.vic.optusnet.com.au [49.186.233.190]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 70D8E10C7C14; Tue, 12 Apr 2022 14:25:47 +1000 (AEST) Received: from discord.disaster.area ([192.168.253.110]) by dread.disaster.area with esmtp (Exim 4.92.3) (envelope-from ) id 1ne865-00Gh2K-So; Tue, 12 Apr 2022 14:25:45 +1000 Received: from dave by discord.disaster.area with local (Exim 4.95) (envelope-from ) id 1ne865-009NrI-Rg; Tue, 12 Apr 2022 14:25:45 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Cc: allison.henderson@oracle.com Subject: [PATCH 01/10] xfs: avoid empty xattr transaction when attrs are inline Date: Tue, 12 Apr 2022 14:25:34 +1000 Message-Id: <20220412042543.2234866-2-david@fromorbit.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220412042543.2234866-1-david@fromorbit.com> References: <20220412042543.2234866-1-david@fromorbit.com> MIME-Version: 1.0 X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.4 cv=VuxAv86n c=1 sm=1 tr=0 ts=6254ff4b a=bHAvQTfMiaNt/bo4vVGwyA==:117 a=bHAvQTfMiaNt/bo4vVGwyA==:17 a=z0gMJWrwH1QA:10 a=20KFwNOVAAAA:8 a=z6sQ9X07RN1rA8HuyEwA:9 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner generic/642 triggered a reproducable assert failure in xlog_cil_commit() that resulted from a xfs_attr_set() committing an empty but dirty transaction. When the CIL is empty and this occurs, xlog_cil_commit() tries a background push and this triggers a "pushing an empty CIL" assert. XFS: Assertion failed: !list_empty(&cil->xc_cil), file: fs/xfs/xfs_log_cil.c, line: 1274 Call Trace: xlog_cil_commit+0xa5a/0xad0 __xfs_trans_commit+0xb8/0x330 xfs_trans_commit+0x10/0x20 xfs_attr_set+0x3e2/0x4c0 xfs_xattr_set+0x8d/0xe0 __vfs_setxattr+0x6b/0x90 __vfs_setxattr_noperm+0x76/0x220 __vfs_setxattr_locked+0xdf/0x100 vfs_setxattr+0x94/0x170 setxattr+0x110/0x200 path_setxattr+0xbf/0xe0 __x64_sys_setxattr+0x2b/0x30 do_syscall_64+0x35/0x80 The problem is related to the breakdown of attribute addition in xfs_attr_set_iter() and how it is called from deferred operations. When we have a pure leaf xattr insert, we add the xattr to the leaf and set the next state to XFS_DAS_FOUND_LBLK and return -EAGAIN. This requeues the xattr defered work, rolls the transaction and runs xfs_attr_set_iter() again. This then checks the xattr for being remote (it's not) and whether a replace op is being done (this is a create op) and if neither are true it returns without having done anything. xfs_xattri_finish_update() then unconditionally sets the transaction dirty, and the deferops finishes and returns to __xfs_trans_commit() which sees the transaction dirty and tries to commit it by calling xlog_cil_commit(). The transaction is empty, and then the assert fires if this happens when the CIL is empty. This patch addresses the structure of xfs_attr_set_iter() that requires re-entry on leaf add even when nothing will be done. This gets rid of the trailing empty transaction and so doesn't trigger the XFS_TRANS_DIRTY assignment in xfs_xattri_finish_update() incorrectly. Addressing that is for a different patch. Signed-off-by: Dave Chinner --- fs/xfs/libxfs/xfs_attr.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 48b7e7efbb30..b3d918195160 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -315,6 +315,7 @@ xfs_attr_leaf_addname( { struct xfs_da_args *args = attr->xattri_da_args; struct xfs_inode *dp = args->dp; + enum xfs_delattr_state next_state = XFS_DAS_UNINIT; int error; if (xfs_attr_is_leaf(dp)) { @@ -335,37 +336,35 @@ xfs_attr_leaf_addname( * when we come back, we'll be a node, so we'll fall * down into the node handling code below */ - trace_xfs_attr_set_iter_return( - attr->xattri_dela_state, args->dp); - return -EAGAIN; + error = -EAGAIN; + goto out; } - - if (error) - return error; - - attr->xattri_dela_state = XFS_DAS_FOUND_LBLK; + next_state = XFS_DAS_FOUND_LBLK; } else { error = xfs_attr_node_addname_find_attr(attr); if (error) return error; + next_state = XFS_DAS_FOUND_NBLK; error = xfs_attr_node_addname(attr); - if (error) - return error; - - /* - * If addname was successful, and we dont need to alloc or - * remove anymore blks, we're done. - */ - if (!args->rmtblkno && - !(args->op_flags & XFS_DA_OP_RENAME)) - return 0; + } + if (error) + return error; - attr->xattri_dela_state = XFS_DAS_FOUND_NBLK; + /* + * We need to commit and roll if we need to allocate remote xattr blocks + * or perform more xattr manipulations. Otherwise there is nothing more + * to do and we can return success. + */ + if (args->rmtblkno || + (args->op_flags & XFS_DA_OP_RENAME)) { + attr->xattri_dela_state = next_state; + error = -EAGAIN; } +out: trace_xfs_attr_leaf_addname_return(attr->xattri_dela_state, args->dp); - return -EAGAIN; + return error; } /* From patchwork Tue Apr 12 04:25:35 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 12810011 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 C0814C433FE for ; Tue, 12 Apr 2022 04:26:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345795AbiDLE2P (ORCPT ); Tue, 12 Apr 2022 00:28:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51536 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345839AbiDLE2L (ORCPT ); Tue, 12 Apr 2022 00:28:11 -0400 Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 5FA8F32992 for ; Mon, 11 Apr 2022 21:25:55 -0700 (PDT) Received: from dread.disaster.area (pa49-186-233-190.pa.vic.optusnet.com.au [49.186.233.190]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 9CE9C10C7C30; Tue, 12 Apr 2022 14:25:47 +1000 (AEST) Received: from discord.disaster.area ([192.168.253.110]) by dread.disaster.area with esmtp (Exim 4.92.3) (envelope-from ) id 1ne865-00Gh2M-Tg; Tue, 12 Apr 2022 14:25:45 +1000 Received: from dave by discord.disaster.area with local (Exim 4.95) (envelope-from ) id 1ne865-009NrM-Sk; Tue, 12 Apr 2022 14:25:45 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Cc: allison.henderson@oracle.com Subject: [PATCH 02/10] xfs: make xattri_leaf_bp more useful Date: Tue, 12 Apr 2022 14:25:35 +1000 Message-Id: <20220412042543.2234866-3-david@fromorbit.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220412042543.2234866-1-david@fromorbit.com> References: <20220412042543.2234866-1-david@fromorbit.com> MIME-Version: 1.0 X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.4 cv=VuxAv86n c=1 sm=1 tr=0 ts=6254ff4b a=bHAvQTfMiaNt/bo4vVGwyA==:117 a=bHAvQTfMiaNt/bo4vVGwyA==:17 a=z0gMJWrwH1QA:10 a=20KFwNOVAAAA:8 a=MSQD5IXnzoLRaKXEXdcA:9 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner We currently set it and hold it when converting from short to leaf form, then release it only to immediately look it back up again to do the leaf insert. Do a bit of refactoring to xfs_attr_leaf_try_add() to avoid this messy handling of the newly allocated leaf buffer. Signed-off-by: Dave Chinner --- fs/xfs/libxfs/xfs_attr.c | 50 +++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index b3d918195160..a4b0b20a3bab 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -319,7 +319,15 @@ xfs_attr_leaf_addname( int error; if (xfs_attr_is_leaf(dp)) { + + /* + * 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. + */ error = xfs_attr_leaf_try_add(args, attr->xattri_leaf_bp); + attr->xattri_leaf_bp = NULL; + if (error == -ENOSPC) { error = xfs_attr3_leaf_to_node(args); if (error) @@ -341,6 +349,8 @@ xfs_attr_leaf_addname( } next_state = XFS_DAS_FOUND_LBLK; } else { + ASSERT(!attr->xattri_leaf_bp); + error = xfs_attr_node_addname_find_attr(attr); if (error) return error; @@ -396,12 +406,6 @@ xfs_attr_set_iter( */ if (xfs_attr_is_shortform(dp)) return xfs_attr_sf_addname(attr); - if (attr->xattri_leaf_bp != NULL) { - xfs_trans_bhold_release(args->trans, - attr->xattri_leaf_bp); - attr->xattri_leaf_bp = NULL; - } - return xfs_attr_leaf_addname(attr); case XFS_DAS_FOUND_LBLK: @@ -992,18 +996,31 @@ xfs_attr_leaf_try_add( struct xfs_da_args *args, struct xfs_buf *bp) { - int retval; + int error; /* - * Look up the given attribute in the leaf block. Figure out if - * the given flags produce an error or call for an atomic rename. + * 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. */ - retval = xfs_attr_leaf_hasname(args, &bp); - if (retval != -ENOATTR && retval != -EEXIST) - return retval; - if (retval == -ENOATTR && (args->attr_flags & XATTR_REPLACE)) + 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; + } + + /* + * Look up the xattr name to set the insertion point for the new xattr. + */ + error = xfs_attr3_leaf_lookup_int(bp, args); + if (error != -ENOATTR && error != -EEXIST) goto out_brelse; - if (retval == -EEXIST) { + if (error == -ENOATTR && (args->attr_flags & XATTR_REPLACE)) + goto out_brelse; + if (error == -EEXIST) { if (args->attr_flags & XATTR_CREATE) goto out_brelse; @@ -1023,14 +1040,11 @@ xfs_attr_leaf_try_add( args->rmtvaluelen = 0; } - /* - * Add the attribute to the leaf block - */ return xfs_attr3_leaf_add(bp, args); out_brelse: xfs_trans_brelse(args->trans, bp); - return retval; + return error; } /* From patchwork Tue Apr 12 04:25:36 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 12810007 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 BBED3C433EF for ; Tue, 12 Apr 2022 04:25:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244464AbiDLE2N (ORCPT ); Tue, 12 Apr 2022 00:28:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51502 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344502AbiDLE2J (ORCPT ); Tue, 12 Apr 2022 00:28:09 -0400 Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 0C4363298F for ; Mon, 11 Apr 2022 21:25:49 -0700 (PDT) Received: from dread.disaster.area (pa49-186-233-190.pa.vic.optusnet.com.au [49.186.233.190]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 655B2534696; Tue, 12 Apr 2022 14:25:47 +1000 (AEST) Received: from discord.disaster.area ([192.168.253.110]) by dread.disaster.area with esmtp (Exim 4.92.3) (envelope-from ) id 1ne865-00Gh2P-Ux; Tue, 12 Apr 2022 14:25:45 +1000 Received: from dave by discord.disaster.area with local (Exim 4.95) (envelope-from ) id 1ne865-009NrQ-Tl; Tue, 12 Apr 2022 14:25:45 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Cc: allison.henderson@oracle.com Subject: [PATCH 03/10] xfs: separate out initial attr_set states Date: Tue, 12 Apr 2022 14:25:36 +1000 Message-Id: <20220412042543.2234866-4-david@fromorbit.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220412042543.2234866-1-david@fromorbit.com> References: <20220412042543.2234866-1-david@fromorbit.com> MIME-Version: 1.0 X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.4 cv=deDjYVbe c=1 sm=1 tr=0 ts=6254ff4b a=bHAvQTfMiaNt/bo4vVGwyA==:117 a=bHAvQTfMiaNt/bo4vVGwyA==:17 a=z0gMJWrwH1QA:10 a=20KFwNOVAAAA:8 a=1pSoUot-eWrqs-dKmUYA:9 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner We current use XFS_DAS_UNINIT for several steps in the attr_set state machine. We use it for setting shortform xattrs, converting from shortform to leaf, leaf add, leaf-to-node and leaf add. All of these things are essentially known before we start the state machine iterating, so we really should separate them out: XFS_DAS_SF_ADD: - tries to do a shortform add - on success -> done - on ENOSPC converts to leaf, -> XFS_DAS_LEAF_ADD - on error, dies. XFS_DAS_LEAF_ADD: - tries to do leaf add - on success: - inline attr -> done - remote xattr || REPLACE -> XFS_DAS_FOUND_LBLK - on ENOSPC converts to node, -> XFS_DAS_NODE_ADD - on error, dies XFS_DAS_NODE_ADD: - tries to do node add - on success: - inline attr -> done - remote xattr || REPLACE -> XFS_DAS_FOUND_NBLK - on error, dies This makes it easier to understand how the state machine starts up and sets us up on the path to further state machine simplifications. This also converts the DAS state tracepoints to use strings rather than numbers, as converting between enums and numbers requires manual counting rather than just reading the name. This also introduces a XFS_DAS_DONE state so that we can trace successful operation completions easily. Signed-off-by: Dave Chinner --- fs/xfs/libxfs/xfs_attr.c | 151 ++++++++++++++++++++++----------------- fs/xfs/libxfs/xfs_attr.h | 49 +++++++++---- fs/xfs/xfs_trace.h | 22 +++++- 3 files changed, 140 insertions(+), 82 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index a4b0b20a3bab..b0bbf827fbca 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -59,7 +59,7 @@ STATIC int xfs_attr_leaf_try_add(struct xfs_da_args *args, struct xfs_buf *bp); */ STATIC int xfs_attr_node_get(xfs_da_args_t *args); STATIC void xfs_attr_restore_rmt_blk(struct xfs_da_args *args); -STATIC int xfs_attr_node_addname(struct xfs_attr_item *attr); +static int xfs_attr_node_try_addname(struct xfs_attr_item *attr); STATIC int xfs_attr_node_addname_find_attr(struct xfs_attr_item *attr); STATIC int xfs_attr_node_addname_clear_incomplete(struct xfs_attr_item *attr); STATIC int xfs_attr_node_hasname(xfs_da_args_t *args, @@ -224,6 +224,11 @@ xfs_init_attr_trans( } } +/* + * Add an attr to a shortform fork. If there is no space, + * xfs_attr_shortform_addname() will convert to leaf format and return -ENOSPC. + * to use. + */ STATIC int xfs_attr_try_sf_addname( struct xfs_inode *dp, @@ -268,7 +273,7 @@ xfs_attr_is_shortform( ip->i_afp->if_nextents == 0); } -STATIC int +static int xfs_attr_sf_addname( struct xfs_attr_item *attr) { @@ -276,14 +281,12 @@ xfs_attr_sf_addname( struct xfs_inode *dp = args->dp; int error = 0; - /* - * Try to add the attr to the attribute list in the inode. - */ error = xfs_attr_try_sf_addname(dp, args); - - /* Should only be 0, -EEXIST or -ENOSPC */ - if (error != -ENOSPC) - return error; + if (error != -ENOSPC) { + ASSERT(!error || error == -EEXIST); + attr->xattri_dela_state = XFS_DAS_DONE; + goto out; + } /* * It won't fit in the shortform, transform to a leaf block. GROT: @@ -299,64 +302,42 @@ xfs_attr_sf_addname( * with the write verifier. */ xfs_trans_bhold(args->trans, attr->xattri_leaf_bp); - - /* - * We're still in XFS_DAS_UNINIT state here. We've converted - * the attr fork to leaf format and will restart with the leaf - * add. - */ - trace_xfs_attr_sf_addname_return(XFS_DAS_UNINIT, args->dp); - return -EAGAIN; + attr->xattri_dela_state = XFS_DAS_LEAF_ADD; + error = -EAGAIN; +out: + trace_xfs_attr_sf_addname_return(attr->xattri_dela_state, args->dp); + return error; } -STATIC int +static int xfs_attr_leaf_addname( struct xfs_attr_item *attr) { struct xfs_da_args *args = attr->xattri_da_args; - struct xfs_inode *dp = args->dp; - enum xfs_delattr_state next_state = XFS_DAS_UNINIT; int error; - if (xfs_attr_is_leaf(dp)) { + ASSERT(xfs_attr_is_leaf(args->dp)); - /* - * 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. - */ - error = xfs_attr_leaf_try_add(args, attr->xattri_leaf_bp); - attr->xattri_leaf_bp = NULL; - - if (error == -ENOSPC) { - error = xfs_attr3_leaf_to_node(args); - if (error) - return error; - - /* - * Finish any deferred work items and roll the - * transaction once more. The goal here is to call - * node_addname with the inode and transaction in the - * same state (inode locked and joined, transaction - * clean) no matter how we got to this step. - * - * At this point, we are still in XFS_DAS_UNINIT, but - * when we come back, we'll be a node, so we'll fall - * down into the node handling code below - */ - error = -EAGAIN; - goto out; - } - next_state = XFS_DAS_FOUND_LBLK; - } else { - ASSERT(!attr->xattri_leaf_bp); + /* + * 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. + */ + error = xfs_attr_leaf_try_add(args, attr->xattri_leaf_bp); + attr->xattri_leaf_bp = NULL; - error = xfs_attr_node_addname_find_attr(attr); + if (error == -ENOSPC) { + error = xfs_attr3_leaf_to_node(args); if (error) return error; - next_state = XFS_DAS_FOUND_NBLK; - error = xfs_attr_node_addname(attr); + /* + * We're not in leaf format anymore, so roll the transaction and + * retry the add to the newly allocated node block. + */ + attr->xattri_dela_state = XFS_DAS_NODE_ADD; + error = -EAGAIN; + goto out; } if (error) return error; @@ -368,15 +349,46 @@ xfs_attr_leaf_addname( */ if (args->rmtblkno || (args->op_flags & XFS_DA_OP_RENAME)) { - attr->xattri_dela_state = next_state; + attr->xattri_dela_state = XFS_DAS_FOUND_LBLK; error = -EAGAIN; + } else { + attr->xattri_dela_state = XFS_DAS_DONE; } - out: trace_xfs_attr_leaf_addname_return(attr->xattri_dela_state, args->dp); return error; } +static int +xfs_attr_node_addname( + struct xfs_attr_item *attr) +{ + 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; + + error = xfs_attr_node_try_addname(attr); + if (error) + return error; + + if (args->rmtblkno || + (args->op_flags & XFS_DA_OP_RENAME)) { + attr->xattri_dela_state = XFS_DAS_FOUND_NBLK; + error = -EAGAIN; + } else { + attr->xattri_dela_state = XFS_DAS_DONE; + } + + trace_xfs_attr_node_addname_return(attr->xattri_dela_state, args->dp); + return error; +} + + /* * Set the attribute specified in @args. * This routine is meant to function as a delayed operation, and may return @@ -397,16 +409,14 @@ xfs_attr_set_iter( /* State machine switch */ switch (attr->xattri_dela_state) { case XFS_DAS_UNINIT: - /* - * If the fork is shortform, attempt to add the attr. If there - * is no space, this converts to leaf format and returns - * -EAGAIN with the leaf buffer held across the roll. The caller - * will deal with a transaction roll error, but otherwise - * release the hold once we return with a clean transaction. - */ - if (xfs_attr_is_shortform(dp)) - return xfs_attr_sf_addname(attr); + ASSERT(0); + return -EFSCORRUPTED; + case XFS_DAS_SF_ADD: + return xfs_attr_sf_addname(attr); + case XFS_DAS_LEAF_ADD: return xfs_attr_leaf_addname(attr); + case XFS_DAS_NODE_ADD: + return xfs_attr_node_addname(attr); case XFS_DAS_FOUND_LBLK: /* @@ -874,6 +884,13 @@ xfs_attr_set_deferred( if (error) return error; + if (xfs_attr_is_shortform(args->dp)) + new->xattri_dela_state = XFS_DAS_SF_ADD; + else if (xfs_attr_is_leaf(args->dp)) + new->xattri_dela_state = XFS_DAS_LEAF_ADD; + else + new->xattri_dela_state = XFS_DAS_NODE_ADD; + xfs_defer_add(args->trans, XFS_DEFER_OPS_TYPE_ATTR, &new->xattri_list); return 0; @@ -1233,8 +1250,8 @@ xfs_attr_node_addname_find_attr( * to handle this, and recall the function until a successful error code is *returned. */ -STATIC int -xfs_attr_node_addname( +static int +xfs_attr_node_try_addname( struct xfs_attr_item *attr) { struct xfs_da_args *args = attr->xattri_da_args; diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h index f6c13d2bfbcd..fc2a177f6994 100644 --- a/fs/xfs/libxfs/xfs_attr.h +++ b/fs/xfs/libxfs/xfs_attr.h @@ -443,21 +443,44 @@ struct xfs_attr_list_context { * to where it was and resume executing where it left off. */ enum xfs_delattr_state { - XFS_DAS_UNINIT = 0, /* No state has been set yet */ - XFS_DAS_RMTBLK, /* Removing remote blks */ - XFS_DAS_RM_NAME, /* Remove attr name */ - XFS_DAS_RM_SHRINK, /* We are shrinking the tree */ - XFS_DAS_FOUND_LBLK, /* We found leaf blk for attr */ - XFS_DAS_FOUND_NBLK, /* We found node blk for attr */ - XFS_DAS_FLIP_LFLAG, /* Flipped leaf INCOMPLETE attr flag */ - XFS_DAS_RM_LBLK, /* A rename is removing leaf blocks */ - XFS_DAS_RD_LEAF, /* Read in the new leaf */ - XFS_DAS_ALLOC_NODE, /* We are allocating node blocks */ - XFS_DAS_FLIP_NFLAG, /* Flipped node INCOMPLETE attr flag */ - XFS_DAS_RM_NBLK, /* A rename is removing node blocks */ - XFS_DAS_CLR_FLAG, /* Clear incomplete flag */ + XFS_DAS_UNINIT = 0, /* No state has been set yet */ + XFS_DAS_SF_ADD, /* Initial shortform set iter state */ + XFS_DAS_LEAF_ADD, /* Initial leaf form set iter state */ + XFS_DAS_NODE_ADD, /* Initial node form set iter state */ + XFS_DAS_RMTBLK, /* Removing remote blks */ + XFS_DAS_RM_NAME, /* Remove attr name */ + XFS_DAS_RM_SHRINK, /* We are shrinking the tree */ + XFS_DAS_FOUND_LBLK, /* We found leaf blk for attr */ + XFS_DAS_FOUND_NBLK, /* We found node blk for attr */ + XFS_DAS_FLIP_LFLAG, /* Flipped leaf INCOMPLETE attr flag */ + XFS_DAS_RM_LBLK, /* A rename is removing leaf blocks */ + XFS_DAS_RD_LEAF, /* Read in the new leaf */ + XFS_DAS_ALLOC_NODE, /* We are allocating node blocks */ + XFS_DAS_FLIP_NFLAG, /* Flipped node INCOMPLETE attr flag */ + XFS_DAS_RM_NBLK, /* A rename is removing node blocks */ + XFS_DAS_CLR_FLAG, /* Clear incomplete flag */ + XFS_DAS_DONE, /* finished operation */ }; +#define XFS_DAS_STRINGS \ + { XFS_DAS_UNINIT, "XFS_DAS_UNINIT" }, \ + { XFS_DAS_SF_ADD, "XFS_DAS_SF_ADD" }, \ + { XFS_DAS_LEAF_ADD, "XFS_DAS_LEAF_ADD" }, \ + { XFS_DAS_NODE_ADD, "XFS_DAS_NODE_ADD" }, \ + { XFS_DAS_RMTBLK, "XFS_DAS_RMTBLK" }, \ + { XFS_DAS_RM_NAME, "XFS_DAS_RM_NAME" }, \ + { XFS_DAS_RM_SHRINK, "XFS_DAS_RM_SHRINK" }, \ + { XFS_DAS_FOUND_LBLK, "XFS_DAS_FOUND_LBLK" }, \ + { XFS_DAS_FOUND_NBLK, "XFS_DAS_FOUND_NBLK" }, \ + { XFS_DAS_FLIP_LFLAG, "XFS_DAS_FLIP_LFLAG" }, \ + { XFS_DAS_RM_LBLK, "XFS_DAS_RM_LBLK" }, \ + { XFS_DAS_RD_LEAF, "XFS_DAS_RD_LEAF" }, \ + { XFS_DAS_ALLOC_NODE, "XFS_DAS_ALLOC_NODE" }, \ + { XFS_DAS_FLIP_NFLAG, "XFS_DAS_FLIP_NFLAG" }, \ + { XFS_DAS_RM_NBLK, "XFS_DAS_RM_NBLK" }, \ + { XFS_DAS_CLR_FLAG, "XFS_DAS_CLR_FLAG" }, \ + { XFS_DAS_DONE, "XFS_DAS_DONE" } + /* * Defines for xfs_attr_item.xattri_flags */ diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index 51e45341cf76..9fc3fe334b5f 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -4098,6 +4098,23 @@ DEFINE_ICLOG_EVENT(xlog_iclog_want_sync); DEFINE_ICLOG_EVENT(xlog_iclog_wait_on); DEFINE_ICLOG_EVENT(xlog_iclog_write); +TRACE_DEFINE_ENUM(XFS_DAS_UNINIT); +TRACE_DEFINE_ENUM(XFS_DAS_SF_ADD); +TRACE_DEFINE_ENUM(XFS_DAS_LEAF_ADD); +TRACE_DEFINE_ENUM(XFS_DAS_NODE_ADD); +TRACE_DEFINE_ENUM(XFS_DAS_RMTBLK); +TRACE_DEFINE_ENUM(XFS_DAS_RM_NAME); +TRACE_DEFINE_ENUM(XFS_DAS_RM_SHRINK); +TRACE_DEFINE_ENUM(XFS_DAS_FOUND_LBLK); +TRACE_DEFINE_ENUM(XFS_DAS_FOUND_NBLK); +TRACE_DEFINE_ENUM(XFS_DAS_FLIP_LFLAG); +TRACE_DEFINE_ENUM(XFS_DAS_RM_LBLK); +TRACE_DEFINE_ENUM(XFS_DAS_RD_LEAF); +TRACE_DEFINE_ENUM(XFS_DAS_ALLOC_NODE); +TRACE_DEFINE_ENUM(XFS_DAS_FLIP_NFLAG); +TRACE_DEFINE_ENUM(XFS_DAS_RM_NBLK); +TRACE_DEFINE_ENUM(XFS_DAS_CLR_FLAG); + DECLARE_EVENT_CLASS(xfs_das_state_class, TP_PROTO(int das, struct xfs_inode *ip), TP_ARGS(das, ip), @@ -4109,8 +4126,9 @@ DECLARE_EVENT_CLASS(xfs_das_state_class, __entry->das = das; __entry->ino = ip->i_ino; ), - TP_printk("state change %d ino 0x%llx", - __entry->das, __entry->ino) + TP_printk("state change %s ino 0x%llx", + __print_symbolic(__entry->das, XFS_DAS_STRINGS), + __entry->ino) ) #define DEFINE_DAS_STATE_EVENT(name) \ From patchwork Tue Apr 12 04:25:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 12810010 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 BA317C433F5 for ; Tue, 12 Apr 2022 04:26:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345794AbiDLE2P (ORCPT ); Tue, 12 Apr 2022 00:28:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51524 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345816AbiDLE2K (ORCPT ); Tue, 12 Apr 2022 00:28:10 -0400 Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 44B2C329A2 for ; Mon, 11 Apr 2022 21:25:53 -0700 (PDT) Received: from dread.disaster.area (pa49-186-233-190.pa.vic.optusnet.com.au [49.186.233.190]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 69F6A53469E; Tue, 12 Apr 2022 14:25:47 +1000 (AEST) Received: from discord.disaster.area ([192.168.253.110]) by dread.disaster.area with esmtp (Exim 4.92.3) (envelope-from ) id 1ne865-00Gh2R-W9; Tue, 12 Apr 2022 14:25:46 +1000 Received: from dave by discord.disaster.area with local (Exim 4.95) (envelope-from ) id 1ne865-009NrV-V0; Tue, 12 Apr 2022 14:25:45 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Cc: allison.henderson@oracle.com Subject: [PATCH 04/10] xfs: kill XFS_DAC_LEAF_ADDNAME_INIT Date: Tue, 12 Apr 2022 14:25:37 +1000 Message-Id: <20220412042543.2234866-5-david@fromorbit.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220412042543.2234866-1-david@fromorbit.com> References: <20220412042543.2234866-1-david@fromorbit.com> MIME-Version: 1.0 X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.4 cv=e9dl9Yl/ c=1 sm=1 tr=0 ts=6254ff4b a=bHAvQTfMiaNt/bo4vVGwyA==:117 a=bHAvQTfMiaNt/bo4vVGwyA==:17 a=z0gMJWrwH1QA:10 a=20KFwNOVAAAA:8 a=-5dJtcHi5CXNrSph2GIA:9 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner We re-enter the XFS_DAS_FOUND_LBLK state when we have to allocate multiple extents for a remote xattr. We currently have a flag called XFS_DAC_LEAF_ADDNAME_INIT to avoid running the remote attr hole finding code more than once. However, for the node format tree, we have a separate state for this so we never reenter the state machine at XFS_DAS_FOUND_NBLK and so it does not need a special flag to skip over the remote attr hold finding code. Convert the leaf block code to use the same state machine as the node blocks and kill the XFS_DAC_LEAF_ADDNAME_INIT flag. This further points out that this "ALLOC" state is only traversed if we have remote xattrs or we are doing a rename operation. Rename both the leaf and node alloc states to _ALLOC_RMT to indicate they are iterating to do allocation of remote xattr blocks. Signed-off-by: Dave Chinner --- fs/xfs/libxfs/xfs_attr.c | 45 ++++++++++++++++++++-------------------- fs/xfs/libxfs/xfs_attr.h | 6 ++++-- fs/xfs/xfs_trace.h | 3 ++- 3 files changed, 29 insertions(+), 25 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index b0bbf827fbca..fed476bd048e 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -419,40 +419,41 @@ xfs_attr_set_iter( return xfs_attr_node_addname(attr); case XFS_DAS_FOUND_LBLK: + /* + * Find space for remote blocks and fall into the allocation + * state. + */ + if (args->rmtblkno > 0) { + error = xfs_attr_rmtval_find_space(attr); + if (error) + return error; + } + attr->xattri_dela_state = XFS_DAS_LEAF_ALLOC_RMT; + fallthrough; + case XFS_DAS_LEAF_ALLOC_RMT: + /* * If there was an out-of-line value, allocate the blocks we * identified for its storage and copy the value. This is done * after we create the attribute so that we don't overflow the * maximum size of a transaction and/or hit a deadlock. */ - - /* Open coded xfs_attr_rmtval_set without trans handling */ - if ((attr->xattri_flags & XFS_DAC_LEAF_ADDNAME_INIT) == 0) { - attr->xattri_flags |= XFS_DAC_LEAF_ADDNAME_INIT; - if (args->rmtblkno > 0) { - error = xfs_attr_rmtval_find_space(attr); + if (args->rmtblkno > 0) { + if (attr->xattri_blkcnt > 0) { + error = xfs_attr_rmtval_set_blk(attr); if (error) return error; + trace_xfs_attr_set_iter_return( + attr->xattri_dela_state, + args->dp); + return -EAGAIN; } - } - /* - * Repeat allocating remote blocks for the attr value until - * blkcnt drops to zero. - */ - if (attr->xattri_blkcnt > 0) { - error = xfs_attr_rmtval_set_blk(attr); + error = xfs_attr_rmtval_set_value(args); if (error) return error; - trace_xfs_attr_set_iter_return(attr->xattri_dela_state, - args->dp); - return -EAGAIN; } - error = xfs_attr_rmtval_set_value(args); - if (error) - return error; - /* * If this is not a rename, clear the incomplete flag and we're * done. @@ -547,15 +548,15 @@ xfs_attr_set_iter( return error; } + attr->xattri_dela_state = XFS_DAS_NODE_ALLOC_RMT; fallthrough; - case XFS_DAS_ALLOC_NODE: + case XFS_DAS_NODE_ALLOC_RMT: /* * If there was an out-of-line value, allocate the blocks we * identified for its storage and copy the value. This is done * after we create the attribute so that we don't overflow the * maximum size of a transaction and/or hit a deadlock. */ - attr->xattri_dela_state = XFS_DAS_ALLOC_NODE; if (args->rmtblkno > 0) { if (attr->xattri_blkcnt > 0) { error = xfs_attr_rmtval_set_blk(attr); diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h index fc2a177f6994..184dca735cf3 100644 --- a/fs/xfs/libxfs/xfs_attr.h +++ b/fs/xfs/libxfs/xfs_attr.h @@ -451,11 +451,12 @@ enum xfs_delattr_state { XFS_DAS_RM_NAME, /* Remove attr name */ XFS_DAS_RM_SHRINK, /* We are shrinking the tree */ XFS_DAS_FOUND_LBLK, /* We found leaf blk for attr */ + XFS_DAS_LEAF_ALLOC_RMT, /* We are allocating remote blocks */ XFS_DAS_FOUND_NBLK, /* We found node blk for attr */ + XFS_DAS_NODE_ALLOC_RMT, /* We are allocating remote blocks */ XFS_DAS_FLIP_LFLAG, /* Flipped leaf INCOMPLETE attr flag */ XFS_DAS_RM_LBLK, /* A rename is removing leaf blocks */ XFS_DAS_RD_LEAF, /* Read in the new leaf */ - XFS_DAS_ALLOC_NODE, /* We are allocating node blocks */ XFS_DAS_FLIP_NFLAG, /* Flipped node INCOMPLETE attr flag */ XFS_DAS_RM_NBLK, /* A rename is removing node blocks */ XFS_DAS_CLR_FLAG, /* Clear incomplete flag */ @@ -471,11 +472,12 @@ enum xfs_delattr_state { { XFS_DAS_RM_NAME, "XFS_DAS_RM_NAME" }, \ { XFS_DAS_RM_SHRINK, "XFS_DAS_RM_SHRINK" }, \ { XFS_DAS_FOUND_LBLK, "XFS_DAS_FOUND_LBLK" }, \ + { XFS_DAS_LEAF_ALLOC_RMT, "XFS_DAS_LEAF_ALLOC_RMT" }, \ { XFS_DAS_FOUND_NBLK, "XFS_DAS_FOUND_NBLK" }, \ + { XFS_DAS_NODE_ALLOC_RMT, "XFS_DAS_NODE_ALLOC_RMT" }, \ { XFS_DAS_FLIP_LFLAG, "XFS_DAS_FLIP_LFLAG" }, \ { XFS_DAS_RM_LBLK, "XFS_DAS_RM_LBLK" }, \ { XFS_DAS_RD_LEAF, "XFS_DAS_RD_LEAF" }, \ - { XFS_DAS_ALLOC_NODE, "XFS_DAS_ALLOC_NODE" }, \ { XFS_DAS_FLIP_NFLAG, "XFS_DAS_FLIP_NFLAG" }, \ { XFS_DAS_RM_NBLK, "XFS_DAS_RM_NBLK" }, \ { XFS_DAS_CLR_FLAG, "XFS_DAS_CLR_FLAG" }, \ diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index 9fc3fe334b5f..8739cc1e0561 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -4106,11 +4106,12 @@ TRACE_DEFINE_ENUM(XFS_DAS_RMTBLK); TRACE_DEFINE_ENUM(XFS_DAS_RM_NAME); TRACE_DEFINE_ENUM(XFS_DAS_RM_SHRINK); TRACE_DEFINE_ENUM(XFS_DAS_FOUND_LBLK); +TRACE_DEFINE_ENUM(XFS_DAS_LEAF_ALLOC_RMT); TRACE_DEFINE_ENUM(XFS_DAS_FOUND_NBLK); +TRACE_DEFINE_ENUM(XFS_DAS_NODE_ALLOC_RMT); TRACE_DEFINE_ENUM(XFS_DAS_FLIP_LFLAG); TRACE_DEFINE_ENUM(XFS_DAS_RM_LBLK); TRACE_DEFINE_ENUM(XFS_DAS_RD_LEAF); -TRACE_DEFINE_ENUM(XFS_DAS_ALLOC_NODE); TRACE_DEFINE_ENUM(XFS_DAS_FLIP_NFLAG); TRACE_DEFINE_ENUM(XFS_DAS_RM_NBLK); TRACE_DEFINE_ENUM(XFS_DAS_CLR_FLAG); From patchwork Tue Apr 12 04:25:38 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 12810013 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 B3394C433F5 for ; Tue, 12 Apr 2022 04:26:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345827AbiDLE2R (ORCPT ); Tue, 12 Apr 2022 00:28:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51520 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345806AbiDLE2K (ORCPT ); Tue, 12 Apr 2022 00:28:10 -0400 Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 2C32B3299A for ; Mon, 11 Apr 2022 21:25:53 -0700 (PDT) Received: from dread.disaster.area (pa49-186-233-190.pa.vic.optusnet.com.au [49.186.233.190]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 6DA7310C7C0A; Tue, 12 Apr 2022 14:25:47 +1000 (AEST) Received: from discord.disaster.area ([192.168.253.110]) by dread.disaster.area with esmtp (Exim 4.92.3) (envelope-from ) id 1ne866-00Gh2T-0s; Tue, 12 Apr 2022 14:25:46 +1000 Received: from dave by discord.disaster.area with local (Exim 4.95) (envelope-from ) id 1ne865-009Nrb-W5; Tue, 12 Apr 2022 14:25:46 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Cc: allison.henderson@oracle.com Subject: [PATCH 05/10] xfs: consolidate leaf/node states in xfs_attr_set_iter Date: Tue, 12 Apr 2022 14:25:38 +1000 Message-Id: <20220412042543.2234866-6-david@fromorbit.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220412042543.2234866-1-david@fromorbit.com> References: <20220412042543.2234866-1-david@fromorbit.com> MIME-Version: 1.0 X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.4 cv=VuxAv86n c=1 sm=1 tr=0 ts=6254ff4b a=bHAvQTfMiaNt/bo4vVGwyA==:117 a=bHAvQTfMiaNt/bo4vVGwyA==:17 a=z0gMJWrwH1QA:10 a=20KFwNOVAAAA:8 a=sSY1SZa-z43ULZiO_Y8A:9 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner The operations performed from XFS_DAS_FOUND_LBLK through to XFS_DAS_RM_LBLK are now identical to XFS_DAS_FOUND_NBLK through to XFS_DAS_RM_NBLK. We can collapse these down into a single set of code. To do this, define the states that leaf and node run through as separate sets of sequential states. Then as we move to the next state, we can use increments rather than specific state assignments to move through the states. This means the state progression is set by the initial state that enters the series and we don't need to duplicate the code anymore. At the exit point of the series we need to select the correct leaf or node state, but that can also be done by state increment rather than assignment. Signed-off-by: Dave Chinner --- fs/xfs/libxfs/xfs_attr.c | 127 ++++++--------------------------------- fs/xfs/libxfs/xfs_attr.h | 9 ++- 2 files changed, 27 insertions(+), 109 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index fed476bd048e..655e4388dfec 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -407,6 +407,7 @@ xfs_attr_set_iter( struct xfs_mount *mp = args->dp->i_mount; /* State machine switch */ +next_state: switch (attr->xattri_dela_state) { case XFS_DAS_UNINIT: ASSERT(0); @@ -419,6 +420,7 @@ xfs_attr_set_iter( return xfs_attr_node_addname(attr); case XFS_DAS_FOUND_LBLK: + case XFS_DAS_FOUND_NBLK: /* * Find space for remote blocks and fall into the allocation * state. @@ -428,9 +430,10 @@ xfs_attr_set_iter( if (error) return error; } - attr->xattri_dela_state = XFS_DAS_LEAF_ALLOC_RMT; + attr->xattri_dela_state++; fallthrough; case XFS_DAS_LEAF_ALLOC_RMT: + case XFS_DAS_NODE_ALLOC_RMT: /* * If there was an out-of-line value, allocate the blocks we @@ -479,16 +482,18 @@ xfs_attr_set_iter( return error; /* * Commit the flag value change and start the next trans - * in series. + * in series at FLIP_FLAG. */ - attr->xattri_dela_state = XFS_DAS_FLIP_LFLAG; + attr->xattri_dela_state++; trace_xfs_attr_set_iter_return(attr->xattri_dela_state, args->dp); return -EAGAIN; } + attr->xattri_dela_state++; fallthrough; case XFS_DAS_FLIP_LFLAG: + case XFS_DAS_FLIP_NFLAG: /* * Dismantle the "old" attribute/value pair by removing a * "remote" value (if it exists). @@ -498,10 +503,10 @@ xfs_attr_set_iter( if (error) return error; + attr->xattri_dela_state++; fallthrough; case XFS_DAS_RM_LBLK: - /* Set state in case xfs_attr_rmtval_remove returns -EAGAIN */ - attr->xattri_dela_state = XFS_DAS_RM_LBLK; + case XFS_DAS_RM_NBLK: if (args->rmtblkno) { error = xfs_attr_rmtval_remove(attr); if (error == -EAGAIN) @@ -516,7 +521,16 @@ xfs_attr_set_iter( return -EAGAIN; } - fallthrough; + /* + * This is the end of the shared leaf/node sequence. We need + * to continue at the next state in the sequence, but we can't + * easily just fall through. So we increment to the next state + * and then jump back to switch statement to evaluate the next + * state correctly. + */ + attr->xattri_dela_state++; + goto next_state; + case XFS_DAS_RD_LEAF: /* * This is the last step for leaf format. Read the block with @@ -537,106 +551,6 @@ xfs_attr_set_iter( return error; - case XFS_DAS_FOUND_NBLK: - /* - * Find space for remote blocks and fall into the allocation - * state. - */ - if (args->rmtblkno > 0) { - error = xfs_attr_rmtval_find_space(attr); - if (error) - return error; - } - - attr->xattri_dela_state = XFS_DAS_NODE_ALLOC_RMT; - fallthrough; - case XFS_DAS_NODE_ALLOC_RMT: - /* - * If there was an out-of-line value, allocate the blocks we - * identified for its storage and copy the value. This is done - * after we create the attribute so that we don't overflow the - * maximum size of a transaction and/or hit a deadlock. - */ - if (args->rmtblkno > 0) { - if (attr->xattri_blkcnt > 0) { - error = xfs_attr_rmtval_set_blk(attr); - if (error) - return error; - trace_xfs_attr_set_iter_return( - attr->xattri_dela_state, args->dp); - return -EAGAIN; - } - - error = xfs_attr_rmtval_set_value(args); - if (error) - return error; - } - - /* - * If this was not a rename, clear the incomplete flag and we're - * done. - */ - if (!(args->op_flags & XFS_DA_OP_RENAME)) { - if (args->rmtblkno > 0) - error = xfs_attr3_leaf_clearflag(args); - goto out; - } - - /* - * If this is an atomic rename operation, we must "flip" the - * incomplete flags on the "new" and "old" attribute/value pairs - * so that one disappears and one appears atomically. Then we - * must remove the "old" attribute/value pair. - * - * In a separate transaction, set the incomplete flag on the - * "old" attr and clear the incomplete flag on the "new" attr. - */ - if (!xfs_has_larp(mp)) { - error = xfs_attr3_leaf_flipflags(args); - if (error) - goto out; - /* - * Commit the flag value change and start the next trans - * in series - */ - attr->xattri_dela_state = XFS_DAS_FLIP_NFLAG; - trace_xfs_attr_set_iter_return(attr->xattri_dela_state, - args->dp); - return -EAGAIN; - } - - fallthrough; - case XFS_DAS_FLIP_NFLAG: - /* - * Dismantle the "old" attribute/value pair by removing a - * "remote" value (if it exists). - */ - xfs_attr_restore_rmt_blk(args); - - error = xfs_attr_rmtval_invalidate(args); - if (error) - return error; - - fallthrough; - case XFS_DAS_RM_NBLK: - /* Set state in case xfs_attr_rmtval_remove returns -EAGAIN */ - attr->xattri_dela_state = XFS_DAS_RM_NBLK; - if (args->rmtblkno) { - error = xfs_attr_rmtval_remove(attr); - if (error == -EAGAIN) - trace_xfs_attr_set_iter_return( - attr->xattri_dela_state, args->dp); - - if (error) - return error; - - attr->xattri_dela_state = XFS_DAS_CLR_FLAG; - trace_xfs_attr_set_iter_return(attr->xattri_dela_state, - args->dp); - return -EAGAIN; - } - - fallthrough; case XFS_DAS_CLR_FLAG: /* * The last state for node format. Look up the old attr and @@ -648,7 +562,6 @@ xfs_attr_set_iter( ASSERT(0); break; } -out: return error; } diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h index 184dca735cf3..0ad78f9279ac 100644 --- a/fs/xfs/libxfs/xfs_attr.h +++ b/fs/xfs/libxfs/xfs_attr.h @@ -450,16 +450,21 @@ enum xfs_delattr_state { XFS_DAS_RMTBLK, /* Removing remote blks */ XFS_DAS_RM_NAME, /* Remove attr name */ XFS_DAS_RM_SHRINK, /* We are shrinking the tree */ + + /* Leaf state set sequence */ XFS_DAS_FOUND_LBLK, /* We found leaf blk for attr */ XFS_DAS_LEAF_ALLOC_RMT, /* We are allocating remote blocks */ - XFS_DAS_FOUND_NBLK, /* We found node blk for attr */ - XFS_DAS_NODE_ALLOC_RMT, /* We are allocating remote blocks */ XFS_DAS_FLIP_LFLAG, /* Flipped leaf INCOMPLETE attr flag */ XFS_DAS_RM_LBLK, /* A rename is removing leaf blocks */ XFS_DAS_RD_LEAF, /* Read in the new leaf */ + + /* Node state set sequence, must match leaf state above */ + XFS_DAS_FOUND_NBLK, /* We found node blk for attr */ + XFS_DAS_NODE_ALLOC_RMT, /* We are allocating remote blocks */ XFS_DAS_FLIP_NFLAG, /* Flipped node INCOMPLETE attr flag */ XFS_DAS_RM_NBLK, /* A rename is removing node blocks */ XFS_DAS_CLR_FLAG, /* Clear incomplete flag */ + XFS_DAS_DONE, /* finished operation */ }; From patchwork Tue Apr 12 04:25:39 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 12810012 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 1D3FAC43217 for ; Tue, 12 Apr 2022 04:26:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345839AbiDLE2Q (ORCPT ); Tue, 12 Apr 2022 00:28:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51534 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345827AbiDLE2L (ORCPT ); Tue, 12 Apr 2022 00:28:11 -0400 Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 02CFB32991 for ; Mon, 11 Apr 2022 21:25:55 -0700 (PDT) Received: from dread.disaster.area (pa49-186-233-190.pa.vic.optusnet.com.au [49.186.233.190]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 9BAB610C7C1B; Tue, 12 Apr 2022 14:25:47 +1000 (AEST) Received: from discord.disaster.area ([192.168.253.110]) by dread.disaster.area with esmtp (Exim 4.92.3) (envelope-from ) id 1ne866-00Gh2V-20; Tue, 12 Apr 2022 14:25:46 +1000 Received: from dave by discord.disaster.area with local (Exim 4.95) (envelope-from ) id 1ne866-009Nrf-0w; Tue, 12 Apr 2022 14:25:46 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Cc: allison.henderson@oracle.com Subject: [PATCH 06/10] xfs: split remote attr setting out from replace path Date: Tue, 12 Apr 2022 14:25:39 +1000 Message-Id: <20220412042543.2234866-7-david@fromorbit.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220412042543.2234866-1-david@fromorbit.com> References: <20220412042543.2234866-1-david@fromorbit.com> MIME-Version: 1.0 X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.4 cv=deDjYVbe c=1 sm=1 tr=0 ts=6254ff4b a=bHAvQTfMiaNt/bo4vVGwyA==:117 a=bHAvQTfMiaNt/bo4vVGwyA==:17 a=z0gMJWrwH1QA:10 a=20KFwNOVAAAA:8 a=58BS8la1inLbC0avub8A:9 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner When we set a new xattr, we have three exit paths: 1. nothign else to do 2. allocate and set the remote xattr value 3. perform the rest of a replace operation Currently we push both 2 and 3 into the same state, regardless of whether we just set a remote attribute or not. Once we've set the remote xattr, we have two exit states: 1. nothign else to do 2. perform the rest of a replace operation Hence we can split the remote xattr allocation and setting into their own states and factor it out of xfs_attr_set_iter() to further clean up the state machine and the implementation of the state machine. Signed-off-by: Dave Chinner --- fs/xfs/libxfs/xfs_attr.c | 113 +++++++++++++++++++++------------------ fs/xfs/libxfs/xfs_attr.h | 14 +++-- fs/xfs/xfs_trace.h | 9 ++-- 3 files changed, 77 insertions(+), 59 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 655e4388dfec..4b9c107fe5de 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -347,9 +347,11 @@ xfs_attr_leaf_addname( * or perform more xattr manipulations. Otherwise there is nothing more * to do and we can return success. */ - if (args->rmtblkno || - (args->op_flags & XFS_DA_OP_RENAME)) { - attr->xattri_dela_state = XFS_DAS_FOUND_LBLK; + if (args->rmtblkno) { + attr->xattri_dela_state = XFS_DAS_LEAF_SET_RMT; + error = -EAGAIN; + } else if (args->op_flags & XFS_DA_OP_RENAME) { + attr->xattri_dela_state = XFS_DAS_LEAF_REPLACE; error = -EAGAIN; } else { attr->xattri_dela_state = XFS_DAS_DONE; @@ -376,9 +378,11 @@ xfs_attr_node_addname( if (error) return error; - if (args->rmtblkno || - (args->op_flags & XFS_DA_OP_RENAME)) { - attr->xattri_dela_state = XFS_DAS_FOUND_NBLK; + if (args->rmtblkno) { + attr->xattri_dela_state = XFS_DAS_NODE_SET_RMT; + error = -EAGAIN; + } else if (args->op_flags & XFS_DA_OP_RENAME) { + attr->xattri_dela_state = XFS_DAS_NODE_REPLACE; error = -EAGAIN; } else { attr->xattri_dela_state = XFS_DAS_DONE; @@ -388,6 +392,40 @@ xfs_attr_node_addname( return error; } +static int +xfs_attr_rmtval_alloc( + struct xfs_attr_item *attr) +{ + struct xfs_da_args *args = attr->xattri_da_args; + int error = 0; + + /* + * If there was an out-of-line value, allocate the blocks we + * identified for its storage and copy the value. This is done + * after we create the attribute so that we don't overflow the + * maximum size of a transaction and/or hit a deadlock. + */ + if (attr->xattri_blkcnt > 0) { + error = xfs_attr_rmtval_set_blk(attr); + if (error) + return error; + error = -EAGAIN; + goto out; + } + + error = xfs_attr_rmtval_set_value(args); + if (error) + return error; + + /* If this is not a rename, clear the incomplete flag and we're done. */ + if (!(args->op_flags & XFS_DA_OP_RENAME)) { + error = xfs_attr3_leaf_clearflag(args); + attr->xattri_dela_state = XFS_DAS_DONE; + } +out: + trace_xfs_attr_rmtval_alloc(attr->xattri_dela_state, args->dp); + return error; +} /* * Set the attribute specified in @args. @@ -419,54 +457,26 @@ xfs_attr_set_iter( case XFS_DAS_NODE_ADD: return xfs_attr_node_addname(attr); - case XFS_DAS_FOUND_LBLK: - case XFS_DAS_FOUND_NBLK: - /* - * Find space for remote blocks and fall into the allocation - * state. - */ - if (args->rmtblkno > 0) { - error = xfs_attr_rmtval_find_space(attr); - if (error) - return error; - } + case XFS_DAS_LEAF_SET_RMT: + case XFS_DAS_NODE_SET_RMT: + error = xfs_attr_rmtval_find_space(attr); + if (error) + return error; attr->xattri_dela_state++; fallthrough; + case XFS_DAS_LEAF_ALLOC_RMT: case XFS_DAS_NODE_ALLOC_RMT: - - /* - * If there was an out-of-line value, allocate the blocks we - * identified for its storage and copy the value. This is done - * after we create the attribute so that we don't overflow the - * maximum size of a transaction and/or hit a deadlock. - */ - if (args->rmtblkno > 0) { - if (attr->xattri_blkcnt > 0) { - error = xfs_attr_rmtval_set_blk(attr); - if (error) - return error; - trace_xfs_attr_set_iter_return( - attr->xattri_dela_state, - args->dp); - return -EAGAIN; - } - - error = xfs_attr_rmtval_set_value(args); - if (error) - return error; - } - - /* - * If this is not a rename, clear the incomplete flag and we're - * done. - */ - if (!(args->op_flags & XFS_DA_OP_RENAME)) { - if (args->rmtblkno > 0) - error = xfs_attr3_leaf_clearflag(args); + error = xfs_attr_rmtval_alloc(attr); + if (error) return error; - } + if (attr->xattri_dela_state == XFS_DAS_DONE) + break; + attr->xattri_dela_state++; + fallthrough; + case XFS_DAS_LEAF_REPLACE: + case XFS_DAS_NODE_REPLACE: /* * If this is an atomic rename operation, we must "flip" the * incomplete flags on the "new" and "old" attribute/value pairs @@ -484,10 +494,9 @@ xfs_attr_set_iter( * Commit the flag value change and start the next trans * in series at FLIP_FLAG. */ + error = -EAGAIN; attr->xattri_dela_state++; - trace_xfs_attr_set_iter_return(attr->xattri_dela_state, - args->dp); - return -EAGAIN; + break; } attr->xattri_dela_state++; @@ -562,6 +571,8 @@ xfs_attr_set_iter( ASSERT(0); break; } + + trace_xfs_attr_set_iter_return(attr->xattri_dela_state, args->dp); return error; } diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h index 0ad78f9279ac..1de6c06b7f19 100644 --- a/fs/xfs/libxfs/xfs_attr.h +++ b/fs/xfs/libxfs/xfs_attr.h @@ -452,15 +452,17 @@ enum xfs_delattr_state { XFS_DAS_RM_SHRINK, /* We are shrinking the tree */ /* Leaf state set sequence */ - XFS_DAS_FOUND_LBLK, /* We found leaf blk for attr */ + XFS_DAS_LEAF_SET_RMT, /* set a remote xattr from a leaf */ XFS_DAS_LEAF_ALLOC_RMT, /* We are allocating remote blocks */ + XFS_DAS_LEAF_REPLACE, /* Perform replace ops on a leaf */ XFS_DAS_FLIP_LFLAG, /* Flipped leaf INCOMPLETE attr flag */ XFS_DAS_RM_LBLK, /* A rename is removing leaf blocks */ XFS_DAS_RD_LEAF, /* Read in the new leaf */ /* Node state set sequence, must match leaf state above */ - XFS_DAS_FOUND_NBLK, /* We found node blk for attr */ + XFS_DAS_NODE_SET_RMT, /* set a remote xattr from a node */ XFS_DAS_NODE_ALLOC_RMT, /* We are allocating remote blocks */ + XFS_DAS_NODE_REPLACE, /* Perform replace ops on a node */ XFS_DAS_FLIP_NFLAG, /* Flipped node INCOMPLETE attr flag */ XFS_DAS_RM_NBLK, /* A rename is removing node blocks */ XFS_DAS_CLR_FLAG, /* Clear incomplete flag */ @@ -476,13 +478,15 @@ enum xfs_delattr_state { { XFS_DAS_RMTBLK, "XFS_DAS_RMTBLK" }, \ { XFS_DAS_RM_NAME, "XFS_DAS_RM_NAME" }, \ { XFS_DAS_RM_SHRINK, "XFS_DAS_RM_SHRINK" }, \ - { XFS_DAS_FOUND_LBLK, "XFS_DAS_FOUND_LBLK" }, \ + { XFS_DAS_LEAF_SET_RMT, "XFS_DAS_LEAF_SET_RMT" }, \ { XFS_DAS_LEAF_ALLOC_RMT, "XFS_DAS_LEAF_ALLOC_RMT" }, \ - { XFS_DAS_FOUND_NBLK, "XFS_DAS_FOUND_NBLK" }, \ - { XFS_DAS_NODE_ALLOC_RMT, "XFS_DAS_NODE_ALLOC_RMT" }, \ + { XFS_DAS_LEAF_REPLACE, "XFS_DAS_LEAF_REPLACE" }, \ { XFS_DAS_FLIP_LFLAG, "XFS_DAS_FLIP_LFLAG" }, \ { XFS_DAS_RM_LBLK, "XFS_DAS_RM_LBLK" }, \ { XFS_DAS_RD_LEAF, "XFS_DAS_RD_LEAF" }, \ + { XFS_DAS_NODE_SET_RMT, "XFS_DAS_NODE_SET_RMT" }, \ + { XFS_DAS_NODE_ALLOC_RMT, "XFS_DAS_NODE_ALLOC_RMT" }, \ + { XFS_DAS_NODE_REPLACE, "XFS_DAS_NODE_REPLACE" }, \ { XFS_DAS_FLIP_NFLAG, "XFS_DAS_FLIP_NFLAG" }, \ { XFS_DAS_RM_NBLK, "XFS_DAS_RM_NBLK" }, \ { XFS_DAS_CLR_FLAG, "XFS_DAS_CLR_FLAG" }, \ diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index 8739cc1e0561..cf99efc5ba5a 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -4105,13 +4105,15 @@ TRACE_DEFINE_ENUM(XFS_DAS_NODE_ADD); TRACE_DEFINE_ENUM(XFS_DAS_RMTBLK); TRACE_DEFINE_ENUM(XFS_DAS_RM_NAME); TRACE_DEFINE_ENUM(XFS_DAS_RM_SHRINK); -TRACE_DEFINE_ENUM(XFS_DAS_FOUND_LBLK); +TRACE_DEFINE_ENUM(XFS_DAS_LEAF_SET_RMT); TRACE_DEFINE_ENUM(XFS_DAS_LEAF_ALLOC_RMT); -TRACE_DEFINE_ENUM(XFS_DAS_FOUND_NBLK); -TRACE_DEFINE_ENUM(XFS_DAS_NODE_ALLOC_RMT); +TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REPLACE); TRACE_DEFINE_ENUM(XFS_DAS_FLIP_LFLAG); TRACE_DEFINE_ENUM(XFS_DAS_RM_LBLK); TRACE_DEFINE_ENUM(XFS_DAS_RD_LEAF); +TRACE_DEFINE_ENUM(XFS_DAS_NODE_SET_RMT); +TRACE_DEFINE_ENUM(XFS_DAS_NODE_ALLOC_RMT); +TRACE_DEFINE_ENUM(XFS_DAS_NODE_REPLACE); TRACE_DEFINE_ENUM(XFS_DAS_FLIP_NFLAG); TRACE_DEFINE_ENUM(XFS_DAS_RM_NBLK); TRACE_DEFINE_ENUM(XFS_DAS_CLR_FLAG); @@ -4141,6 +4143,7 @@ DEFINE_DAS_STATE_EVENT(xfs_attr_set_iter_return); DEFINE_DAS_STATE_EVENT(xfs_attr_leaf_addname_return); DEFINE_DAS_STATE_EVENT(xfs_attr_node_addname_return); DEFINE_DAS_STATE_EVENT(xfs_attr_remove_iter_return); +DEFINE_DAS_STATE_EVENT(xfs_attr_rmtval_alloc); DEFINE_DAS_STATE_EVENT(xfs_attr_rmtval_remove_return); TRACE_EVENT(xfs_force_shutdown, From patchwork Tue Apr 12 04:25:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 12810004 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 8E38CC433EF for ; Tue, 12 Apr 2022 04:25:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345790AbiDLE2J (ORCPT ); Tue, 12 Apr 2022 00:28:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51474 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232969AbiDLE2H (ORCPT ); Tue, 12 Apr 2022 00:28:07 -0400 Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 0C53132991 for ; Mon, 11 Apr 2022 21:25:49 -0700 (PDT) Received: from dread.disaster.area (pa49-186-233-190.pa.vic.optusnet.com.au [49.186.233.190]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 59C3210C7BC6; Tue, 12 Apr 2022 14:25:47 +1000 (AEST) Received: from discord.disaster.area ([192.168.253.110]) by dread.disaster.area with esmtp (Exim 4.92.3) (envelope-from ) id 1ne866-00Gh2W-36; Tue, 12 Apr 2022 14:25:46 +1000 Received: from dave by discord.disaster.area with local (Exim 4.95) (envelope-from ) id 1ne866-009Nrl-1y; Tue, 12 Apr 2022 14:25:46 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Cc: allison.henderson@oracle.com Subject: [PATCH 07/10] xfs: XFS_DAS_LEAF_REPLACE state only needed if !LARP Date: Tue, 12 Apr 2022 14:25:40 +1000 Message-Id: <20220412042543.2234866-8-david@fromorbit.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220412042543.2234866-1-david@fromorbit.com> References: <20220412042543.2234866-1-david@fromorbit.com> MIME-Version: 1.0 X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.4 cv=VuxAv86n c=1 sm=1 tr=0 ts=6254ff4b a=bHAvQTfMiaNt/bo4vVGwyA==:117 a=bHAvQTfMiaNt/bo4vVGwyA==:17 a=z0gMJWrwH1QA:10 a=20KFwNOVAAAA:8 a=OQAiHw-VPrrbQ8VQ4P4A:9 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner We can skip the REPLACE state when LARP is enabled, but that means the XFS_DAS_FLIP_LFLAG state is now poorly named - it indicates something that has been done rather than what the state is going to do. Rename it to "REMOVE_OLD" to indicate that we are now going to perform removal of the old attr. Signed-off-by: Dave Chinner --- fs/xfs/libxfs/xfs_attr.c | 49 ++++++++++++++++++++-------------------- fs/xfs/libxfs/xfs_attr.h | 44 ++++++++++++++++++------------------ fs/xfs/xfs_trace.h | 4 ++-- 3 files changed, 49 insertions(+), 48 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 4b9c107fe5de..27fa94d1c4db 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -472,37 +472,38 @@ xfs_attr_set_iter( return error; if (attr->xattri_dela_state == XFS_DAS_DONE) break; + + /* + * If we need to run a transaction to flip flags, we go to the + * next state (REPLACE) otherwise we jump straight to the + * removal state. + */ attr->xattri_dela_state++; - fallthrough; + if (xfs_has_larp(mp)) + attr->xattri_dela_state++; + goto next_state; case XFS_DAS_LEAF_REPLACE: case XFS_DAS_NODE_REPLACE: /* - * If this is an atomic rename operation, we must "flip" the - * incomplete flags on the "new" and "old" attribute/value pairs - * so that one disappears and one appears atomically. Then we - * must remove the "old" attribute/value pair. - * - * In a separate transaction, set the incomplete flag on the - * "old" attr and clear the incomplete flag on the "new" attr. + * We must "flip" the incomplete flags on the "new" and "old" + * attribute/value pairs so that one disappears and one appears + * atomically. Then we must remove the "old" attribute/value + * pair. */ - if (!xfs_has_larp(mp)) { - error = xfs_attr3_leaf_flipflags(args); - if (error) - return error; - /* - * Commit the flag value change and start the next trans - * in series at FLIP_FLAG. - */ - error = -EAGAIN; - attr->xattri_dela_state++; - break; - } - + error = xfs_attr3_leaf_flipflags(args); + if (error) + return error; + /* + * Commit the flag value change and start the next trans + * in series at REMOVE_OLD. + */ + error = -EAGAIN; attr->xattri_dela_state++; - fallthrough; - case XFS_DAS_FLIP_LFLAG: - case XFS_DAS_FLIP_NFLAG: + break; + + case XFS_DAS_LEAF_REMOVE_OLD: + case XFS_DAS_NODE_REMOVE_OLD: /* * Dismantle the "old" attribute/value pair by removing a * "remote" value (if it exists). diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h index 1de6c06b7f19..a4ff0a2305d6 100644 --- a/fs/xfs/libxfs/xfs_attr.h +++ b/fs/xfs/libxfs/xfs_attr.h @@ -455,7 +455,7 @@ enum xfs_delattr_state { XFS_DAS_LEAF_SET_RMT, /* set a remote xattr from a leaf */ XFS_DAS_LEAF_ALLOC_RMT, /* We are allocating remote blocks */ XFS_DAS_LEAF_REPLACE, /* Perform replace ops on a leaf */ - XFS_DAS_FLIP_LFLAG, /* Flipped leaf INCOMPLETE attr flag */ + XFS_DAS_LEAF_REMOVE_OLD, /* Start removing old attr from leaf */ XFS_DAS_RM_LBLK, /* A rename is removing leaf blocks */ XFS_DAS_RD_LEAF, /* Read in the new leaf */ @@ -463,7 +463,7 @@ enum xfs_delattr_state { XFS_DAS_NODE_SET_RMT, /* set a remote xattr from a node */ XFS_DAS_NODE_ALLOC_RMT, /* We are allocating remote blocks */ XFS_DAS_NODE_REPLACE, /* Perform replace ops on a node */ - XFS_DAS_FLIP_NFLAG, /* Flipped node INCOMPLETE attr flag */ + XFS_DAS_NODE_REMOVE_OLD, /* Start removing old attr from node */ XFS_DAS_RM_NBLK, /* A rename is removing node blocks */ XFS_DAS_CLR_FLAG, /* Clear incomplete flag */ @@ -471,26 +471,26 @@ enum xfs_delattr_state { }; #define XFS_DAS_STRINGS \ - { XFS_DAS_UNINIT, "XFS_DAS_UNINIT" }, \ - { XFS_DAS_SF_ADD, "XFS_DAS_SF_ADD" }, \ - { XFS_DAS_LEAF_ADD, "XFS_DAS_LEAF_ADD" }, \ - { XFS_DAS_NODE_ADD, "XFS_DAS_NODE_ADD" }, \ - { XFS_DAS_RMTBLK, "XFS_DAS_RMTBLK" }, \ - { XFS_DAS_RM_NAME, "XFS_DAS_RM_NAME" }, \ - { XFS_DAS_RM_SHRINK, "XFS_DAS_RM_SHRINK" }, \ - { XFS_DAS_LEAF_SET_RMT, "XFS_DAS_LEAF_SET_RMT" }, \ - { XFS_DAS_LEAF_ALLOC_RMT, "XFS_DAS_LEAF_ALLOC_RMT" }, \ - { XFS_DAS_LEAF_REPLACE, "XFS_DAS_LEAF_REPLACE" }, \ - { XFS_DAS_FLIP_LFLAG, "XFS_DAS_FLIP_LFLAG" }, \ - { XFS_DAS_RM_LBLK, "XFS_DAS_RM_LBLK" }, \ - { XFS_DAS_RD_LEAF, "XFS_DAS_RD_LEAF" }, \ - { XFS_DAS_NODE_SET_RMT, "XFS_DAS_NODE_SET_RMT" }, \ - { XFS_DAS_NODE_ALLOC_RMT, "XFS_DAS_NODE_ALLOC_RMT" }, \ - { XFS_DAS_NODE_REPLACE, "XFS_DAS_NODE_REPLACE" }, \ - { XFS_DAS_FLIP_NFLAG, "XFS_DAS_FLIP_NFLAG" }, \ - { XFS_DAS_RM_NBLK, "XFS_DAS_RM_NBLK" }, \ - { XFS_DAS_CLR_FLAG, "XFS_DAS_CLR_FLAG" }, \ - { XFS_DAS_DONE, "XFS_DAS_DONE" } + { XFS_DAS_UNINIT, "XFS_DAS_UNINIT" }, \ + { XFS_DAS_SF_ADD, "XFS_DAS_SF_ADD" }, \ + { XFS_DAS_LEAF_ADD, "XFS_DAS_LEAF_ADD" }, \ + { XFS_DAS_NODE_ADD, "XFS_DAS_NODE_ADD" }, \ + { XFS_DAS_RMTBLK, "XFS_DAS_RMTBLK" }, \ + { XFS_DAS_RM_NAME, "XFS_DAS_RM_NAME" }, \ + { XFS_DAS_RM_SHRINK, "XFS_DAS_RM_SHRINK" }, \ + { XFS_DAS_LEAF_SET_RMT, "XFS_DAS_LEAF_SET_RMT" }, \ + { XFS_DAS_LEAF_ALLOC_RMT, "XFS_DAS_LEAF_ALLOC_RMT" }, \ + { XFS_DAS_LEAF_REPLACE, "XFS_DAS_LEAF_REPLACE" }, \ + { XFS_DAS_LEAF_REMOVE_OLD, "XFS_DAS_LEAF_REMOVE_OLD" }, \ + { XFS_DAS_RM_LBLK, "XFS_DAS_RM_LBLK" }, \ + { XFS_DAS_RD_LEAF, "XFS_DAS_RD_LEAF" }, \ + { XFS_DAS_NODE_SET_RMT, "XFS_DAS_NODE_SET_RMT" }, \ + { XFS_DAS_NODE_ALLOC_RMT, "XFS_DAS_NODE_ALLOC_RMT" }, \ + { XFS_DAS_NODE_REPLACE, "XFS_DAS_NODE_REPLACE" }, \ + { XFS_DAS_NODE_REMOVE_OLD, "XFS_DAS_NODE_REMOVE_OLD" }, \ + { XFS_DAS_RM_NBLK, "XFS_DAS_RM_NBLK" }, \ + { XFS_DAS_CLR_FLAG, "XFS_DAS_CLR_FLAG" }, \ + { XFS_DAS_DONE, "XFS_DAS_DONE" } /* * Defines for xfs_attr_item.xattri_flags diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index cf99efc5ba5a..a4b99c7f8ef0 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -4108,13 +4108,13 @@ TRACE_DEFINE_ENUM(XFS_DAS_RM_SHRINK); TRACE_DEFINE_ENUM(XFS_DAS_LEAF_SET_RMT); TRACE_DEFINE_ENUM(XFS_DAS_LEAF_ALLOC_RMT); TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REPLACE); -TRACE_DEFINE_ENUM(XFS_DAS_FLIP_LFLAG); +TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REMOVE_OLD); TRACE_DEFINE_ENUM(XFS_DAS_RM_LBLK); TRACE_DEFINE_ENUM(XFS_DAS_RD_LEAF); TRACE_DEFINE_ENUM(XFS_DAS_NODE_SET_RMT); TRACE_DEFINE_ENUM(XFS_DAS_NODE_ALLOC_RMT); TRACE_DEFINE_ENUM(XFS_DAS_NODE_REPLACE); -TRACE_DEFINE_ENUM(XFS_DAS_FLIP_NFLAG); +TRACE_DEFINE_ENUM(XFS_DAS_NODE_REMOVE_OLD); TRACE_DEFINE_ENUM(XFS_DAS_RM_NBLK); TRACE_DEFINE_ENUM(XFS_DAS_CLR_FLAG); From patchwork Tue Apr 12 04:25:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 12810005 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 0809AC433F5 for ; Tue, 12 Apr 2022 04:25:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232969AbiDLE2L (ORCPT ); Tue, 12 Apr 2022 00:28:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51476 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243444AbiDLE2H (ORCPT ); Tue, 12 Apr 2022 00:28:07 -0400 Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 0CBAF32995 for ; Mon, 11 Apr 2022 21:25:49 -0700 (PDT) Received: from dread.disaster.area (pa49-186-233-190.pa.vic.optusnet.com.au [49.186.233.190]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 5763C10C7BB8; Tue, 12 Apr 2022 14:25:47 +1000 (AEST) Received: from discord.disaster.area ([192.168.253.110]) by dread.disaster.area with esmtp (Exim 4.92.3) (envelope-from ) id 1ne866-00Gh2g-4Z; Tue, 12 Apr 2022 14:25:46 +1000 Received: from dave by discord.disaster.area with local (Exim 4.95) (envelope-from ) id 1ne866-009Nrq-33; Tue, 12 Apr 2022 14:25:46 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Cc: allison.henderson@oracle.com Subject: [PATCH 08/10] xfs: remote xattr removal in xfs_attr_set_iter() is conditional Date: Tue, 12 Apr 2022 14:25:41 +1000 Message-Id: <20220412042543.2234866-9-david@fromorbit.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220412042543.2234866-1-david@fromorbit.com> References: <20220412042543.2234866-1-david@fromorbit.com> MIME-Version: 1.0 X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.4 cv=deDjYVbe c=1 sm=1 tr=0 ts=6254ff4b a=bHAvQTfMiaNt/bo4vVGwyA==:117 a=bHAvQTfMiaNt/bo4vVGwyA==:17 a=z0gMJWrwH1QA:10 a=20KFwNOVAAAA:8 a=0S6kOs-DtnbvdYliHJcA:9 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner We may not have a remote value for the old xattr we have to remove, so skip over the remote value removal states and go straight to the xattr name removal in the leaf/node block. Signed-off-by: Dave Chinner --- fs/xfs/libxfs/xfs_attr.c | 59 ++++++++++++++++++++-------------------- fs/xfs/libxfs/xfs_attr.h | 8 +++--- fs/xfs/xfs_trace.h | 4 +-- 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 27fa94d1c4db..09490fcd1e28 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -488,15 +488,14 @@ xfs_attr_set_iter( /* * We must "flip" the incomplete flags on the "new" and "old" * attribute/value pairs so that one disappears and one appears - * atomically. Then we must remove the "old" attribute/value - * pair. + * atomically. */ error = xfs_attr3_leaf_flipflags(args); if (error) return error; /* - * Commit the flag value change and start the next trans - * in series at REMOVE_OLD. + * We must commit the flag value change now to make it atomic + * and then we can start the next trans in series at REMOVE_OLD. */ error = -EAGAIN; attr->xattri_dela_state++; @@ -505,41 +504,43 @@ xfs_attr_set_iter( case XFS_DAS_LEAF_REMOVE_OLD: case XFS_DAS_NODE_REMOVE_OLD: /* - * Dismantle the "old" attribute/value pair by removing a - * "remote" value (if it exists). + * If we have a remote attr, start the process of removing it + * by invalidating any cached buffers. + * + * If we don't have a remote attr, we skip the remote block + * removal state altogether with a second state increment. */ xfs_attr_restore_rmt_blk(args); - error = xfs_attr_rmtval_invalidate(args); - if (error) - return error; - - attr->xattri_dela_state++; - fallthrough; - case XFS_DAS_RM_LBLK: - case XFS_DAS_RM_NBLK: if (args->rmtblkno) { - error = xfs_attr_rmtval_remove(attr); - if (error == -EAGAIN) - trace_xfs_attr_set_iter_return( - attr->xattri_dela_state, args->dp); + error = xfs_attr_rmtval_invalidate(args); if (error) return error; - - attr->xattri_dela_state = XFS_DAS_RD_LEAF; - trace_xfs_attr_set_iter_return(attr->xattri_dela_state, - args->dp); - return -EAGAIN; + } else { + attr->xattri_dela_state++; } + attr->xattri_dela_state++; + goto next_state; + + case XFS_DAS_LEAF_REMOVE_RMT: + case XFS_DAS_NODE_REMOVE_RMT: + error = xfs_attr_rmtval_remove(attr); + if (error == -EAGAIN) + break; + if (error) + return error; + /* - * This is the end of the shared leaf/node sequence. We need - * to continue at the next state in the sequence, but we can't - * easily just fall through. So we increment to the next state - * and then jump back to switch statement to evaluate the next - * state correctly. + * We've finished removing the remote attr blocks, so commit the + * transaction and move on to removing the attr name from the + * leaf/node block. Removing the attr might require a full + * transaction reservation for btree block freeing, so we + * can't do that in the same transaction where we removed the + * remote attr blocks. */ + error = -EAGAIN; attr->xattri_dela_state++; - goto next_state; + break; case XFS_DAS_RD_LEAF: /* diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h index a4ff0a2305d6..18e157bf19cb 100644 --- a/fs/xfs/libxfs/xfs_attr.h +++ b/fs/xfs/libxfs/xfs_attr.h @@ -456,7 +456,7 @@ enum xfs_delattr_state { XFS_DAS_LEAF_ALLOC_RMT, /* We are allocating remote blocks */ XFS_DAS_LEAF_REPLACE, /* Perform replace ops on a leaf */ XFS_DAS_LEAF_REMOVE_OLD, /* Start removing old attr from leaf */ - XFS_DAS_RM_LBLK, /* A rename is removing leaf blocks */ + XFS_DAS_LEAF_REMOVE_RMT, /* A rename is removing remote blocks */ XFS_DAS_RD_LEAF, /* Read in the new leaf */ /* Node state set sequence, must match leaf state above */ @@ -464,7 +464,7 @@ enum xfs_delattr_state { XFS_DAS_NODE_ALLOC_RMT, /* We are allocating remote blocks */ XFS_DAS_NODE_REPLACE, /* Perform replace ops on a node */ XFS_DAS_NODE_REMOVE_OLD, /* Start removing old attr from node */ - XFS_DAS_RM_NBLK, /* A rename is removing node blocks */ + XFS_DAS_NODE_REMOVE_RMT, /* A rename is removing remote blocks */ XFS_DAS_CLR_FLAG, /* Clear incomplete flag */ XFS_DAS_DONE, /* finished operation */ @@ -482,13 +482,13 @@ enum xfs_delattr_state { { XFS_DAS_LEAF_ALLOC_RMT, "XFS_DAS_LEAF_ALLOC_RMT" }, \ { XFS_DAS_LEAF_REPLACE, "XFS_DAS_LEAF_REPLACE" }, \ { XFS_DAS_LEAF_REMOVE_OLD, "XFS_DAS_LEAF_REMOVE_OLD" }, \ - { XFS_DAS_RM_LBLK, "XFS_DAS_RM_LBLK" }, \ + { XFS_DAS_LEAF_REMOVE_RMT, "XFS_DAS_LEAF_REMOVE_RMT" }, \ { XFS_DAS_RD_LEAF, "XFS_DAS_RD_LEAF" }, \ { XFS_DAS_NODE_SET_RMT, "XFS_DAS_NODE_SET_RMT" }, \ { XFS_DAS_NODE_ALLOC_RMT, "XFS_DAS_NODE_ALLOC_RMT" }, \ { XFS_DAS_NODE_REPLACE, "XFS_DAS_NODE_REPLACE" }, \ { XFS_DAS_NODE_REMOVE_OLD, "XFS_DAS_NODE_REMOVE_OLD" }, \ - { XFS_DAS_RM_NBLK, "XFS_DAS_RM_NBLK" }, \ + { XFS_DAS_NODE_REMOVE_RMT, "XFS_DAS_NODE_REMOVE_RMT" }, \ { XFS_DAS_CLR_FLAG, "XFS_DAS_CLR_FLAG" }, \ { XFS_DAS_DONE, "XFS_DAS_DONE" } diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index a4b99c7f8ef0..91852b9721e4 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -4109,13 +4109,13 @@ TRACE_DEFINE_ENUM(XFS_DAS_LEAF_SET_RMT); TRACE_DEFINE_ENUM(XFS_DAS_LEAF_ALLOC_RMT); TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REPLACE); TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REMOVE_OLD); -TRACE_DEFINE_ENUM(XFS_DAS_RM_LBLK); +TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REMOVE_RMT); TRACE_DEFINE_ENUM(XFS_DAS_RD_LEAF); TRACE_DEFINE_ENUM(XFS_DAS_NODE_SET_RMT); TRACE_DEFINE_ENUM(XFS_DAS_NODE_ALLOC_RMT); TRACE_DEFINE_ENUM(XFS_DAS_NODE_REPLACE); TRACE_DEFINE_ENUM(XFS_DAS_NODE_REMOVE_OLD); -TRACE_DEFINE_ENUM(XFS_DAS_RM_NBLK); +TRACE_DEFINE_ENUM(XFS_DAS_NODE_REMOVE_RMT); TRACE_DEFINE_ENUM(XFS_DAS_CLR_FLAG); DECLARE_EVENT_CLASS(xfs_das_state_class, From patchwork Tue Apr 12 04:25:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 12810009 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 1C2B4C43219 for ; Tue, 12 Apr 2022 04:26:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345816AbiDLE2P (ORCPT ); Tue, 12 Apr 2022 00:28:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51512 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345795AbiDLE2K (ORCPT ); Tue, 12 Apr 2022 00:28:10 -0400 Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 2C9C2329A1 for ; Mon, 11 Apr 2022 21:25:53 -0700 (PDT) Received: from dread.disaster.area (pa49-186-233-190.pa.vic.optusnet.com.au [49.186.233.190]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 695E210C7BDD; Tue, 12 Apr 2022 14:25:47 +1000 (AEST) Received: from discord.disaster.area ([192.168.253.110]) by dread.disaster.area with esmtp (Exim 4.92.3) (envelope-from ) id 1ne866-00Gh2j-5h; Tue, 12 Apr 2022 14:25:46 +1000 Received: from dave by discord.disaster.area with local (Exim 4.95) (envelope-from ) id 1ne866-009Nrv-4Z; Tue, 12 Apr 2022 14:25:46 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Cc: allison.henderson@oracle.com Subject: [PATCH 09/10] xfs: clean up final attr removal in xfs_attr_set_iter Date: Tue, 12 Apr 2022 14:25:42 +1000 Message-Id: <20220412042543.2234866-10-david@fromorbit.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220412042543.2234866-1-david@fromorbit.com> References: <20220412042543.2234866-1-david@fromorbit.com> MIME-Version: 1.0 X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.4 cv=deDjYVbe c=1 sm=1 tr=0 ts=6254ff4b a=bHAvQTfMiaNt/bo4vVGwyA==:117 a=bHAvQTfMiaNt/bo4vVGwyA==:17 a=z0gMJWrwH1QA:10 a=20KFwNOVAAAA:8 a=G_-VHr62sIVnPuVDeK8A:9 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner Clean up the final leaf/node states in xfs_attr_set_iter() to further simplify the highe level state machine and to set the completion state correctly. Signed-off-by: Dave Chinner --- fs/xfs/libxfs/xfs_attr.c | 67 +++++++++++++++++++++------------------- fs/xfs/libxfs/xfs_attr.h | 12 +++---- fs/xfs/xfs_trace.h | 4 +-- 3 files changed, 44 insertions(+), 39 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 09490fcd1e28..85fd9804f290 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -61,7 +61,7 @@ STATIC int xfs_attr_node_get(xfs_da_args_t *args); STATIC void xfs_attr_restore_rmt_blk(struct xfs_da_args *args); static int xfs_attr_node_try_addname(struct xfs_attr_item *attr); STATIC int xfs_attr_node_addname_find_attr(struct xfs_attr_item *attr); -STATIC int xfs_attr_node_addname_clear_incomplete(struct xfs_attr_item *attr); +STATIC int xfs_attr_node_remove_attr(struct xfs_attr_item *attr); STATIC int xfs_attr_node_hasname(xfs_da_args_t *args, struct xfs_da_state **state); STATIC int xfs_attr_fillstate(xfs_da_state_t *state); @@ -427,6 +427,31 @@ xfs_attr_rmtval_alloc( return error; } +static int +xfs_attr_leaf_remove_attr( + struct xfs_attr_item *attr) +{ + struct xfs_da_args *args = attr->xattri_da_args; + struct xfs_inode *dp = args->dp; + struct xfs_buf *bp = NULL; + int forkoff; + int error; + + error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, + &bp); + if (error) + return error; + + xfs_attr3_leaf_remove(bp, args); + + forkoff = xfs_attr_shortform_allfit(bp, dp); + if (forkoff) + error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); + /* bp is gone due to xfs_da_shrink_inode */ + + return error; +} + /* * Set the attribute specified in @args. * This routine is meant to function as a delayed operation, and may return @@ -439,10 +464,8 @@ xfs_attr_set_iter( struct xfs_attr_item *attr) { struct xfs_da_args *args = attr->xattri_da_args; - struct xfs_inode *dp = args->dp; - struct xfs_buf *bp = NULL; - int forkoff, error = 0; struct xfs_mount *mp = args->dp->i_mount; + int error = 0; /* State machine switch */ next_state: @@ -542,32 +565,14 @@ xfs_attr_set_iter( attr->xattri_dela_state++; break; - case XFS_DAS_RD_LEAF: - /* - * This is the last step for leaf format. Read the block with - * the old attr, remove the old attr, check for shortform - * conversion and return. - */ - error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, - &bp); - if (error) - return error; - - xfs_attr3_leaf_remove(bp, args); - - forkoff = xfs_attr_shortform_allfit(bp, dp); - if (forkoff) - error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); - /* bp is gone due to xfs_da_shrink_inode */ - - return error; + case XFS_DAS_LEAF_REMOVE_ATTR: + error = xfs_attr_leaf_remove_attr(attr); + attr->xattri_dela_state = XFS_DAS_DONE; + break; - case XFS_DAS_CLR_FLAG: - /* - * The last state for node format. Look up the old attr and - * remove it. - */ - error = xfs_attr_node_addname_clear_incomplete(attr); + case XFS_DAS_NODE_REMOVE_ATTR: + error = xfs_attr_node_remove_attr(attr); + attr->xattri_dela_state = XFS_DAS_DONE; break; default: ASSERT(0); @@ -1240,8 +1245,8 @@ xfs_attr_node_try_addname( } -STATIC int -xfs_attr_node_addname_clear_incomplete( +static int +xfs_attr_node_remove_attr( struct xfs_attr_item *attr) { struct xfs_da_args *args = attr->xattri_da_args; diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h index 18e157bf19cb..f4f78d841857 100644 --- a/fs/xfs/libxfs/xfs_attr.h +++ b/fs/xfs/libxfs/xfs_attr.h @@ -451,21 +451,21 @@ enum xfs_delattr_state { XFS_DAS_RM_NAME, /* Remove attr name */ XFS_DAS_RM_SHRINK, /* We are shrinking the tree */ - /* Leaf state set sequence */ + /* Leaf state set/replace sequence */ XFS_DAS_LEAF_SET_RMT, /* set a remote xattr from a leaf */ XFS_DAS_LEAF_ALLOC_RMT, /* We are allocating remote blocks */ XFS_DAS_LEAF_REPLACE, /* Perform replace ops on a leaf */ XFS_DAS_LEAF_REMOVE_OLD, /* Start removing old attr from leaf */ XFS_DAS_LEAF_REMOVE_RMT, /* A rename is removing remote blocks */ - XFS_DAS_RD_LEAF, /* Read in the new leaf */ + XFS_DAS_LEAF_REMOVE_ATTR, /* Remove the old attr from a leaf */ - /* Node state set sequence, must match leaf state above */ + /* Node state set/replace sequence, must match leaf state above */ XFS_DAS_NODE_SET_RMT, /* set a remote xattr from a node */ XFS_DAS_NODE_ALLOC_RMT, /* We are allocating remote blocks */ XFS_DAS_NODE_REPLACE, /* Perform replace ops on a node */ XFS_DAS_NODE_REMOVE_OLD, /* Start removing old attr from node */ XFS_DAS_NODE_REMOVE_RMT, /* A rename is removing remote blocks */ - XFS_DAS_CLR_FLAG, /* Clear incomplete flag */ + XFS_DAS_NODE_REMOVE_ATTR, /* Remove the old attr from a node */ XFS_DAS_DONE, /* finished operation */ }; @@ -483,13 +483,13 @@ enum xfs_delattr_state { { XFS_DAS_LEAF_REPLACE, "XFS_DAS_LEAF_REPLACE" }, \ { XFS_DAS_LEAF_REMOVE_OLD, "XFS_DAS_LEAF_REMOVE_OLD" }, \ { XFS_DAS_LEAF_REMOVE_RMT, "XFS_DAS_LEAF_REMOVE_RMT" }, \ - { XFS_DAS_RD_LEAF, "XFS_DAS_RD_LEAF" }, \ + { XFS_DAS_LEAF_REMOVE_ATTR, "XFS_DAS_LEAF_REMOVE_ATTR" }, \ { XFS_DAS_NODE_SET_RMT, "XFS_DAS_NODE_SET_RMT" }, \ { XFS_DAS_NODE_ALLOC_RMT, "XFS_DAS_NODE_ALLOC_RMT" }, \ { XFS_DAS_NODE_REPLACE, "XFS_DAS_NODE_REPLACE" }, \ { XFS_DAS_NODE_REMOVE_OLD, "XFS_DAS_NODE_REMOVE_OLD" }, \ { XFS_DAS_NODE_REMOVE_RMT, "XFS_DAS_NODE_REMOVE_RMT" }, \ - { XFS_DAS_CLR_FLAG, "XFS_DAS_CLR_FLAG" }, \ + { XFS_DAS_NODE_REMOVE_ATTR, "XFS_DAS_NODE_REMOVE_ATTR" }, \ { XFS_DAS_DONE, "XFS_DAS_DONE" } /* diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index 91852b9721e4..b72dd79355e4 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -4110,13 +4110,13 @@ TRACE_DEFINE_ENUM(XFS_DAS_LEAF_ALLOC_RMT); TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REPLACE); TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REMOVE_OLD); TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REMOVE_RMT); -TRACE_DEFINE_ENUM(XFS_DAS_RD_LEAF); +TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REMOVE_ATTR); TRACE_DEFINE_ENUM(XFS_DAS_NODE_SET_RMT); TRACE_DEFINE_ENUM(XFS_DAS_NODE_ALLOC_RMT); TRACE_DEFINE_ENUM(XFS_DAS_NODE_REPLACE); TRACE_DEFINE_ENUM(XFS_DAS_NODE_REMOVE_OLD); TRACE_DEFINE_ENUM(XFS_DAS_NODE_REMOVE_RMT); -TRACE_DEFINE_ENUM(XFS_DAS_CLR_FLAG); +TRACE_DEFINE_ENUM(XFS_DAS_NODE_REMOVE_ATTR); DECLARE_EVENT_CLASS(xfs_das_state_class, TP_PROTO(int das, struct xfs_inode *ip), From patchwork Tue Apr 12 04:25:43 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 12810008 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 38D25C4332F for ; Tue, 12 Apr 2022 04:26:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344502AbiDLE2O (ORCPT ); Tue, 12 Apr 2022 00:28:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51510 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345794AbiDLE2K (ORCPT ); Tue, 12 Apr 2022 00:28:10 -0400 Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 2A77F32997 for ; Mon, 11 Apr 2022 21:25:53 -0700 (PDT) Received: from dread.disaster.area (pa49-186-233-190.pa.vic.optusnet.com.au [49.186.233.190]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 6B9A010C7C01; Tue, 12 Apr 2022 14:25:47 +1000 (AEST) Received: from discord.disaster.area ([192.168.253.110]) by dread.disaster.area with esmtp (Exim 4.92.3) (envelope-from ) id 1ne866-00Gh2m-6y; Tue, 12 Apr 2022 14:25:46 +1000 Received: from dave by discord.disaster.area with local (Exim 4.95) (envelope-from ) id 1ne866-009Ns0-5g; Tue, 12 Apr 2022 14:25:46 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Cc: allison.henderson@oracle.com Subject: [PATCH 10/10] xfs: xfs_attr_set_iter() does not need to return EAGAIN Date: Tue, 12 Apr 2022 14:25:43 +1000 Message-Id: <20220412042543.2234866-11-david@fromorbit.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220412042543.2234866-1-david@fromorbit.com> References: <20220412042543.2234866-1-david@fromorbit.com> MIME-Version: 1.0 X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.4 cv=deDjYVbe c=1 sm=1 tr=0 ts=6254ff4b a=bHAvQTfMiaNt/bo4vVGwyA==:117 a=bHAvQTfMiaNt/bo4vVGwyA==:17 a=z0gMJWrwH1QA:10 a=20KFwNOVAAAA:8 a=58BS8la1inLbC0avub8A:9 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner Now that the full xfs_attr_set_iter() state machine always terminates with either the state being XFS_DAS_DONE on success or an error on failure, we can get rid of the need for it to return -EAGAIN whenever it needs to roll the transaction before running the next state. That is, we don't need to spray -EAGAIN return states everywhere, the caller just check the state machine state for completion to determine what action should be taken next. This greatly simplifies the code within the state machine implementation as it now only has to handle 0 for success or -errno for error and it doesn't need to tell the caller to retry. Signed-off-by: Dave Chinner --- fs/xfs/libxfs/xfs_attr.c | 108 +++++++++++++++++++-------------------- fs/xfs/xfs_attr_item.c | 2 + 2 files changed, 54 insertions(+), 56 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 85fd9804f290..903039408a25 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -303,7 +303,6 @@ xfs_attr_sf_addname( */ xfs_trans_bhold(args->trans, attr->xattri_leaf_bp); attr->xattri_dela_state = XFS_DAS_LEAF_ADD; - error = -EAGAIN; out: trace_xfs_attr_sf_addname_return(attr->xattri_dela_state, args->dp); return error; @@ -336,7 +335,6 @@ xfs_attr_leaf_addname( * retry the add to the newly allocated node block. */ attr->xattri_dela_state = XFS_DAS_NODE_ADD; - error = -EAGAIN; goto out; } if (error) @@ -347,20 +345,24 @@ xfs_attr_leaf_addname( * or perform more xattr manipulations. Otherwise there is nothing more * to do and we can return success. */ - if (args->rmtblkno) { + if (args->rmtblkno) attr->xattri_dela_state = XFS_DAS_LEAF_SET_RMT; - error = -EAGAIN; - } else if (args->op_flags & XFS_DA_OP_RENAME) { + else if (args->op_flags & XFS_DA_OP_RENAME) attr->xattri_dela_state = XFS_DAS_LEAF_REPLACE; - error = -EAGAIN; - } else { + else attr->xattri_dela_state = XFS_DAS_DONE; - } out: trace_xfs_attr_leaf_addname_return(attr->xattri_dela_state, args->dp); return error; } +/* + * Add an entry to a node format attr tree. + * + * Note that we might still have a leaf here - xfs_attr_is_leaf() cannot tell + * the difference between leaf + remote attr blocks and a node format tree, + * so we may still end up having to convert from leaf to node format here. + */ static int xfs_attr_node_addname( struct xfs_attr_item *attr) @@ -375,19 +377,27 @@ xfs_attr_node_addname( return error; error = xfs_attr_node_try_addname(attr); + if (error == -ENOSPC) { + error = xfs_attr3_leaf_to_node(args); + if (error) + return error; + /* + * No state change, we really are in node form now + * but we need the transaction rolled to continue. + */ + goto out; + } if (error) return error; - if (args->rmtblkno) { + if (args->rmtblkno) attr->xattri_dela_state = XFS_DAS_NODE_SET_RMT; - error = -EAGAIN; - } else if (args->op_flags & XFS_DA_OP_RENAME) { + else if (args->op_flags & XFS_DA_OP_RENAME) attr->xattri_dela_state = XFS_DAS_NODE_REPLACE; - error = -EAGAIN; - } else { + else attr->xattri_dela_state = XFS_DAS_DONE; - } +out: trace_xfs_attr_node_addname_return(attr->xattri_dela_state, args->dp); return error; } @@ -409,7 +419,6 @@ xfs_attr_rmtval_alloc( error = xfs_attr_rmtval_set_blk(attr); if (error) return error; - error = -EAGAIN; goto out; } @@ -421,6 +430,15 @@ xfs_attr_rmtval_alloc( if (!(args->op_flags & XFS_DA_OP_RENAME)) { error = xfs_attr3_leaf_clearflag(args); attr->xattri_dela_state = XFS_DAS_DONE; + } else { + /* + * If we need to run a transaction to flip flags, we go to the + * next state (REPLACE) otherwise we jump straight to the + * REMOVE_OLD state. + */ + attr->xattri_dela_state++; + if (xfs_has_larp(args->dp->i_mount)) + attr->xattri_dela_state++; } out: trace_xfs_attr_rmtval_alloc(attr->xattri_dela_state, args->dp); @@ -453,18 +471,18 @@ xfs_attr_leaf_remove_attr( } /* - * Set the attribute specified in @args. - * This routine is meant to function as a delayed operation, and may return - * -EAGAIN when the transaction needs to be rolled. Calling functions will need - * to handle this, and recall the function until a successful error code is - * returned. + * Run the attribute operation specified in @attr. + * + * This routine is meant to function as a delayed operation and will set the + * state to XFS_DAS_DONE when the operation is complete. Calling functions will + * need to handle this, and recall the function until either an error or + * XFS_DAS_DONE is detected. */ int xfs_attr_set_iter( struct xfs_attr_item *attr) { struct xfs_da_args *args = attr->xattri_da_args; - struct xfs_mount *mp = args->dp->i_mount; int error = 0; /* State machine switch */ @@ -493,17 +511,17 @@ xfs_attr_set_iter( error = xfs_attr_rmtval_alloc(attr); if (error) return error; - if (attr->xattri_dela_state == XFS_DAS_DONE) - break; /* - * If we need to run a transaction to flip flags, we go to the - * next state (REPLACE) otherwise we jump straight to the - * removal state. + * If there is still more to allocate we need to roll the + * transaction so we have a full transaction reservation for + * the next allocation. */ - attr->xattri_dela_state++; - if (xfs_has_larp(mp)) - attr->xattri_dela_state++; + if (attr->xattri_blkcnt > 0) + break; + if (attr->xattri_dela_state == XFS_DAS_DONE) + break; + goto next_state; case XFS_DAS_LEAF_REPLACE: @@ -520,7 +538,6 @@ xfs_attr_set_iter( * We must commit the flag value change now to make it atomic * and then we can start the next trans in series at REMOVE_OLD. */ - error = -EAGAIN; attr->xattri_dela_state++; break; @@ -548,8 +565,10 @@ xfs_attr_set_iter( case XFS_DAS_LEAF_REMOVE_RMT: case XFS_DAS_NODE_REMOVE_RMT: error = xfs_attr_rmtval_remove(attr); - if (error == -EAGAIN) + if (error == -EAGAIN) { + error = 0; break; + } if (error) return error; @@ -561,7 +580,6 @@ xfs_attr_set_iter( * can't do that in the same transaction where we removed the * remote attr blocks. */ - error = -EAGAIN; attr->xattri_dela_state++; break; @@ -1173,14 +1191,6 @@ xfs_attr_node_addname_find_attr( * This will involve walking down the Btree, and may involve splitting * leaf nodes and even splitting intermediate nodes up to and including * the root node (a special case of an intermediate node). - * - * "Remote" attribute values confuse the issue and atomic rename operations - * add a whole extra layer of confusion on top of that. - * - * This routine is meant to function as a delayed operation, and may return - * -EAGAIN when the transaction needs to be rolled. Calling functions will need - * to handle this, and recall the function until a successful error code is - *returned. */ static int xfs_attr_node_try_addname( @@ -1202,24 +1212,10 @@ xfs_attr_node_try_addname( /* * Its really a single leaf node, but it had * out-of-line values so it looked like it *might* - * have been a b-tree. + * have been a b-tree. Let the caller deal with this. */ xfs_da_state_free(state); - state = NULL; - error = xfs_attr3_leaf_to_node(args); - if (error) - goto out; - - /* - * Now that we have converted the leaf to a node, we can - * roll the transaction, and try xfs_attr3_leaf_add - * again on re-entry. No need to set dela_state to do - * this. dela_state is still unset by this function at - * this point. - */ - trace_xfs_attr_node_addname_return( - attr->xattri_dela_state, args->dp); - return -EAGAIN; + return -ENOSPC; } /* diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c index 0e2ef0dedb28..85d09f1035c9 100644 --- a/fs/xfs/xfs_attr_item.c +++ b/fs/xfs/xfs_attr_item.c @@ -314,6 +314,8 @@ xfs_xattri_finish_update( switch (op) { case XFS_ATTR_OP_FLAGS_SET: error = xfs_attr_set_iter(attr); + if (!error && attr->xattri_dela_state != XFS_DAS_DONE) + error = -EAGAIN; break; case XFS_ATTR_OP_FLAGS_REMOVE: ASSERT(XFS_IFORK_Q(args->dp)); From patchwork Tue Apr 12 09:09:16 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 12810346 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 63738C433F5 for ; Tue, 12 Apr 2022 10:14:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241168AbiDLKQz (ORCPT ); Tue, 12 Apr 2022 06:16:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35912 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1353861AbiDLKC5 (ORCPT ); Tue, 12 Apr 2022 06:02:57 -0400 Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 4DBBC340C1 for ; Tue, 12 Apr 2022 02:09:19 -0700 (PDT) Received: from dread.disaster.area (pa49-186-233-190.pa.vic.optusnet.com.au [49.186.233.190]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 05203534515; Tue, 12 Apr 2022 19:09:17 +1000 (AEST) Received: from dave by dread.disaster.area with local (Exim 4.92.3) (envelope-from ) id 1neCWS-00GlqL-JY; Tue, 12 Apr 2022 19:09:16 +1000 Date: Tue, 12 Apr 2022 19:09:16 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Cc: allison.henderson@oracle.com Subject: [PATCH 11/10] xfs: initialise attrd item to zero Message-ID: <20220412090916.GF1544202@dread.disaster.area> References: <20220412042543.2234866-1-david@fromorbit.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220412042543.2234866-1-david@fromorbit.com> X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.4 cv=deDjYVbe c=1 sm=1 tr=0 ts=625541be a=bHAvQTfMiaNt/bo4vVGwyA==:117 a=bHAvQTfMiaNt/bo4vVGwyA==:17 a=kj9zAlcOel0A:10 a=z0gMJWrwH1QA:10 a=20KFwNOVAAAA:8 a=VdGwt750K41ze9-0lWsA:9 a=CjuIK1q_8ugA:10 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner On the first allocation of a attrd item, xfs_trans_add_item() fires an assert like so: XFS (pmem0): EXPERIMENTAL logged extended attributes feature added. Use at your own risk! XFS: Assertion failed: !test_bit(XFS_LI_DIRTY, &lip->li_flags), file: fs/xfs/xfs_trans.c, line: 683 ------------[ cut here ]------------ kernel BUG at fs/xfs/xfs_message.c:102! Call Trace: xfs_trans_add_item+0x17e/0x190 xfs_trans_get_attrd+0x67/0x90 xfs_attr_create_done+0x13/0x20 xfs_defer_finish_noroll+0x100/0x690 __xfs_trans_commit+0x144/0x330 xfs_trans_commit+0x10/0x20 xfs_attr_set+0x3e2/0x4c0 xfs_initxattrs+0xaa/0xe0 security_inode_init_security+0xb0/0x130 xfs_init_security+0x18/0x20 xfs_generic_create+0x13a/0x340 xfs_vn_create+0x17/0x20 path_openat+0xff3/0x12f0 do_filp_open+0xb2/0x150 The attrd log item is allocated via kmem_cache_alloc, and xfs_log_item_init() does not zero the entire log item structure - it assumes that the structure is already all zeros as it only initialises non-zero fields. Fix the attr items to be allocated via the *zalloc methods. Signed-off-by: Dave Chinner --- With this patch, the series also passes the attr group fstests with larp enabled. This probably should be folded back into the original patchset where this allocation is added. fs/xfs/xfs_attr_item.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c index 0e2ef0dedb28..b6561861ef01 100644 --- a/fs/xfs/xfs_attr_item.c +++ b/fs/xfs/xfs_attr_item.c @@ -725,7 +725,7 @@ xfs_trans_get_attrd(struct xfs_trans *tp, ASSERT(tp != NULL); - attrdp = kmem_cache_alloc(xfs_attrd_cache, GFP_NOFS | __GFP_NOFAIL); + attrdp = kmem_cache_zalloc(xfs_attrd_cache, GFP_NOFS | __GFP_NOFAIL); xfs_log_item_init(tp->t_mountp, &attrdp->attrd_item, XFS_LI_ATTRD, &xfs_attrd_item_ops);