Re: [PATCH 07/28] xfs: refactor log recovery dquot item dispatch for pass2 commit functions

[Chandan Babu R <chandanrlinux@gmail.com>]

On Tuesday 5 May 2020 6:41:16 AM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move the log dquot item pass2 commit code into the per-item source code
> files and use the dispatch function to call it.  We do these one at a
> time because there's a lot of code to move.  No functional changes.
>

Dquot item pass2 processing is functionally consistent with what was done
before this patch is applied.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_dquot_item_recover.c |  109 ++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_log_recover.c        |  112 ---------------------------------------
>  2 files changed, 109 insertions(+), 112 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_dquot_item_recover.c b/fs/xfs/xfs_dquot_item_recover.c
> index ebc44c1bc2b1..07ff943972a3 100644
> --- a/fs/xfs/xfs_dquot_item_recover.c
> +++ b/fs/xfs/xfs_dquot_item_recover.c
> @@ -53,9 +53,118 @@ xlog_recover_dquot_ra_pass2(
>  			&xfs_dquot_buf_ra_ops);
>  }
>  
> +/*
> + * Recover a dquot record
> + */
> +STATIC int
> +xlog_recover_dquot_commit_pass2(
> +	struct xlog			*log,
> +	struct list_head		*buffer_list,
> +	struct xlog_recover_item	*item,
> +	xfs_lsn_t			current_lsn)
> +{
> +	struct xfs_mount		*mp = log->l_mp;
> +	struct xfs_buf			*bp;
> +	struct xfs_disk_dquot		*ddq, *recddq;
> +	struct xfs_dq_logformat		*dq_f;
> +	xfs_failaddr_t			fa;
> +	int				error;
> +	uint				type;
> +
> +	/*
> +	 * Filesystems are required to send in quota flags at mount time.
> +	 */
> +	if (mp->m_qflags == 0)
> +		return 0;
> +
> +	recddq = item->ri_buf[1].i_addr;
> +	if (recddq == NULL) {
> +		xfs_alert(log->l_mp, "NULL dquot in %s.", __func__);
> +		return -EFSCORRUPTED;
> +	}
> +	if (item->ri_buf[1].i_len < sizeof(struct xfs_disk_dquot)) {
> +		xfs_alert(log->l_mp, "dquot too small (%d) in %s.",
> +			item->ri_buf[1].i_len, __func__);
> +		return -EFSCORRUPTED;
> +	}
> +
> +	/*
> +	 * This type of quotas was turned off, so ignore this record.
> +	 */
> +	type = recddq->d_flags & (XFS_DQ_USER | XFS_DQ_PROJ | XFS_DQ_GROUP);
> +	ASSERT(type);
> +	if (log->l_quotaoffs_flag & type)
> +		return 0;
> +
> +	/*
> +	 * At this point we know that quota was _not_ turned off.
> +	 * Since the mount flags are not indicating to us otherwise, this
> +	 * must mean that quota is on, and the dquot needs to be replayed.
> +	 * Remember that we may not have fully recovered the superblock yet,
> +	 * so we can't do the usual trick of looking at the SB quota bits.
> +	 *
> +	 * The other possibility, of course, is that the quota subsystem was
> +	 * removed since the last mount - ENOSYS.
> +	 */
> +	dq_f = item->ri_buf[0].i_addr;
> +	ASSERT(dq_f);
> +	fa = xfs_dquot_verify(mp, recddq, dq_f->qlf_id, 0);
> +	if (fa) {
> +		xfs_alert(mp, "corrupt dquot ID 0x%x in log at %pS",
> +				dq_f->qlf_id, fa);
> +		return -EFSCORRUPTED;
> +	}
> +	ASSERT(dq_f->qlf_len == 1);
> +
> +	/*
> +	 * At this point we are assuming that the dquots have been allocated
> +	 * and hence the buffer has valid dquots stamped in it. It should,
> +	 * therefore, pass verifier validation. If the dquot is bad, then the
> +	 * we'll return an error here, so we don't need to specifically check
> +	 * the dquot in the buffer after the verifier has run.
> +	 */
> +	error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dq_f->qlf_blkno,
> +				   XFS_FSB_TO_BB(mp, dq_f->qlf_len), 0, &bp,
> +				   &xfs_dquot_buf_ops);
> +	if (error)
> +		return error;
> +
> +	ASSERT(bp);
> +	ddq = xfs_buf_offset(bp, dq_f->qlf_boffset);
> +
> +	/*
> +	 * If the dquot has an LSN in it, recover the dquot only if it's less
> +	 * than the lsn of the transaction we are replaying.
> +	 */
> +	if (xfs_sb_version_hascrc(&mp->m_sb)) {
> +		struct xfs_dqblk *dqb = (struct xfs_dqblk *)ddq;
> +		xfs_lsn_t	lsn = be64_to_cpu(dqb->dd_lsn);
> +
> +		if (lsn && lsn != -1 && XFS_LSN_CMP(lsn, current_lsn) >= 0) {
> +			goto out_release;
> +		}
> +	}
> +
> +	memcpy(ddq, recddq, item->ri_buf[1].i_len);
> +	if (xfs_sb_version_hascrc(&mp->m_sb)) {
> +		xfs_update_cksum((char *)ddq, sizeof(struct xfs_dqblk),
> +				 XFS_DQUOT_CRC_OFF);
> +	}
> +
> +	ASSERT(dq_f->qlf_size == 2);
> +	ASSERT(bp->b_mount == mp);
> +	bp->b_iodone = xlog_recover_iodone;
> +	xfs_buf_delwri_queue(bp, buffer_list);
> +
> +out_release:
> +	xfs_buf_relse(bp);
> +	return 0;
> +}
> +
>  const struct xlog_recover_item_ops xlog_dquot_item_ops = {
>  	.item_type		= XFS_LI_DQUOT,
>  	.ra_pass2		= xlog_recover_dquot_ra_pass2,
> +	.commit_pass2		= xlog_recover_dquot_commit_pass2,
>  };
>  
>  /*
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index cb5902550e8c..ea2a53b614c7 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2034,115 +2034,6 @@ xlog_buf_readahead(
>  		xfs_buf_readahead(log->l_mp->m_ddev_targp, blkno, len, ops);
>  }
>  
> -/*
> - * Recover a dquot record
> - */
> -STATIC int
> -xlog_recover_dquot_pass2(
> -	struct xlog			*log,
> -	struct list_head		*buffer_list,
> -	struct xlog_recover_item	*item,
> -	xfs_lsn_t			current_lsn)
> -{
> -	xfs_mount_t		*mp = log->l_mp;
> -	xfs_buf_t		*bp;
> -	struct xfs_disk_dquot	*ddq, *recddq;
> -	xfs_failaddr_t		fa;
> -	int			error;
> -	xfs_dq_logformat_t	*dq_f;
> -	uint			type;
> -
> -
> -	/*
> -	 * Filesystems are required to send in quota flags at mount time.
> -	 */
> -	if (mp->m_qflags == 0)
> -		return 0;
> -
> -	recddq = item->ri_buf[1].i_addr;
> -	if (recddq == NULL) {
> -		xfs_alert(log->l_mp, "NULL dquot in %s.", __func__);
> -		return -EFSCORRUPTED;
> -	}
> -	if (item->ri_buf[1].i_len < sizeof(struct xfs_disk_dquot)) {
> -		xfs_alert(log->l_mp, "dquot too small (%d) in %s.",
> -			item->ri_buf[1].i_len, __func__);
> -		return -EFSCORRUPTED;
> -	}
> -
> -	/*
> -	 * This type of quotas was turned off, so ignore this record.
> -	 */
> -	type = recddq->d_flags & (XFS_DQ_USER | XFS_DQ_PROJ | XFS_DQ_GROUP);
> -	ASSERT(type);
> -	if (log->l_quotaoffs_flag & type)
> -		return 0;
> -
> -	/*
> -	 * At this point we know that quota was _not_ turned off.
> -	 * Since the mount flags are not indicating to us otherwise, this
> -	 * must mean that quota is on, and the dquot needs to be replayed.
> -	 * Remember that we may not have fully recovered the superblock yet,
> -	 * so we can't do the usual trick of looking at the SB quota bits.
> -	 *
> -	 * The other possibility, of course, is that the quota subsystem was
> -	 * removed since the last mount - ENOSYS.
> -	 */
> -	dq_f = item->ri_buf[0].i_addr;
> -	ASSERT(dq_f);
> -	fa = xfs_dquot_verify(mp, recddq, dq_f->qlf_id, 0);
> -	if (fa) {
> -		xfs_alert(mp, "corrupt dquot ID 0x%x in log at %pS",
> -				dq_f->qlf_id, fa);
> -		return -EFSCORRUPTED;
> -	}
> -	ASSERT(dq_f->qlf_len == 1);
> -
> -	/*
> -	 * At this point we are assuming that the dquots have been allocated
> -	 * and hence the buffer has valid dquots stamped in it. It should,
> -	 * therefore, pass verifier validation. If the dquot is bad, then the
> -	 * we'll return an error here, so we don't need to specifically check
> -	 * the dquot in the buffer after the verifier has run.
> -	 */
> -	error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dq_f->qlf_blkno,
> -				   XFS_FSB_TO_BB(mp, dq_f->qlf_len), 0, &bp,
> -				   &xfs_dquot_buf_ops);
> -	if (error)
> -		return error;
> -
> -	ASSERT(bp);
> -	ddq = xfs_buf_offset(bp, dq_f->qlf_boffset);
> -
> -	/*
> -	 * If the dquot has an LSN in it, recover the dquot only if it's less
> -	 * than the lsn of the transaction we are replaying.
> -	 */
> -	if (xfs_sb_version_hascrc(&mp->m_sb)) {
> -		struct xfs_dqblk *dqb = (struct xfs_dqblk *)ddq;
> -		xfs_lsn_t	lsn = be64_to_cpu(dqb->dd_lsn);
> -
> -		if (lsn && lsn != -1 && XFS_LSN_CMP(lsn, current_lsn) >= 0) {
> -			goto out_release;
> -		}
> -	}
> -
> -	memcpy(ddq, recddq, item->ri_buf[1].i_len);
> -	if (xfs_sb_version_hascrc(&mp->m_sb)) {
> -		xfs_update_cksum((char *)ddq, sizeof(struct xfs_dqblk),
> -				 XFS_DQUOT_CRC_OFF);
> -	}
> -
> -	ASSERT(dq_f->qlf_size == 2);
> -	ASSERT(bp->b_mount == mp);
> -	bp->b_iodone = xlog_recover_iodone;
> -	xfs_buf_delwri_queue(bp, buffer_list);
> -
> -out_release:
> -	xfs_buf_relse(bp);
> -	return 0;
> -}
> -
>  /*
>   * This routine is called to create an in-core extent free intent
>   * item from the efi format structure which was logged on disk.
> @@ -2730,9 +2621,6 @@ xlog_recover_commit_pass2(
>  		return xlog_recover_bui_pass2(log, item, trans->r_lsn);
>  	case XFS_LI_BUD:
>  		return xlog_recover_bud_pass2(log, item);
> -	case XFS_LI_DQUOT:
> -		return xlog_recover_dquot_pass2(log, buffer_list, item,
> -						trans->r_lsn);
>  	case XFS_LI_ICREATE:
>  		return xlog_recover_do_icreate_pass2(log, buffer_list, item);
>  	case XFS_LI_QUOTAOFF:
> 
> 


-- 
chandan