| Message ID | 159353178119.2864738.14352743945962585449.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: remove xfs_disk_quot from incore dquot | expand |
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; > >
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>
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.
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
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; >
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;