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

Darrick J. Wong
2020-03-16T21:27:43Z

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
>