On 26 Mar 2021 at 06:03, Allison Henderson 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 is merged with
> xfs_attr_node_removename become a new function, xfs_attr_remove_iter.
> This new version uses a sort of state machine like switch to keep track
> of where it was when EAGAIN was returned. A new version of
> xfs_attr_remove_args consists of a simple loop to refresh the
> transaction until the operation is completed. A new XFS_DAC_DEFER_FINISH
> flag is used to finish the transaction where ever the existing code used
> to.
>
> Calls to xfs_attr_rmtval_remove are replaced with the delay ready
> version __xfs_attr_rmtval_remove. We will rename
> __xfs_attr_rmtval_remove back to xfs_attr_rmtval_remove when we are
> done.
>
> xfs_attr_rmtval_remove itself is still in use by the set routines (used
> during a rename). For reasons of preserving existing function, we
> modify xfs_attr_rmtval_remove to call xfs_defer_finish when the flag is
> set. Similar to how xfs_attr_remove_args does here. Once we transition
> the set routines to be delay ready, xfs_attr_rmtval_remove is no longer
> used and will be removed.
>
> 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. See xfs_attr.h for a more
> detailed diagram of the states.
>
> Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_attr.c | 206 +++++++++++++++++++++++++++-------------
> fs/xfs/libxfs/xfs_attr.h | 125 ++++++++++++++++++++++++
> fs/xfs/libxfs/xfs_attr_leaf.c | 2 +-
> fs/xfs/libxfs/xfs_attr_remote.c | 48 ++++++----
> fs/xfs/libxfs/xfs_attr_remote.h | 2 +-
[...]
> STATIC
> int xfs_attr_node_removename_setup(
> - struct xfs_da_args *args,
> - struct xfs_da_state **state)
> + struct xfs_delattr_context *dac)
> {
> - int error;
> + struct xfs_da_args *args = dac->da_args;
> + struct xfs_da_state **state = &dac->da_state;
> + int error;
>
> error = xfs_attr_node_hasname(args, state);
> if (error != -EEXIST)
> return error;
> + error = 0;
>
> ASSERT((*state)->path.blk[(*state)->path.active - 1].bp != NULL);
> ASSERT((*state)->path.blk[(*state)->path.active - 1].magic ==
> @@ -1204,10 +1233,13 @@ int xfs_attr_node_removename_setup(
> if (args->rmtblkno > 0) {
> error = xfs_attr_leaf_mark_incomplete(args, *state);
> if (error)
> - return error;
> + goto out;
>
> - return xfs_attr_rmtval_invalidate(args);
> + error = xfs_attr_rmtval_invalidate(args);
> }
> +out:
> + if (error)
> + xfs_da_state_free(*state);
>
> return 0;
If the call to xfs_attr_rmtval_invalidate() returned a non-zero value, the
above change would cause xfs_attr_node_removename_setup() to incorrectly
return success.
> }
> @@ -1232,70 +1264,114 @@ xfs_attr_node_remove_cleanup(
> }
>
--
chandan
