Re: [PATCH 1/2] xfs: factor out quotaoff intent AIL removal and memory free

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

On Mon, Mar 16, 2020 at 01:00:31PM -0400, Brian Foster wrote:
> AIL removal of the quotaoff start intent and free of both intents is
> hardcoded to the ->iop_committed() handler of the end intent. Factor
> out the start intent handling code so it can be used in a future
> patch to properly handle quotaoff errors. Use xfs_trans_ail_remove()
> instead of the _delete() variant to acquire the AIL lock and also
> handle cases where an intent might not reside in the AIL at the
> time of a failure.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Looks like a straightforward hoist...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_dquot_item.c | 29 ++++++++++++++++++++---------
>  fs/xfs/xfs_dquot_item.h |  1 +
>  2 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
> index d60647d7197b..2b816e9b4465 100644
> --- a/fs/xfs/xfs_dquot_item.c
> +++ b/fs/xfs/xfs_dquot_item.c
> @@ -307,18 +307,10 @@ xfs_qm_qoffend_logitem_committed(
>  {
>  	struct xfs_qoff_logitem	*qfe = QOFF_ITEM(lip);
>  	struct xfs_qoff_logitem	*qfs = qfe->qql_start_lip;
> -	struct xfs_ail		*ailp = qfs->qql_item.li_ailp;
>  
> -	/*
> -	 * Delete the qoff-start logitem from the AIL.
> -	 * xfs_trans_ail_delete() drops the AIL lock.
> -	 */
> -	spin_lock(&ailp->ail_lock);
> -	xfs_trans_ail_delete(ailp, &qfs->qql_item, SHUTDOWN_LOG_IO_ERROR);
> +	xfs_qm_qoff_logitem_relse(qfs);
>  
> -	kmem_free(qfs->qql_item.li_lv_shadow);
>  	kmem_free(lip->li_lv_shadow);
> -	kmem_free(qfs);
>  	kmem_free(qfe);
>  	return (xfs_lsn_t)-1;
>  }
> @@ -336,6 +328,25 @@ static const struct xfs_item_ops xfs_qm_qoff_logitem_ops = {
>  	.iop_push	= xfs_qm_qoff_logitem_push,
>  };
>  
> +/*
> + * Delete the quotaoff intent from the AIL and free it. On success,
> + * this should only be called for the start item. It can be used for
> + * either on shutdown or abort.
> + */
> +void
> +xfs_qm_qoff_logitem_relse(
> +	struct xfs_qoff_logitem	*qoff)
> +{
> +	struct xfs_log_item	*lip = &qoff->qql_item;
> +
> +	ASSERT(test_bit(XFS_LI_IN_AIL, &lip->li_flags) ||
> +	       test_bit(XFS_LI_ABORTED, &lip->li_flags) ||
> +	       XFS_FORCED_SHUTDOWN(lip->li_mountp));
> +	xfs_trans_ail_remove(lip, SHUTDOWN_LOG_IO_ERROR);
> +	kmem_free(lip->li_lv_shadow);
> +	kmem_free(qoff);
> +}
> +
>  /*
>   * Allocate and initialize an quotaoff item of the correct quota type(s).
>   */
> diff --git a/fs/xfs/xfs_dquot_item.h b/fs/xfs/xfs_dquot_item.h
> index 3bb19e556ade..2b86a43d7ce2 100644
> --- a/fs/xfs/xfs_dquot_item.h
> +++ b/fs/xfs/xfs_dquot_item.h
> @@ -28,6 +28,7 @@ void xfs_qm_dquot_logitem_init(struct xfs_dquot *dqp);
>  struct xfs_qoff_logitem	*xfs_qm_qoff_logitem_init(struct xfs_mount *mp,
>  		struct xfs_qoff_logitem *start,
>  		uint flags);
> +void xfs_qm_qoff_logitem_relse(struct xfs_qoff_logitem *);
>  struct xfs_qoff_logitem	*xfs_trans_get_qoff_item(struct xfs_trans *tp,
>  		struct xfs_qoff_logitem *startqoff,
>  		uint flags);
> -- 
> 2.21.1
>