From: Chandan Rajendra <chandan@linux.ibm.com>
To: Allison Collins <allison.henderson@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH v7 13/19] xfs: Add delay ready attr remove routines
Date: Tue, 03 Mar 2020 10:33:43 +0530 [thread overview]
Message-ID: <5805763.2Ij3A3caSj@localhost.localdomain> (raw)
In-Reply-To: <20200223020611.1802-14-allison.henderson@oracle.com>
On Sunday, February 23, 2020 7:36 AM 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_iter, which uses a sort of state machine like switch to keep
> track of where it was when EAGAIN was returned. xfs_attr_node_removename has also
> been modified to use the switch, and a new version of xfs_attr_remove_args
> consists of a simple loop to refresh the transaction until the operation is
> completed.
>
> This patch also adds a new struct xfs_delattr_context, which we will use to keep
> track of the current state of an attribute operation. The new xfs_delattr_state
> enum is used to track various operations that are in progress so that we know not
> to repeat them, and resume where we left off before EAGAIN was returned to cycle
> out the transaction. Other members take the place of local variables that need
> to retain their values across multiple function recalls.
>
> Below is a state machine diagram for attr remove operations. The XFS_DAS_* states
> indicate places where the function would return -EAGAIN, and then immediately
> resume from after being recalled by the calling function. States marked as a
> "subroutine state" indicate that they belong to a subroutine, and so the calling
> function needs to pass them back to that subroutine to allow it to finish where
> it left off. But they otherwise do not have a role in the calling function other
> than just passing through.
>
> xfs_attr_remove_iter()
> XFS_DAS_RM_SHRINK ─â”
> (subroutine state) │
> │
> XFS_DAS_RMTVAL_REMOVE ─┤
> (subroutine state) │
> └─>xfs_attr_node_removename()
> │
> v
> need to remove
> ┌─n── rmt blocks?
> │ │
> │ y
> │ │
> │ v
> │ ┌─>XFS_DAS_RMTVAL_REMOVE
> │ │ │
> │ │ v
> │ └──y── more blks
> │ to remove?
> │ │
> │ n
> │ │
> │ v
> │ need to
> └─────> shrink tree? ─n─â”
> │ │
> y │
> │ │
> v │
> XFS_DAS_RM_SHRINK │
> │ │
> v │
> done <─────┘
>
> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
> ---
> fs/xfs/libxfs/xfs_attr.c | 114 +++++++++++++++++++++++++++++++++++++------
> fs/xfs/libxfs/xfs_attr.h | 1 +
> fs/xfs/libxfs/xfs_da_btree.h | 30 ++++++++++++
> fs/xfs/scrub/common.c | 2 +
> fs/xfs/xfs_acl.c | 2 +
> fs/xfs/xfs_attr_list.c | 1 +
> fs/xfs/xfs_ioctl.c | 2 +
> fs/xfs/xfs_ioctl32.c | 2 +
> fs/xfs/xfs_iops.c | 2 +
> fs/xfs/xfs_xattr.c | 1 +
> 10 files changed, 141 insertions(+), 16 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 5d73bdf..cd3a3f7 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -368,11 +368,60 @@ xfs_has_attr(
> */
> int
> xfs_attr_remove_args(
> + struct xfs_da_args *args)
> +{
> + int error = 0;
> + int err2 = 0;
> +
> + do {
> + error = xfs_attr_remove_iter(args);
> + if (error && error != -EAGAIN)
> + goto out;
> +
> + if (args->dac.flags & XFS_DAC_FINISH_TRANS) {
> + args->dac.flags &= ~XFS_DAC_FINISH_TRANS;
> +
> + err2 = xfs_defer_finish(&args->trans);
> + if (err2) {
> + error = err2;
> + goto out;
> + }
> + }
> +
> + err2 = xfs_trans_roll_inode(&args->trans, args->dp);
> + if (err2) {
> + error = err2;
> + goto out;
> + }
> +
> + } while (error == -EAGAIN);
> +out:
> + return error;
> +}
> +
> +/*
> + * Remove the attribute specified in @args.
> + *
> + * This function may return -EAGAIN to signal that the transaction needs to be
> + * rolled. Callers should continue calling this function until they receive a
> + * return value other than -EAGAIN.
> + */
> +int
> +xfs_attr_remove_iter(
> struct xfs_da_args *args)
> {
> struct xfs_inode *dp = args->dp;
> int error;
>
> + /* State machine switch */
> + switch (args->dac.dela_state) {
> + case XFS_DAS_RM_SHRINK:
> + case XFS_DAS_RMTVAL_REMOVE:
> + goto node;
> + default:
> + break;
> + }
> +
On the very first invocation of xfs_attr_remote_iter() from
xfs_attr_remove_args() (via a call from xfs_attr_remove()),
args->dac.dela_state is set to a value of 0. This happens because
xfs_attr_args_init() invokes memset() on args. A value of 0 for
args->dac.dela_state maps to XFS_DAS_RM_SHRINK.
If the xattr was stored in say local or leaf format we end up incorrectly
invoking xfs_attr_node_removename() right?
--
chandan
next prev parent reply other threads:[~2020-03-03 5:00 UTC|newest]
Thread overview: 135+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-23 2:05 [PATCH v7 00/19] xfs: Delayed Ready Attrs Allison Collins
2020-02-23 2:05 ` [PATCH v7 01/19] xfs: Replace attribute parameters with struct xfs_name Allison Collins
2020-02-23 9:34 ` Amir Goldstein
2020-02-23 16:03 ` Allison Collins
2020-02-25 0:49 ` Dave Chinner
2020-02-24 12:08 ` Chandan Rajendra
2020-02-24 16:25 ` Allison Collins
2020-02-24 13:06 ` Brian Foster
2020-02-24 16:25 ` Allison Collins
2020-02-23 2:05 ` [PATCH v7 02/19] xfs: Embed struct xfs_name in xfs_da_args Allison Collins
2020-02-23 11:54 ` Amir Goldstein
2020-02-23 16:51 ` Allison Collins
2020-02-24 6:50 ` Amir Goldstein
2020-02-24 7:36 ` Allison Collins
2020-02-24 7:43 ` Amir Goldstein
2020-02-25 0:57 ` Dave Chinner
2020-02-25 2:00 ` Allison Collins
2020-02-25 4:06 ` Dave Chinner
2020-02-25 4:19 ` Allison Collins
2020-02-25 4:27 ` Darrick J. Wong
2020-02-25 6:07 ` Allison Collins
2020-02-25 6:30 ` Dave Chinner
2020-02-25 17:21 ` Christoph Hellwig
2020-02-25 6:56 ` Chandan Rajendra
2020-02-25 23:26 ` Allison Collins
2020-02-23 2:05 ` [PATCH v7 03/19] xfs: Add xfs_has_attr and subroutines Allison Collins
2020-02-23 12:20 ` Amir Goldstein
2020-02-23 17:28 ` Allison Collins
2020-02-24 6:58 ` Amir Goldstein
2020-02-25 6:26 ` Dave Chinner
2020-02-25 6:43 ` Amir Goldstein
2020-02-25 22:27 ` Dave Chinner
2020-02-24 13:08 ` Brian Foster
2020-02-24 21:18 ` Allison Collins
2020-02-25 13:25 ` Brian Foster
2020-02-26 2:31 ` Allison Collins
2020-02-25 9:49 ` Chandan Rajendra
2020-02-25 10:15 ` Chandan Rajendra
2020-02-26 2:19 ` Allison Collins
2020-02-26 2:18 ` Allison Collins
2020-02-23 2:05 ` [PATCH v7 04/19] xfs: Check for -ENOATTR or -EEXIST Allison Collins
2020-02-23 12:25 ` Amir Goldstein
2020-02-23 17:33 ` Allison Collins
2020-02-24 13:08 ` Brian Foster
2020-02-24 21:18 ` Allison Collins
2020-02-23 2:05 ` [PATCH v7 05/19] xfs: Factor out new helper functions xfs_attr_rmtval_set Allison Collins
2020-02-25 12:53 ` Chandan Rajendra
2020-02-26 2:20 ` Allison Collins
2020-02-23 2:05 ` [PATCH v7 06/19] xfs: Factor out trans handling in xfs_attr3_leaf_flipflags Allison Collins
2020-02-23 12:30 ` Amir Goldstein
2020-02-23 17:36 ` Allison Collins
2020-02-28 4:56 ` Chandan Rajendra
2020-02-23 2:05 ` [PATCH v7 07/19] xfs: Factor out xfs_attr_leaf_addname helper Allison Collins
2020-02-23 12:42 ` Amir Goldstein
2020-02-23 18:38 ` Allison Collins
2020-02-24 6:38 ` Amir Goldstein
2020-02-24 7:09 ` Allison Collins
2020-02-24 7:27 ` Amir Goldstein
2020-02-25 6:42 ` Dave Chinner
2020-02-25 13:26 ` Brian Foster
2020-02-25 23:26 ` Allison Collins
2020-02-28 6:51 ` Chandan Rajendra
2020-02-23 2:06 ` [PATCH v7 08/19] xfs: Refactor xfs_attr_try_sf_addname Allison Collins
2020-02-23 13:04 ` Amir Goldstein
2020-02-23 17:51 ` Allison Collins
2020-02-24 13:08 ` Brian Foster
2020-02-24 21:19 ` Allison Collins
2020-02-28 7:42 ` Chandan Rajendra
2020-02-28 18:14 ` Allison Collins
2020-02-23 2:06 ` [PATCH v7 09/19] xfs: Factor out trans roll from xfs_attr3_leaf_setflag Allison Collins
2020-02-28 7:59 ` Chandan Rajendra
2020-02-23 2:06 ` [PATCH v7 10/19] xfs: Factor out xfs_attr_rmtval_invalidate Allison Collins
2020-02-28 10:42 ` Chandan Rajendra
2020-02-23 2:06 ` [PATCH v7 11/19] xfs: Factor out trans roll in xfs_attr3_leaf_clearflag Allison Collins
2020-02-28 10:56 ` Chandan Rajendra
2020-02-23 2:06 ` [PATCH v7 12/19] xfs: Add helper function xfs_attr_rmtval_unmap Allison Collins
2020-02-24 13:40 ` Brian Foster
2020-02-24 21:44 ` Allison Collins
2020-02-25 13:27 ` Brian Foster
2020-02-26 3:29 ` Allison Collins
2020-02-26 13:47 ` Brian Foster
2020-02-25 7:21 ` Dave Chinner
2020-02-25 23:27 ` Allison Collins
2020-02-28 14:22 ` Chandan Rajendra
2020-02-23 2:06 ` [PATCH v7 13/19] xfs: Add delay ready attr remove routines Allison Collins
2020-02-24 15:25 ` Brian Foster
2020-02-24 17:03 ` Brian Foster
2020-02-24 23:14 ` Allison Collins
2020-02-24 23:56 ` Darrick J. Wong
2020-02-25 13:34 ` Brian Foster
2020-02-26 5:36 ` Allison Collins
2020-02-26 13:48 ` Brian Foster
2020-02-26 19:23 ` Allison Collins
2020-02-25 8:57 ` Dave Chinner
2020-02-26 0:57 ` Allison Collins
2020-02-26 22:34 ` Dave Chinner
2020-02-27 4:18 ` Allison Collins
2020-03-03 5:03 ` Chandan Rajendra [this message]
2020-03-03 5:40 ` Allison Collins
2020-02-23 2:06 ` [PATCH v7 14/19] xfs: Add delay ready attr set routines Allison Collins
2020-03-03 13:41 ` Chandan Rajendra
2020-03-03 17:07 ` Allison Collins
2020-02-23 2:06 ` [PATCH v7 15/19] xfs: Add helper function xfs_attr_node_shrink Allison Collins
2020-02-23 13:22 ` Amir Goldstein
2020-02-23 18:41 ` Allison Collins
2020-02-25 9:05 ` Dave Chinner
2020-02-26 1:48 ` Allison Collins
2020-02-23 2:06 ` [PATCH v7 16/19] xfs: Simplify xfs_attr_set_iter Allison Collins
2020-02-23 13:26 ` Amir Goldstein
2020-02-23 18:42 ` Allison Collins
2020-02-25 9:21 ` Dave Chinner
2020-02-26 2:13 ` Allison Collins
2020-02-26 22:39 ` Dave Chinner
2020-03-04 4:30 ` Chandan Rajendra
2020-03-04 17:04 ` Allison Collins
2020-03-05 3:39 ` Chandan Rajendra
2020-02-23 2:06 ` [PATCH v7 17/19] xfs: Add helper function xfs_attr_leaf_mark_incomplete Allison Collins
2020-02-23 13:47 ` Amir Goldstein
2020-02-23 18:43 ` Allison Collins
2020-02-25 9:31 ` Dave Chinner
2020-02-26 2:17 ` Allison Collins
2020-03-04 4:37 ` Chandan Rajendra
2020-02-23 2:06 ` [PATCH v7 18/19] xfs: Add remote block helper functions Allison Collins
2020-02-23 13:45 ` Amir Goldstein
2020-03-04 4:59 ` Chandan Rajendra
2020-02-23 2:06 ` [PATCH v7 19/19] xfs: Remove xfs_attr_rmtval_remove Allison Collins
2020-02-23 13:54 ` Amir Goldstein
2020-02-23 18:50 ` Allison Collins
2020-02-23 7:55 ` [PATCH v7 00/19] xfs: Delayed Ready Attrs Amir Goldstein
2020-02-23 16:02 ` Allison Collins
2020-02-24 6:30 ` Amir Goldstein
2020-02-24 16:23 ` Allison Collins
2020-02-25 5:53 ` Amir Goldstein
2020-02-24 8:31 ` Chandan Rajendra
2020-02-25 9:52 ` Dave Chinner
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=5805763.2Ij3A3caSj@localhost.localdomain \
--to=chandan@linux.ibm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.