Re: [PATCH 07/12] xfs: abort consistently on dquot flush failure

[Allison Collins <allison.henderson@oracle.com>]

On 4/17/20 8:08 AM, Brian Foster wrote:
> The dquot flush handler effectively aborts the dquot flush if the
> filesystem is already shut down, but doesn't actually shut down if
> the flush fails. Update xfs_qm_dqflush() to consistently abort the
> dquot flush and shutdown the fs if the flush fails with an
> unexpected error.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
Ok, makes sense:
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   fs/xfs/xfs_dquot.c | 27 ++++++++-------------------
>   1 file changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 73032c18a94a..41750f797861 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -1068,6 +1068,7 @@ xfs_qm_dqflush(
>   	struct xfs_buf		**bpp)
>   {
>   	struct xfs_mount	*mp = dqp->q_mount;
> +	struct xfs_log_item	*lip = &dqp->q_logitem.qli_item;
>   	struct xfs_buf		*bp;
>   	struct xfs_dqblk	*dqb;
>   	struct xfs_disk_dquot	*ddqp;
> @@ -1082,32 +1083,16 @@ xfs_qm_dqflush(
>   
>   	xfs_qm_dqunpin_wait(dqp);
>   
> -	/*
> -	 * This may have been unpinned because the filesystem is shutting
> -	 * down forcibly. If that's the case we must not write this dquot
> -	 * to disk, because the log record didn't make it to disk.
> -	 *
> -	 * We also have to remove the log item from the AIL in this case,
> -	 * as we wait for an emptry AIL as part of the unmount process.
> -	 */
> -	if (XFS_FORCED_SHUTDOWN(mp)) {
> -		struct xfs_log_item	*lip = &dqp->q_logitem.qli_item;
> -		dqp->dq_flags &= ~XFS_DQ_DIRTY;
> -
> -		xfs_trans_ail_remove(lip, SHUTDOWN_CORRUPT_INCORE);
> -
> -		error = -EIO;
> -		goto out_unlock;
> -	}
> -
>   	/*
>   	 * Get the buffer containing the on-disk dquot
>   	 */
>   	error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno,
>   				   mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK,
>   				   &bp, &xfs_dquot_buf_ops);
> -	if (error)
> +	if (error == -EAGAIN)
>   		goto out_unlock;
> +	if (error)
> +		goto out_abort;
>   
>   	/*
>   	 * Calculate the location of the dquot inside the buffer.
> @@ -1161,6 +1146,10 @@ xfs_qm_dqflush(
>   	*bpp = bp;
>   	return 0;
>   
> +out_abort:
> +	dqp->dq_flags &= ~XFS_DQ_DIRTY;
> +	xfs_trans_ail_remove(lip, SHUTDOWN_CORRUPT_INCORE);
> +	xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
>   out_unlock:
>   	xfs_dqfunlock(dqp);
>   	return error;
>