Re: [PATCH] xfs: drop extra transaction roll from inode extent truncate

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, Sep 10, 2020 at 09:29:26AM -0400, Brian Foster wrote:
> The inode extent truncate path unmaps extents from the inode block
> mapping, finishes deferred ops to free the associated extents and
> then explicitly rolls the transaction before processing the next
> extent. The latter extent roll is spurious as xfs_defer_finish()
> always returns a clean transaction and automatically relogs inodes
> attached to the transaction (with lock_flags == 0). This can
> unnecessarily increase the number of log ticket regrants that occur
> during a long running truncate operation. Remove the explicit
> transaction roll.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Looks ok to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> Just something I noticed when reading through the code based on Dave's
> recent EFI recovery reservation patches..
> 
> Brian
> 
>  fs/xfs/xfs_inode.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c06129cffba9..7af99c7a2821 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1532,17 +1532,10 @@ xfs_itruncate_extents_flags(
>  		if (error)
>  			goto out;
>  
> -		/*
> -		 * Duplicate the transaction that has the permanent
> -		 * reservation and commit the old transaction.
> -		 */
> +		/* free the just unmapped extents */
>  		error = xfs_defer_finish(&tp);
>  		if (error)
>  			goto out;
> -
> -		error = xfs_trans_roll_inode(&tp, ip);
> -		if (error)
> -			goto out;
>  	}
>  
>  	if (whichfork == XFS_DATA_FORK) {
> -- 
> 2.25.4
>