[11/18] xfs: remove qcore from incore dquots
diff mbox series

Message ID 159353178119.2864738.14352743945962585449.stgit@magnolia
State Superseded
Headers show
Series
  • xfs: remove xfs_disk_quot from incore dquot
Related show

Commit Message

Darrick J. Wong June 30, 2020, 3:43 p.m. UTC
From: Darrick J. Wong <darrick.wong@oracle.com>

Now that we've stopped using qcore entirely, drop it from the incore
dquot.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/scrub/quota.c |    4 ----
 fs/xfs/xfs_dquot.c   |   29 +++++++++--------------------
 fs/xfs/xfs_dquot.h   |    1 -
 3 files changed, 9 insertions(+), 25 deletions(-)

Comments

Chandan Babu R July 1, 2020, 8:34 a.m. UTC | #1
On Tuesday 30 June 2020 9:13:01 PM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Now that we've stopped using qcore entirely, drop it from the incore
> dquot.
>

The changes are logically correct.

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

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/scrub/quota.c |    4 ----
>  fs/xfs/xfs_dquot.c   |   29 +++++++++--------------------
>  fs/xfs/xfs_dquot.h   |    1 -
>  3 files changed, 9 insertions(+), 25 deletions(-)
> 
> 
> diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
> index 2fc2625feca0..f4aad5b00188 100644
> --- a/fs/xfs/scrub/quota.c
> +++ b/fs/xfs/scrub/quota.c
> @@ -79,7 +79,6 @@ xchk_quota_item(
>  	struct xchk_quota_info	*sqi = priv;
>  	struct xfs_scrub	*sc = sqi->sc;
>  	struct xfs_mount	*mp = sc->mp;
> -	struct xfs_disk_dquot	*d = &dq->q_core;
>  	struct xfs_quotainfo	*qi = mp->m_quotainfo;
>  	xfs_fileoff_t		offset;
>  	xfs_ino_t		fs_icount;
> @@ -98,9 +97,6 @@ xchk_quota_item(
>  
>  	sqi->last_id = dq->q_id;
>  
> -	if (d->d_pad0 != cpu_to_be32(0) || d->d_pad != cpu_to_be16(0))
> -		xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, offset);
> -
>  	/*
>  	 * Warn if the hard limits are larger than the fs.
>  	 * Administrators can do this, though in production this seems
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 7434ee57ec43..2d6b50760962 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -529,7 +529,6 @@ xfs_dquot_from_disk(
>  	}
>  
>  	/* copy everything from disk dquot to the incore dquot */
> -	memcpy(&dqp->q_core, ddqp, sizeof(struct xfs_disk_dquot));
>  	dqp->q_blk.hardlimit = be64_to_cpu(ddqp->d_blk_hardlimit);
>  	dqp->q_blk.softlimit = be64_to_cpu(ddqp->d_blk_softlimit);
>  	dqp->q_ino.hardlimit = be64_to_cpu(ddqp->d_ino_hardlimit);
> @@ -568,8 +567,13 @@ xfs_dquot_to_disk(
>  	struct xfs_disk_dquot	*ddqp,
>  	struct xfs_dquot	*dqp)
>  {
> -	memcpy(ddqp, &dqp->q_core, sizeof(struct xfs_disk_dquot));
> +	ddqp->d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
> +	ddqp->d_version = XFS_DQUOT_VERSION;
>  	ddqp->d_flags = dqp->dq_flags & XFS_DQ_ONDISK;
> +	ddqp->d_id = cpu_to_be32(dqp->q_id);
> +	ddqp->d_pad0 = 0;
> +	ddqp->d_pad = 0;
> +
>  	ddqp->d_blk_hardlimit = cpu_to_be64(dqp->q_blk.hardlimit);
>  	ddqp->d_blk_softlimit = cpu_to_be64(dqp->q_blk.softlimit);
>  	ddqp->d_ino_hardlimit = cpu_to_be64(dqp->q_ino.hardlimit);
> @@ -1180,7 +1184,6 @@ xfs_qm_dqflush(
>  	struct xfs_log_item	*lip = &dqp->q_logitem.qli_item;
>  	struct xfs_buf		*bp;
>  	struct xfs_dqblk	*dqb;
> -	struct xfs_disk_dquot	*ddqp;
>  	xfs_failaddr_t		fa;
>  	int			error;
>  
> @@ -1204,22 +1207,6 @@ xfs_qm_dqflush(
>  	if (error)
>  		goto out_abort;
>  
> -	/*
> -	 * Calculate the location of the dquot inside the buffer.
> -	 */
> -	dqb = bp->b_addr + dqp->q_bufoffset;
> -	ddqp = &dqb->dd_diskdq;
> -
> -	/* sanity check the in-core structure before we flush */
> -	fa = xfs_dquot_verify(mp, &dqp->q_core, dqp->q_id, 0);
> -	if (fa) {
> -		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
> -				dqp->q_id, fa);
> -		xfs_buf_relse(bp);
> -		error = -EFSCORRUPTED;
> -		goto out_abort;
> -	}
> -
>  	fa = xfs_qm_dqflush_check(dqp);
>  	if (fa) {
>  		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
> @@ -1229,7 +1216,9 @@ xfs_qm_dqflush(
>  		goto out_abort;
>  	}
>  
> -	xfs_dquot_to_disk(ddqp, dqp);
> +	/* Flush the incore dquot to the ondisk buffer. */
> +	dqb = bp->b_addr + dqp->q_bufoffset;
> +	xfs_dquot_to_disk(&dqb->dd_diskdq, dqp);
>  
>  	/*
>  	 * Clear the dirty field and remember the flush lsn for later use.
> diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> index 414bae537b1d..62b0fc6e0133 100644
> --- a/fs/xfs/xfs_dquot.h
> +++ b/fs/xfs/xfs_dquot.h
> @@ -71,7 +71,6 @@ struct xfs_dquot {
>  	struct xfs_dquot_res	q_ino;	/* inodes */
>  	struct xfs_dquot_res	q_rtb;	/* realtime blocks */
>  
> -	struct xfs_disk_dquot	q_core;
>  	struct xfs_dq_logitem	q_logitem;
>  
>  	xfs_qcnt_t		q_prealloc_lo_wmark;
> 
>
Christoph Hellwig July 1, 2020, 8:52 a.m. UTC | #2
On Tue, Jun 30, 2020 at 08:43:01AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Now that we've stopped using qcore entirely, drop it from the incore
> dquot.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
Dave Chinner July 1, 2020, 11:07 p.m. UTC | #3
On Tue, Jun 30, 2020 at 08:43:01AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Now that we've stopped using qcore entirely, drop it from the incore
> dquot.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
....
> @@ -1180,7 +1184,6 @@ xfs_qm_dqflush(
>  	struct xfs_log_item	*lip = &dqp->q_logitem.qli_item;
>  	struct xfs_buf		*bp;
>  	struct xfs_dqblk	*dqb;
> -	struct xfs_disk_dquot	*ddqp;
>  	xfs_failaddr_t		fa;
>  	int			error;
>  
> @@ -1204,22 +1207,6 @@ xfs_qm_dqflush(
>  	if (error)
>  		goto out_abort;
>  
> -	/*
> -	 * Calculate the location of the dquot inside the buffer.
> -	 */
> -	dqb = bp->b_addr + dqp->q_bufoffset;
> -	ddqp = &dqb->dd_diskdq;
> -
> -	/* sanity check the in-core structure before we flush */
> -	fa = xfs_dquot_verify(mp, &dqp->q_core, dqp->q_id, 0);
> -	if (fa) {
> -		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
> -				dqp->q_id, fa);
> -		xfs_buf_relse(bp);
> -		error = -EFSCORRUPTED;
> -		goto out_abort;
> -	}
> -
>  	fa = xfs_qm_dqflush_check(dqp);
>  	if (fa) {
>  		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
> @@ -1229,7 +1216,9 @@ xfs_qm_dqflush(
>  		goto out_abort;
>  	}
>  
> -	xfs_dquot_to_disk(ddqp, dqp);
> +	/* Flush the incore dquot to the ondisk buffer. */
> +	dqb = bp->b_addr + dqp->q_bufoffset;
> +	xfs_dquot_to_disk(&dqb->dd_diskdq, dqp);

Oh, this is really hard to read now. d, q, b, and p are all the same
shape just at different rotations/mirroring, so this now just looks
like a jumble of random letter salad...

Can you rename dqb to dqblk so it's clearly the pointer to the
on-disk dquot block and so easy to differentiate at a glance from
the in-memory dquot pointer?

Cheers,

Dave.
Darrick J. Wong July 1, 2020, 11:14 p.m. UTC | #4
On Thu, Jul 02, 2020 at 09:07:54AM +1000, Dave Chinner wrote:
> On Tue, Jun 30, 2020 at 08:43:01AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Now that we've stopped using qcore entirely, drop it from the incore
> > dquot.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ....
> > @@ -1180,7 +1184,6 @@ xfs_qm_dqflush(
> >  	struct xfs_log_item	*lip = &dqp->q_logitem.qli_item;
> >  	struct xfs_buf		*bp;
> >  	struct xfs_dqblk	*dqb;
> > -	struct xfs_disk_dquot	*ddqp;
> >  	xfs_failaddr_t		fa;
> >  	int			error;
> >  
> > @@ -1204,22 +1207,6 @@ xfs_qm_dqflush(
> >  	if (error)
> >  		goto out_abort;
> >  
> > -	/*
> > -	 * Calculate the location of the dquot inside the buffer.
> > -	 */
> > -	dqb = bp->b_addr + dqp->q_bufoffset;
> > -	ddqp = &dqb->dd_diskdq;
> > -
> > -	/* sanity check the in-core structure before we flush */
> > -	fa = xfs_dquot_verify(mp, &dqp->q_core, dqp->q_id, 0);
> > -	if (fa) {
> > -		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
> > -				dqp->q_id, fa);
> > -		xfs_buf_relse(bp);
> > -		error = -EFSCORRUPTED;
> > -		goto out_abort;
> > -	}
> > -
> >  	fa = xfs_qm_dqflush_check(dqp);
> >  	if (fa) {
> >  		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
> > @@ -1229,7 +1216,9 @@ xfs_qm_dqflush(
> >  		goto out_abort;
> >  	}
> >  
> > -	xfs_dquot_to_disk(ddqp, dqp);
> > +	/* Flush the incore dquot to the ondisk buffer. */
> > +	dqb = bp->b_addr + dqp->q_bufoffset;
> > +	xfs_dquot_to_disk(&dqb->dd_diskdq, dqp);
> 
> Oh, this is really hard to read now. d, q, b, and p are all the same
> shape just at different rotations/mirroring, so this now just looks
> like a jumble of random letter salad...
> 
> Can you rename dqb to dqblk so it's clearly the pointer to the
> on-disk dquot block and so easy to differentiate at a glance from
> the in-memory dquot pointer?

Ok.

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
Allison Collins July 2, 2020, 2:06 a.m. UTC | #5
On 6/30/20 8:43 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Now that we've stopped using qcore entirely, drop it from the incore
> dquot.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks ok
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   fs/xfs/scrub/quota.c |    4 ----
>   fs/xfs/xfs_dquot.c   |   29 +++++++++--------------------
>   fs/xfs/xfs_dquot.h   |    1 -
>   3 files changed, 9 insertions(+), 25 deletions(-)
> 
> 
> diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
> index 2fc2625feca0..f4aad5b00188 100644
> --- a/fs/xfs/scrub/quota.c
> +++ b/fs/xfs/scrub/quota.c
> @@ -79,7 +79,6 @@ xchk_quota_item(
>   	struct xchk_quota_info	*sqi = priv;
>   	struct xfs_scrub	*sc = sqi->sc;
>   	struct xfs_mount	*mp = sc->mp;
> -	struct xfs_disk_dquot	*d = &dq->q_core;
>   	struct xfs_quotainfo	*qi = mp->m_quotainfo;
>   	xfs_fileoff_t		offset;
>   	xfs_ino_t		fs_icount;
> @@ -98,9 +97,6 @@ xchk_quota_item(
>   
>   	sqi->last_id = dq->q_id;
>   
> -	if (d->d_pad0 != cpu_to_be32(0) || d->d_pad != cpu_to_be16(0))
> -		xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, offset);
> -
>   	/*
>   	 * Warn if the hard limits are larger than the fs.
>   	 * Administrators can do this, though in production this seems
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 7434ee57ec43..2d6b50760962 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -529,7 +529,6 @@ xfs_dquot_from_disk(
>   	}
>   
>   	/* copy everything from disk dquot to the incore dquot */
> -	memcpy(&dqp->q_core, ddqp, sizeof(struct xfs_disk_dquot));
>   	dqp->q_blk.hardlimit = be64_to_cpu(ddqp->d_blk_hardlimit);
>   	dqp->q_blk.softlimit = be64_to_cpu(ddqp->d_blk_softlimit);
>   	dqp->q_ino.hardlimit = be64_to_cpu(ddqp->d_ino_hardlimit);
> @@ -568,8 +567,13 @@ xfs_dquot_to_disk(
>   	struct xfs_disk_dquot	*ddqp,
>   	struct xfs_dquot	*dqp)
>   {
> -	memcpy(ddqp, &dqp->q_core, sizeof(struct xfs_disk_dquot));
> +	ddqp->d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
> +	ddqp->d_version = XFS_DQUOT_VERSION;
>   	ddqp->d_flags = dqp->dq_flags & XFS_DQ_ONDISK;
> +	ddqp->d_id = cpu_to_be32(dqp->q_id);
> +	ddqp->d_pad0 = 0;
> +	ddqp->d_pad = 0;
> +
>   	ddqp->d_blk_hardlimit = cpu_to_be64(dqp->q_blk.hardlimit);
>   	ddqp->d_blk_softlimit = cpu_to_be64(dqp->q_blk.softlimit);
>   	ddqp->d_ino_hardlimit = cpu_to_be64(dqp->q_ino.hardlimit);
> @@ -1180,7 +1184,6 @@ xfs_qm_dqflush(
>   	struct xfs_log_item	*lip = &dqp->q_logitem.qli_item;
>   	struct xfs_buf		*bp;
>   	struct xfs_dqblk	*dqb;
> -	struct xfs_disk_dquot	*ddqp;
>   	xfs_failaddr_t		fa;
>   	int			error;
>   
> @@ -1204,22 +1207,6 @@ xfs_qm_dqflush(
>   	if (error)
>   		goto out_abort;
>   
> -	/*
> -	 * Calculate the location of the dquot inside the buffer.
> -	 */
> -	dqb = bp->b_addr + dqp->q_bufoffset;
> -	ddqp = &dqb->dd_diskdq;
> -
> -	/* sanity check the in-core structure before we flush */
> -	fa = xfs_dquot_verify(mp, &dqp->q_core, dqp->q_id, 0);
> -	if (fa) {
> -		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
> -				dqp->q_id, fa);
> -		xfs_buf_relse(bp);
> -		error = -EFSCORRUPTED;
> -		goto out_abort;
> -	}
> -
>   	fa = xfs_qm_dqflush_check(dqp);
>   	if (fa) {
>   		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
> @@ -1229,7 +1216,9 @@ xfs_qm_dqflush(
>   		goto out_abort;
>   	}
>   
> -	xfs_dquot_to_disk(ddqp, dqp);
> +	/* Flush the incore dquot to the ondisk buffer. */
> +	dqb = bp->b_addr + dqp->q_bufoffset;
> +	xfs_dquot_to_disk(&dqb->dd_diskdq, dqp);
>   
>   	/*
>   	 * Clear the dirty field and remember the flush lsn for later use.
> diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> index 414bae537b1d..62b0fc6e0133 100644
> --- a/fs/xfs/xfs_dquot.h
> +++ b/fs/xfs/xfs_dquot.h
> @@ -71,7 +71,6 @@ struct xfs_dquot {
>   	struct xfs_dquot_res	q_ino;	/* inodes */
>   	struct xfs_dquot_res	q_rtb;	/* realtime blocks */
>   
> -	struct xfs_disk_dquot	q_core;
>   	struct xfs_dq_logitem	q_logitem;
>   
>   	xfs_qcnt_t		q_prealloc_lo_wmark;
>

Patch
diff mbox series

diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
index 2fc2625feca0..f4aad5b00188 100644
--- a/fs/xfs/scrub/quota.c
+++ b/fs/xfs/scrub/quota.c
@@ -79,7 +79,6 @@  xchk_quota_item(
 	struct xchk_quota_info	*sqi = priv;
 	struct xfs_scrub	*sc = sqi->sc;
 	struct xfs_mount	*mp = sc->mp;
-	struct xfs_disk_dquot	*d = &dq->q_core;
 	struct xfs_quotainfo	*qi = mp->m_quotainfo;
 	xfs_fileoff_t		offset;
 	xfs_ino_t		fs_icount;
@@ -98,9 +97,6 @@  xchk_quota_item(
 
 	sqi->last_id = dq->q_id;
 
-	if (d->d_pad0 != cpu_to_be32(0) || d->d_pad != cpu_to_be16(0))
-		xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, offset);
-
 	/*
 	 * Warn if the hard limits are larger than the fs.
 	 * Administrators can do this, though in production this seems
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 7434ee57ec43..2d6b50760962 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -529,7 +529,6 @@  xfs_dquot_from_disk(
 	}
 
 	/* copy everything from disk dquot to the incore dquot */
-	memcpy(&dqp->q_core, ddqp, sizeof(struct xfs_disk_dquot));
 	dqp->q_blk.hardlimit = be64_to_cpu(ddqp->d_blk_hardlimit);
 	dqp->q_blk.softlimit = be64_to_cpu(ddqp->d_blk_softlimit);
 	dqp->q_ino.hardlimit = be64_to_cpu(ddqp->d_ino_hardlimit);
@@ -568,8 +567,13 @@  xfs_dquot_to_disk(
 	struct xfs_disk_dquot	*ddqp,
 	struct xfs_dquot	*dqp)
 {
-	memcpy(ddqp, &dqp->q_core, sizeof(struct xfs_disk_dquot));
+	ddqp->d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
+	ddqp->d_version = XFS_DQUOT_VERSION;
 	ddqp->d_flags = dqp->dq_flags & XFS_DQ_ONDISK;
+	ddqp->d_id = cpu_to_be32(dqp->q_id);
+	ddqp->d_pad0 = 0;
+	ddqp->d_pad = 0;
+
 	ddqp->d_blk_hardlimit = cpu_to_be64(dqp->q_blk.hardlimit);
 	ddqp->d_blk_softlimit = cpu_to_be64(dqp->q_blk.softlimit);
 	ddqp->d_ino_hardlimit = cpu_to_be64(dqp->q_ino.hardlimit);
@@ -1180,7 +1184,6 @@  xfs_qm_dqflush(
 	struct xfs_log_item	*lip = &dqp->q_logitem.qli_item;
 	struct xfs_buf		*bp;
 	struct xfs_dqblk	*dqb;
-	struct xfs_disk_dquot	*ddqp;
 	xfs_failaddr_t		fa;
 	int			error;
 
@@ -1204,22 +1207,6 @@  xfs_qm_dqflush(
 	if (error)
 		goto out_abort;
 
-	/*
-	 * Calculate the location of the dquot inside the buffer.
-	 */
-	dqb = bp->b_addr + dqp->q_bufoffset;
-	ddqp = &dqb->dd_diskdq;
-
-	/* sanity check the in-core structure before we flush */
-	fa = xfs_dquot_verify(mp, &dqp->q_core, dqp->q_id, 0);
-	if (fa) {
-		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
-				dqp->q_id, fa);
-		xfs_buf_relse(bp);
-		error = -EFSCORRUPTED;
-		goto out_abort;
-	}
-
 	fa = xfs_qm_dqflush_check(dqp);
 	if (fa) {
 		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
@@ -1229,7 +1216,9 @@  xfs_qm_dqflush(
 		goto out_abort;
 	}
 
-	xfs_dquot_to_disk(ddqp, dqp);
+	/* Flush the incore dquot to the ondisk buffer. */
+	dqb = bp->b_addr + dqp->q_bufoffset;
+	xfs_dquot_to_disk(&dqb->dd_diskdq, dqp);
 
 	/*
 	 * Clear the dirty field and remember the flush lsn for later use.
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 414bae537b1d..62b0fc6e0133 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -71,7 +71,6 @@  struct xfs_dquot {
 	struct xfs_dquot_res	q_ino;	/* inodes */
 	struct xfs_dquot_res	q_rtb;	/* realtime blocks */
 
-	struct xfs_disk_dquot	q_core;
 	struct xfs_dq_logitem	q_logitem;
 
 	xfs_qcnt_t		q_prealloc_lo_wmark;