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@xxxxxxxxxx>
Looks ok to me,
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--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
>
