From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 81ADAC433EF for ; Tue, 10 May 2022 23:26:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237439AbiEJX0x (ORCPT ); Tue, 10 May 2022 19:26:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54798 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238130AbiEJX0r (ORCPT ); Tue, 10 May 2022 19:26:47 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7ACD25A0A5 for ; Tue, 10 May 2022 16:26:44 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 06E0261838 for ; Tue, 10 May 2022 23:26:44 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 54D69C385CE; Tue, 10 May 2022 23:26:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1652225203; bh=z0XAL+biJFIWwMu7G0XvsXxAS5SOOkaQGbrmEgCvLRc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=OzPT4IB1KOf2WnlXwH6JJtTEjnKo0HcwpbrHerT/bJQiyS5okF/2cdClkgR5bPAKF 5CfmV2BY7tjI7mv4eIdHpSKqNly9aE8SfMBgekubX8CPjiE/GDkrpPjaFwrmFTmZGw tM4Dzh877jt79qTLbQ0bqN3lelSWE3SOuJs+mNgmIHhWyYk2uS0V21lJJrQzjgmvn0 DoTJgzOtl9qlTN1SF28qOl+oILu7AKbG0GS5SDP63CeWj3wOuz5uqk+hBaXmc3OUS1 3etKKktZUCRkwnZjhyDaDAJ0WY85hYRHOTavk6KI2VZN7FpDkTyoErggzOy2Hup1zV kN7FE402RwtHQ== Date: Tue, 10 May 2022 16:26:42 -0700 From: "Darrick J. Wong" To: Dave Chinner Cc: linux-xfs@vger.kernel.org Subject: Re: [PATCH 10/18] xfs: remote xattr removal in xfs_attr_set_iter() is conditional Message-ID: <20220510232642.GN27195@magnolia> References: <20220509004138.762556-1-david@fromorbit.com> <20220509004138.762556-11-david@fromorbit.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220509004138.762556-11-david@fromorbit.com> Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org On Mon, May 09, 2022 at 10:41:30AM +1000, Dave Chinner wrote: > 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 > Reviewed-by: Allison Henderson Makes sense, Reviewed-by: Darrick J. Wong --D > --- > 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 0f4636e2e246..d65abaead9a1 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -495,15 +495,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++; > @@ -512,41 +511,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 3f1234272f3a..d67535e4ce5a 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 b528c0f375c2..793d2a86ab2c 100644 > --- a/fs/xfs/xfs_trace.h > +++ b/fs/xfs/xfs_trace.h > @@ -4140,13 +4140,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, > -- > 2.35.1 >