Re: [PATCH] xfs: use ordered buffers to initialize dquot buffers during quotacheck

[Dave Chinner <david@xxxxxxxxxxxxx>]

On Tue, May 12, 2020 at 02:00:33PM -0700, Darrick J. Wong wrote:
> @@ -277,11 +279,34 @@ xfs_qm_init_dquot_blk(
>  		}
>  	}
>  
> -	xfs_trans_dquot_buf(tp, bp,
> -			    (type & XFS_DQ_USER ? XFS_BLF_UDQUOT_BUF :
> -			    ((type & XFS_DQ_PROJ) ? XFS_BLF_PDQUOT_BUF :
> -			     XFS_BLF_GDQUOT_BUF)));
> -	xfs_trans_log_buf(tp, bp, 0, BBTOB(q->qi_dqchunklen) - 1);
> +	if (type & XFS_DQ_USER) {
> +		qflag = XFS_UQUOTA_CHKD;
> +		blftype = XFS_BLF_UDQUOT_BUF;
> +	} else if (type & XFS_DQ_PROJ) {
> +		qflag = XFS_PQUOTA_CHKD;
> +		blftype = XFS_BLF_PDQUOT_BUF;
> +	} else {
> +		qflag = XFS_GQUOTA_CHKD;
> +		blftype = XFS_BLF_GDQUOT_BUF;
> +	}
> +
> +	xfs_trans_dquot_buf(tp, bp, blftype);
> +
> +	/*
> +	 * If the CHKD flag isn't set, we're running quotacheck and need to use
> +	 * ordered buffers so that the logged initialization buffer does not
> +	 * get replayed over the delwritten quotacheck buffer.  If we crash
> +	 * before the end of quotacheck, the CHKD flags will not be set in the
> +	 * superblock and we'll re-run quotacheck at next mount.
> +	 *
> +	 * Outside of quotacheck, dquot updates are logged via dquot items and
> +	 * we must use the regular buffer logging mechanisms to ensure that the
> +	 * initial buffer state is recovered before dquot items.
> +	 */
> +	if (mp->m_qflags & qflag)
> +		xfs_trans_log_buf(tp, bp, 0, BBTOB(q->qi_dqchunklen) - 1);
> +	else
> +		xfs_trans_ordered_buf(tp, bp);
>  }

That comment is ... difficult to understand. It conflates what we
are currently doing with what might happen in future if we did
something differently at the current time. IIUC, what you actually
mean is this:

	/*
	 * When quotacheck runs, we use delayed writes to update all the dquots
	 * on disk in an efficient manner instead of logging the individual
	 * dquot changes as they are made.
	 *
	 * Hence if we log the buffer that we allocate here, then crash
	 * post-quotacheck while the logged initialisation is still in the
	 * active region of the log, we can lose the information quotacheck
	 * wrote directly to the buffer. That is, log recovery will replay the
	 * dquot buffer initialisation over the top of whatever information
	 * quotacheck had written to the buffer.
	 *
	 * To avoid this problem, dquot allocation during quotacheck needs to
	 * avoid logging the initialised buffer, but we still need to have
	 * writeback of the buffer pin the tail of the log so that it is
	 * initialised on disk before we remove the allocation transaction from
	 * the active region of the log. Marking the buffer as ordered instead
	 * of logging it provides this behaviour.
	 */

Also, does this mean quotacheck completion should force the log and push the AIL
to ensure that all the allocations are completed and removed from the log before
marking the quota as CHKD?

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx