Re: [PATCH v2 7/7] xfs: eliminate duplicate icreate tx reservation functions

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

On Thu, Nov 30, 2017 at 01:58:36PM -0500, Brian Foster wrote:
> The create transaction reservation calculation has two different
> branches of code depending on whether the filesystem is a v5 format
> fs or older. Each branch considers the max reservation between the
> allocation case (new chunk allocation + record insert) and the
> modify case (chunk exists, record modification) of inode allocation.
> 
> The modify case is the same for both superblock versions with the
> exception of the finobt. The finobt helper checks the feature bit,
> however, and so the modify case already shares the same code.
> 
> Now that inode chunk allocation has been refactored into a helper
> that checks the superblock version to calculate the appropriate
> reservation for the create transaction, the only remaining
> difference between the create and icreate branches is the call to
> the finobt helper. As noted above, the finobt helper is a no-op when
> the feature is not enabled. Therefore, these branches are
> effectively duplicate and can be condensed.
> 
> Remove the xfs_calc_create_*() branch of functions and update the
> various callers to use the xfs_calc_icreate_*() variant. The latter
> creates the same reservation size for v4 create transactions as the
> removed branch. As such, this patch does not result in transaction
> reservation changes.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Looks ok for 4.16, will test...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/xfs/libxfs/xfs_trans_resv.c | 52 +++++-------------------------------------
>  1 file changed, 6 insertions(+), 46 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
> index 432dd7d7afea..f91e6680b0c5 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.c
> +++ b/fs/xfs/libxfs/xfs_trans_resv.c
> @@ -414,38 +414,12 @@ xfs_calc_create_resv_modify(
>  }
>  
>  /*
> - * For create we can allocate some inodes giving:
> - *    the agi and agf of the ag getting the new inodes: 2 * sectorsize
> - *    the superblock for the nlink flag: sector size
> - *    the inode chunk (allocation/init)
> - *    the inode btree (record insertion)
> - */
> -STATIC uint
> -xfs_calc_create_resv_alloc(
> -	struct xfs_mount	*mp)
> -{
> -	return xfs_calc_buf_res(2, mp->m_sb.sb_sectsize) +
> -		mp->m_sb.sb_sectsize +
> -		xfs_calc_inode_chunk_res(mp, _ALLOC) +
> -		xfs_calc_inobt_res(mp);
> -}
> -
> -STATIC uint
> -__xfs_calc_create_reservation(
> -	struct xfs_mount	*mp)
> -{
> -	return XFS_DQUOT_LOGRES(mp) +
> -		MAX(xfs_calc_create_resv_alloc(mp),
> -		    xfs_calc_create_resv_modify(mp));
> -}
> -
> -/*
>   * For icreate we can allocate some inodes giving:
>   *    the agi and agf of the ag getting the new inodes: 2 * sectorsize
>   *    the superblock for the nlink flag: sector size
> - *    the inode chunk (allocation, no init)
> + *    the inode chunk (allocation, optional init)
>   *    the inobt (record insertion)
> - *    the finobt (record insertion)
> + *    the finobt (optional, record insertion)
>   */
>  STATIC uint
>  xfs_calc_icreate_resv_alloc(
> @@ -467,26 +441,12 @@ xfs_calc_icreate_reservation(xfs_mount_t *mp)
>  }
>  
>  STATIC uint
> -xfs_calc_create_reservation(
> -	struct xfs_mount	*mp)
> -{
> -	if (xfs_sb_version_hascrc(&mp->m_sb))
> -		return xfs_calc_icreate_reservation(mp);
> -	return __xfs_calc_create_reservation(mp);
> -
> -}
> -
> -STATIC uint
>  xfs_calc_create_tmpfile_reservation(
>  	struct xfs_mount        *mp)
>  {
>  	uint	res = XFS_DQUOT_LOGRES(mp);
>  
> -	if (xfs_sb_version_hascrc(&mp->m_sb))
> -		res += xfs_calc_icreate_resv_alloc(mp);
> -	else
> -		res += xfs_calc_create_resv_alloc(mp);
> -
> +	res += xfs_calc_icreate_resv_alloc(mp);
>  	return res + xfs_calc_iunlink_add_reservation(mp);
>  }
>  
> @@ -497,7 +457,7 @@ STATIC uint
>  xfs_calc_mkdir_reservation(
>  	struct xfs_mount	*mp)
>  {
> -	return xfs_calc_create_reservation(mp);
> +	return xfs_calc_icreate_reservation(mp);
>  }
>  
>  
> @@ -510,7 +470,7 @@ STATIC uint
>  xfs_calc_symlink_reservation(
>  	struct xfs_mount	*mp)
>  {
> -	return xfs_calc_create_reservation(mp) +
> +	return xfs_calc_icreate_reservation(mp) +
>  	       xfs_calc_buf_res(1, XFS_SYMLINK_MAXLEN);
>  }
>  
> @@ -872,7 +832,7 @@ xfs_trans_resv_calc(
>  	resp->tr_symlink.tr_logcount = XFS_SYMLINK_LOG_COUNT;
>  	resp->tr_symlink.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
>  
> -	resp->tr_create.tr_logres = xfs_calc_create_reservation(mp);
> +	resp->tr_create.tr_logres = xfs_calc_icreate_reservation(mp);
>  	resp->tr_create.tr_logcount = XFS_CREATE_LOG_COUNT;
>  	resp->tr_create.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
>  
> -- 
> 2.13.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html