Re: [PATCH v15 17/22] xfs: Skip flip flags for delayed attrs
From: "Darrick J. Wong" <djwong@kernel.org>
Date: 2021-02-26 05:03:30
On Thu, Feb 18, 2021 at 09:53:43AM -0700, Allison Henderson wrote:
This is a clean up patch that skips the flip flag logic for delayed attr renames. Since the log replay keeps the inode locked, we do not need to worry about race windows with attr lookups. So we can skip over flipping the flag and the extra transaction roll for it Signed-off-by: Allison Henderson <redacted>
I wonder, have you done much performance analysis of the old vs. new xattr code paths? Does skipping the extra step + roll make attr operations faster? This looks pretty straightforward though: Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D
quoted hunk ↗ jump to hunk
--- fs/xfs/libxfs/xfs_attr.c | 51 +++++++++++++++++++++++++------------------ fs/xfs/libxfs/xfs_attr_leaf.c | 3 ++- 2 files changed, 32 insertions(+), 22 deletions(-)diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index e4c1b4b..666cc69 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c@@ -337,6 +337,7 @@ xfs_attr_set_iter( struct xfs_da_state *state = NULL; int forkoff, error = 0; int retval = 0; + struct xfs_mount *mp = args->dp->i_mount; /* State machine switch */ switch (dac->dela_state) {@@ -470,16 +471,21 @@ xfs_attr_set_iter( * "old" attr and clear the incomplete flag on the "new" attr. */ - error = xfs_attr3_leaf_flipflags(args); - if (error) - return error; - /* - * Commit the flag value change and start the next trans in - * series. - */ - dac->dela_state = XFS_DAS_FLIP_LFLAG; - trace_xfs_attr_set_iter_return(dac->dela_state, args->dp); - return -EAGAIN; + if (!xfs_hasdelattr(mp)) { + error = xfs_attr3_leaf_flipflags(args); + if (error) + return error; + /* + * Commit the flag value change and start the next trans + * in series. + */ + dac->dela_state = XFS_DAS_FLIP_LFLAG; + trace_xfs_attr_set_iter_return(dac->dela_state, + args->dp); + return -EAGAIN; + } + + /* fallthrough */ case XFS_DAS_FLIP_LFLAG: /* * Dismantle the "old" attribute/value pair by removing a@@ -588,17 +594,21 @@ xfs_attr_set_iter( * In a separate transaction, set the incomplete flag on the * "old" attr and clear the incomplete flag on the "new" attr. */ - error = xfs_attr3_leaf_flipflags(args); - if (error) - goto out; - /* - * Commit the flag value change and start the next trans in - * series - */ - dac->dela_state = XFS_DAS_FLIP_NFLAG; - trace_xfs_attr_set_iter_return(dac->dela_state, args->dp); - return -EAGAIN; + if (!xfs_hasdelattr(mp)) { + error = xfs_attr3_leaf_flipflags(args); + if (error) + goto out; + /* + * Commit the flag value change and start the next trans + * in series + */ + dac->dela_state = XFS_DAS_FLIP_NFLAG; + trace_xfs_attr_set_iter_return(dac->dela_state, + args->dp); + return -EAGAIN; + } + /* fallthrough */ case XFS_DAS_FLIP_NFLAG: /* * Dismantle the "old" attribute/value pair by removing a@@ -1277,7 +1287,6 @@ int xfs_attr_node_addname_work( * Re-find the "old" attribute entry after any split ops. The INCOMPLETE * flag means that we will find the "old" attr, not the "new" one. */ - args->attr_filter |= XFS_ATTR_INCOMPLETE; state = xfs_da_state_alloc(args); state->inleaf = 0; error = xfs_da3_node_lookup_int(state, &retval);diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c index 3780141..ec707bd 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.c +++ b/fs/xfs/libxfs/xfs_attr_leaf.c@@ -1486,7 +1486,8 @@ xfs_attr3_leaf_add_work( if (tmp) entry->flags |= XFS_ATTR_LOCAL; if (args->op_flags & XFS_DA_OP_RENAME) { - entry->flags |= XFS_ATTR_INCOMPLETE; + if (!xfs_hasdelattr(mp)) + entry->flags |= XFS_ATTR_INCOMPLETE; if ((args->blkno2 == args->blkno) && (args->index2 <= args->index)) { args->index2++;-- 2.7.4