On Mon, Jun 03, 2013 at 02:18:33PM -0400, Brian Foster wrote:
> On 06/03/2013 01:28 AM, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > Calculating dquot CRCs when the backing buffer is written back just
> > doesn't work reliably. There are several places which manipulate
> > dquots directly in the buffers, and they don't calculate CRCs
> > appropriately, nor do they always set the buffer up to calculate
> > CRCs appropriately.
> >
> > Firstly, if we log a dquot buffer (e.g. during allocation) it gets
> > logged without valid CRC, and so on recovery we end up with a dquot
> > that is not valid.
> >
> > Secondly, if we recover/repair a dquot, we don't have a verifier
> > attached to the buffer and hence CRCs arenot calculate don the way
> > down to disk.
> >
> > Thirdly, calculating the CRC after we've changed the contents means
> > that if we re-read the dquot from the buffer, we cannot verify the
> > contents of the dquot are valid, as the CRC is invalid.
> >
> > So, to avoid all the dquot CRC errors that are being detected by the
> > read verifier, change to using the same model as for inodes. that
> > is, dquot CRCs are calculated and written to the backing buffer at
> > the time the dquot is flushed to the backing buffer. If we modify
> > the dquuot directly in the backing buffer, calculate the CRC
> > immediately after the modification is complete. Hence the dquot in
> > the on-disk buffer should always have a valid CRC.
> >
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
>
> > ---
> > fs/xfs/xfs_dquot.c | 37 ++++++++++++++++---------------------
> > fs/xfs/xfs_log_recover.c | 10 ++++++++++
> > fs/xfs/xfs_qm.c | 40 ++++++++++++++++++++++++++++++----------
> > fs/xfs/xfs_quota.h | 2 ++
> > 4 files changed, 58 insertions(+), 31 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index a41f8bf..044e97a 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> > @@ -249,8 +249,11 @@ xfs_qm_init_dquot_blk(
> > d->dd_diskdq.d_version = XFS_DQUOT_VERSION;
> > d->dd_diskdq.d_id = cpu_to_be32(curid);
> > d->dd_diskdq.d_flags = type;
> > - if (xfs_sb_version_hascrc(&mp->m_sb))
> > + if (xfs_sb_version_hascrc(&mp->m_sb)) {
> > uuid_copy(&d->dd_uuid, &mp->m_sb.sb_uuid);
> > + xfs_update_cksum((char *)d, sizeof(struct xfs_dqblk),
> > + XFS_DQUOT_CRC_OFF);
> > + }
> > }
> >
> > xfs_trans_dquot_buf(tp, bp,
Huh. Looks like this buffer will not be recovered. See
xlog_recover_do_reg_buffer... I think he'll be skipped in recovery. Goto next.
That's not an issue with this patch though.
Reviewed-by: Ben Myers <bpm@xxxxxxx>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
