Re: [PATCH v11 14/25] xfs: Remove xfs_trans_roll in xfs_attr_node_removename

Darrick J. Wong
2020-07-22T00:29:58Z

On Mon, Jul 20, 2020 at 05:15:55PM -0700, Allison Collins wrote:
> A transaction roll is not necessary immediately after setting the
> INCOMPLETE flag when removing a node xattr entry with remote value
> blocks. The remote block invalidation that immediately follows setting
> the flag is an in-core only change. The next step after that is to start
> unmapping the remote blocks from the attr fork, but the xattr remove
> transaction reservation includes reservation for full tree splits of the
> dabtree and bmap tree. The remote block unmap code will roll the
> transaction as extents are unmapped and freed.
>
> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
> Reviewed-by: Brian Foster <bfoster@redhat.com>

Urrrk. The analysis is correct here, but whoooee was it hard to find.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> fs/xfs/libxfs/xfs_attr.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 1a78023..f1becca 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1148,10 +1148,6 @@ xfs_attr_node_removename(
> if (error)
> goto out;
>
> - error = xfs_trans_roll_inode(&args->trans, args->dp);
> - if (error)
> - goto out;
> -
> error = xfs_attr_rmtval_invalidate(args);
> if (error)
> return error;
> --
> 2.7.4
>