From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Allison Collins <allison.henderson@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH v4 16/17] xfs: Add delay ready attr remove routines
Date: Fri, 8 Nov 2019 13:37:01 -0800 [thread overview]
Message-ID: <20191108213701.GG6219@magnolia> (raw)
In-Reply-To: <20191107012801.22863-17-allison.henderson@oracle.com>
On Wed, Nov 06, 2019 at 06:28:00PM -0700, Allison Collins wrote:
> This patch modifies the attr remove routines to be delay ready.
> This means they no longer roll or commit transactions, but instead
> return -EAGAIN to have the calling routine roll and refresh the
> transaction. In this series, xfs_attr_remove_args has become
> xfs_attr_remove_later, which uses a state machine to keep track
> of where it was when EAGAIN was returned. xfs_attr_node_removename
> has also been modified to use the state machine, and a new version of
> xfs_attr_remove_args consists of a simple loop to refresh the
> transaction until the operation is completed.
>
> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
> ---
> fs/xfs/libxfs/xfs_attr.c | 123 +++++++++++++++++++++++++++++++++++++++--------
> fs/xfs/libxfs/xfs_attr.h | 1 +
> 2 files changed, 104 insertions(+), 20 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 626d4a98..38d5c5c 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -369,10 +369,56 @@ xfs_has_attr(
> */
> int
> xfs_attr_remove_args(
> + struct xfs_da_args *args)
> +{
> + int error = 0;
> + int err2 = 0;
> +
> + do {
> + error = xfs_attr_remove_later(args);
> + if (error && error != -EAGAIN)
> + goto out;
> +
> + xfs_trans_log_inode(args->trans, args->dp,
> + XFS_ILOG_CORE | XFS_ILOG_ADATA);
Don't the individual pieces of attr removal log ADATA on their own, when
needed?
If it weren't for that, this could just be xfs_trans_roll_inode, right?
--D
> +
> + err2 = xfs_trans_roll(&args->trans);
> + if (err2) {
> + error = err2;
> + goto out;
> + }
> +
> + /* Rejoin inode */
> + xfs_trans_ijoin(args->trans, args->dp, 0);
> +
> + } while (error == -EAGAIN);
> +out:
> + return error;
> +}
> +
> +/*
> + * Remove the attribute specified in @args.
> + * This routine is meant to function as a delayed operation, and may return
> + * -EGAIN 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.
> + */
> +int
> +xfs_attr_remove_later(
> struct xfs_da_args *args)
> {
> struct xfs_inode *dp = args->dp;
> - int error;
> + int error = 0;
> +
> + /* State machine switch */
> + switch (args->dc.dc_state) {
> + case XFS_DC_RM_INVALIDATE:
> + case XFS_DC_RM_SHRINK:
> + case XFS_DC_RM_NODE_BLKS:
> + goto node;
> + default:
> + break;
> + }
>
> if (!xfs_inode_hasattr(dp)) {
> error = -ENOATTR;
> @@ -382,6 +428,7 @@ xfs_attr_remove_args(
> } else if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
> error = xfs_attr_leaf_removename(args);
> } else {
> +node:
> error = xfs_attr_node_removename(args);
> }
>
> @@ -892,9 +939,6 @@ xfs_attr_leaf_removename(
> /* bp is gone due to xfs_da_shrink_inode */
> if (error)
> return error;
> - error = xfs_defer_finish(&args->trans);
> - if (error)
> - return error;
> }
> return 0;
> }
> @@ -1212,6 +1256,11 @@ xfs_attr_node_addname(
> * This will involve walking down the Btree, and may involve joining
> * leaf nodes and even joining intermediate nodes up to and including
> * the root node (a special case of an intermediate node).
> + *
> + * 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_removename(
> @@ -1222,12 +1271,29 @@ xfs_attr_node_removename(
> struct xfs_buf *bp;
> int retval, error, forkoff;
> struct xfs_inode *dp = args->dp;
> + int done = 0;
>
> trace_xfs_attr_node_removename(args);
> + state = args->dc.da_state;
> + blk = args->dc.blk;
> +
> + /* State machine switch */
> + switch (args->dc.dc_state) {
> + case XFS_DC_RM_NODE_BLKS:
> + goto rm_node_blks;
> + case XFS_DC_RM_INVALIDATE:
> + goto rm_invalidate;
> + case XFS_DC_RM_SHRINK:
> + goto rm_shrink;
> + default:
> + break;
> + }
>
> error = xfs_attr_node_hasname(args, &state);
> if (error != -EEXIST)
> goto out;
> + else
> + error = 0;
>
> /*
> * If there is an out-of-line value, de-allocate the blocks.
> @@ -1237,6 +1303,14 @@ xfs_attr_node_removename(
> blk = &state->path.blk[ state->path.active-1 ];
> ASSERT(blk->bp != NULL);
> ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
> +
> + /*
> + * Store blk and state in the context incase we need to cycle out the
> + * transaction
> + */
> + args->dc.blk = blk;
> + args->dc.da_state = state;
> +
> if (args->rmtblkno > 0) {
> /*
> * Fill in disk block numbers in the state structure
> @@ -1255,13 +1329,30 @@ xfs_attr_node_removename(
> if (error)
> goto out;
>
> - error = xfs_trans_roll_inode(&args->trans, args->dp);
> + args->dc.dc_state = XFS_DC_RM_INVALIDATE;
> + return -EAGAIN;
> +rm_invalidate:
> + error = xfs_attr_rmtval_invalidate(args);
> if (error)
> goto out;
> +rm_node_blks:
> + /*
> + * Unmap value blocks for this attr. This is similar to
> + * xfs_attr_rmtval_remove, but open coded here to return EAGAIN
> + * for new transactions
> + */
> + while (!done && !error) {
> + error = xfs_bunmapi(args->trans, args->dp,
> + args->rmtblkno, args->rmtblkcnt,
> + XFS_BMAPI_ATTRFORK, 1, &done);
> + if (error)
> + return error;
>
> - error = xfs_attr_rmtval_remove(args);
> - if (error)
> - goto out;
> + if (!done) {
> + args->dc.dc_state = XFS_DC_RM_NODE_BLKS;
> + return -EAGAIN;
> + }
> + }
>
> /*
> * Refill the state structure with buffers, the prior calls
> @@ -1287,17 +1378,12 @@ xfs_attr_node_removename(
> error = xfs_da3_join(state);
> if (error)
> goto out;
> - error = xfs_defer_finish(&args->trans);
> - if (error)
> - goto out;
> - /*
> - * Commit the Btree join operation and start a new trans.
> - */
> - error = xfs_trans_roll_inode(&args->trans, dp);
> - if (error)
> - goto out;
> +
> + args->dc.dc_state = XFS_DC_RM_SHRINK;
> + return -EAGAIN;
> }
>
> +rm_shrink:
> /*
> * If the result is small enough, push it all into the inode.
> */
> @@ -1319,9 +1405,6 @@ xfs_attr_node_removename(
> /* bp is gone due to xfs_da_shrink_inode */
> if (error)
> goto out;
> - error = xfs_defer_finish(&args->trans);
> - if (error)
> - goto out;
> } else
> xfs_trans_brelse(args->trans, bp);
> }
> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> index 3b5dad4..fb8bf5b 100644
> --- a/fs/xfs/libxfs/xfs_attr.h
> +++ b/fs/xfs/libxfs/xfs_attr.h
> @@ -152,6 +152,7 @@ int xfs_attr_set_args(struct xfs_da_args *args);
> int xfs_attr_remove(struct xfs_inode *dp, struct xfs_name *name, int flags);
> int xfs_has_attr(struct xfs_da_args *args);
> int xfs_attr_remove_args(struct xfs_da_args *args);
> +int xfs_attr_remove_later(struct xfs_da_args *args);
> int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
> int flags, struct attrlist_cursor_kern *cursor);
> bool xfs_attr_namecheck(const void *name, size_t length);
> --
> 2.7.4
>
next prev parent reply other threads:[~2019-11-08 21:37 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-07 1:27 [PATCH v4 00/17] xfs: Delay Ready Attributes Allison Collins
2019-11-07 1:27 ` [PATCH v4 01/17] xfs: Remove all strlen in all xfs_attr_* functions for attr names Allison Collins
2019-11-11 17:47 ` Christoph Hellwig
2019-11-11 23:35 ` Allison Collins
2019-11-07 1:27 ` [PATCH v4 02/17] xfs: Replace attribute parameters with struct xfs_name Allison Collins
2019-11-08 1:13 ` Darrick J. Wong
2019-11-08 17:16 ` Allison Collins
2019-11-11 17:49 ` Christoph Hellwig
2019-11-11 20:07 ` Allison Collins
2019-11-13 15:12 ` Allison Collins
2019-11-20 18:20 ` Christoph Hellwig
2019-11-07 1:27 ` [PATCH v4 03/17] xfs: Embed struct xfs_name in xfs_da_args Allison Collins
2019-11-08 1:25 ` Darrick J. Wong
2019-11-08 16:11 ` Allison Collins
2019-11-08 21:47 ` Darrick J. Wong
2019-11-07 1:27 ` [PATCH v4 04/17] xfs: Add xfs_dabuf defines Allison Collins
2019-11-08 19:19 ` Darrick J. Wong
2019-11-09 17:32 ` Allison Collins
2019-11-09 20:11 ` Darrick J. Wong
2019-11-09 22:06 ` Allison Collins
2019-11-07 1:27 ` [PATCH v4 05/17] xfs: Add xfs_has_attr and subroutines Allison Collins
2019-11-08 19:32 ` Darrick J. Wong
2019-11-08 19:51 ` Allison Collins
2019-11-11 17:40 ` Brian Foster
2019-11-11 23:34 ` Allison Collins
2019-11-11 17:53 ` Christoph Hellwig
2019-11-11 23:36 ` Allison Collins
2019-11-07 1:27 ` [PATCH v4 06/17] xfs: Factor out new helper functions xfs_attr_rmtval_set Allison Collins
2019-11-08 19:34 ` Darrick J. Wong
2019-11-08 19:51 ` Allison Collins
2019-11-07 1:27 ` [PATCH v4 07/17] xfs: Factor up trans handling in xfs_attr3_leaf_flipflags Allison Collins
2019-11-08 19:35 ` Darrick J. Wong
2019-11-08 19:52 ` Allison Collins
2019-11-07 1:27 ` [PATCH v4 08/17] xfs: Factor out xfs_attr_leaf_addname helper Allison Collins
2019-11-08 20:57 ` Darrick J. Wong
2019-11-09 21:41 ` Allison Collins
2019-11-07 1:27 ` [PATCH v4 09/17] xfs: Factor up commit from xfs_attr_try_sf_addname Allison Collins
2019-11-08 21:04 ` Darrick J. Wong
2019-11-08 23:13 ` Allison Collins
2019-11-07 1:27 ` [PATCH v4 10/17] xfs: Factor up trans roll from xfs_attr3_leaf_setflag Allison Collins
2019-11-07 1:27 ` [PATCH v4 11/17] xfs: Add xfs_attr3_leaf helper functions Allison Collins
2019-11-08 21:17 ` Darrick J. Wong
2019-11-09 0:09 ` Allison Collins
2019-11-07 1:27 ` [PATCH v4 12/17] xfs: Factor out xfs_attr_rmtval_invalidate Allison Collins
2019-11-08 21:19 ` Darrick J. Wong
2019-11-09 0:10 ` Allison Collins
2019-11-07 1:27 ` [PATCH v4 13/17] xfs: Factor up trans roll in xfs_attr3_leaf_clearflag Allison Collins
2019-11-08 21:19 ` Darrick J. Wong
2019-11-09 0:11 ` Allison Collins
2019-11-11 18:23 ` Brian Foster
2019-11-11 23:37 ` Allison Collins
2019-11-07 1:27 ` [PATCH v4 14/17] xfs: Add delay context to xfs_da_args Allison Collins
2019-11-08 21:22 ` Darrick J. Wong
2019-11-09 0:23 ` Allison Collins
2019-11-11 18:23 ` Brian Foster
2019-11-11 23:42 ` Allison Collins
2019-11-07 1:27 ` [PATCH v4 15/17] xfs: Check for -ENOATTR or -EEXIST Allison Collins
2019-11-08 21:28 ` Darrick J. Wong
2019-11-08 21:42 ` Allison Collins
2019-11-08 21:51 ` Darrick J. Wong
2019-11-11 18:24 ` Brian Foster
2019-11-12 0:33 ` Allison Collins
2019-11-07 1:28 ` [PATCH v4 16/17] xfs: Add delay ready attr remove routines Allison Collins
2019-11-08 21:37 ` Darrick J. Wong [this message]
2019-11-09 0:25 ` Allison Collins
2019-11-12 13:37 ` Brian Foster
2019-11-13 0:43 ` Allison Collins
2019-11-13 11:54 ` Brian Foster
2019-11-13 23:39 ` Allison Collins
2019-11-14 12:48 ` Brian Foster
2019-11-14 17:58 ` Allison Collins
2019-11-07 1:28 ` [PATCH v4 17/17] xfs: Add delay ready attr set routines Allison Collins
2019-11-08 21:42 ` Darrick J. Wong
2019-11-08 21:52 ` Allison Collins
2019-11-09 4:07 ` Allison Collins
2019-11-12 13:37 ` Brian Foster
2019-11-13 4:57 ` Allison Collins
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191108213701.GG6219@magnolia \
--to=darrick.wong@oracle.com \
--cc=allison.henderson@oracle.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox