Re: [PATCH v4.2 4/5] xfs: xfs_defer_capture should absorb remaining block reservation

[Brian Foster <bfoster@redhat.com>]

On Thu, Oct 01, 2020 at 09:20:15PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When xfs_defer_capture extracts the deferred ops and transaction state
> from a transaction, it should record the remaining block reservations so
> that when we continue the dfops chain, we can reserve the same number of
> blocks to use.  We capture the reservations for both data and realtime
> volumes.
> 
> This adds the requirement that every log intent item recovery function
> must be careful to reserve enough blocks to handle both itself and all
> defer ops that it can queue.  On the other hand, this enables us to do
> away with the handwaving block estimation nonsense that was going on in
> xlog_finish_defer_ops.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> v4.2: don't fiddle with transaction internals, and save the rt
> reservation too
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_defer.c |    4 ++++
>  fs/xfs/libxfs/xfs_defer.h |    4 ++++
>  fs/xfs/xfs_log_recover.c  |   21 +++------------------
>  3 files changed, 11 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index 85c371d29e8d..10aeae7353ab 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -575,6 +575,10 @@ xfs_defer_ops_capture(
>  	dfc->dfc_tpflags = tp->t_flags & XFS_TRANS_LOWMODE;
>  	tp->t_flags &= ~XFS_TRANS_LOWMODE;
>  
> +	/* Capture the remaining block reservations along with the dfops. */
> +	dfc->dfc_blkres = tp->t_blk_res - tp->t_blk_res_used;
> +	dfc->dfc_rtxres = tp->t_rtx_res - tp->t_rtx_res_used;
> +
>  	return dfc;
>  }
>  
> diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
> index 3af82ebc1249..5c0e59b69ffa 100644
> --- a/fs/xfs/libxfs/xfs_defer.h
> +++ b/fs/xfs/libxfs/xfs_defer.h
> @@ -75,6 +75,10 @@ struct xfs_defer_capture {
>  	/* Deferred ops state saved from the transaction. */
>  	struct list_head	dfc_dfops;
>  	unsigned int		dfc_tpflags;
> +
> +	/* Block reservations for the data and rt devices. */
> +	unsigned int		dfc_blkres;
> +	unsigned int		dfc_rtxres;
>  };
>  
>  /*
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 7804906d145b..1be5208e2a2f 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2439,27 +2439,12 @@ xlog_finish_defer_ops(
>  {
>  	struct xfs_defer_capture *dfc, *next;
>  	struct xfs_trans	*tp;
> -	int64_t			freeblks;
> -	uint64_t		resblks;
>  	int			error = 0;
>  
>  	list_for_each_entry_safe(dfc, next, capture_list, dfc_list) {
> -		/*
> -		 * We're finishing the defer_ops that accumulated as a result
> -		 * of recovering unfinished intent items during log recovery.
> -		 * We reserve an itruncate transaction because it is the
> -		 * largest permanent transaction type.  Since we're the only
> -		 * user of the fs right now, take 93% (15/16) of the available
> -		 * free blocks.  Use weird math to avoid a 64-bit division.
> -		 */
> -		freeblks = percpu_counter_sum(&mp->m_fdblocks);
> -		if (freeblks <= 0)
> -			return -ENOSPC;
> -
> -		resblks = min_t(uint64_t, UINT_MAX, freeblks);
> -		resblks = (resblks * 15) >> 4;
> -		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, resblks,
> -				0, XFS_TRANS_RESERVE, &tp);
> +		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
> +				dfc->dfc_blkres, dfc->dfc_rtxres,
> +				XFS_TRANS_RESERVE, &tp);
>  		if (error)
>  			return error;
>  
>