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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 25, 2020 at 04:30:07PM -0700, Allison Collins wrote:
> The xfs_trans_roll in _removename is not needed because invalidating
> blocks is an incore-only change.  This is analogous to the non-remote
> remove case where an entry is removed and a potential dabtree join
> occurs under the same transaction.
> 
> Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx>
> ---

Ok, but I think we should be a bit more descriptive in the commit log so
the reasoning is available for historical reference. For example:

"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."

With something like that in place:

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

>  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
> 




[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux