On Tue, Dec 19, 2017 at 05:22:05PM +1100, Dave Chinner wrote:
> On Wed, Dec 13, 2017 at 03:59:25PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Expose all metadata structure buffer verifier functions via buf_ops.
> > These will be used by the online scrub mechanism to look for problems
> > with buffers that are already sitting around in memory.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> ....
> > @@ -2468,7 +2478,7 @@ xfs_agf_read_verify(
> > .verify_write = xfs_attr3_leaf_write_verify,
> > + .verify_struct = xfs_attr3_leaf_verify,
> > };
> >
> > int
> > diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> > index d4d2902..1be995b 100644
> > --- a/fs/xfs/libxfs/xfs_attr_remote.c
> > +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> > @@ -204,10 +204,42 @@ xfs_attr3_rmt_write_verify(
> > ASSERT(len == 0);
> > }
> >
> > +static xfs_failaddr_t
> > +xfs_attr3_rmt_verify_struct(
> > + struct xfs_buf *bp)
> > +{
> > + struct xfs_mount *mp = bp->b_target->bt_mount;
> > + char *ptr;
> > + void *failed_at;
> > + int len;
> > + xfs_daddr_t bno;
> > + int blksize = mp->m_attr_geo->blksize;
> > +
> > + /* no verification of non-crc buffers */
> > + if (!xfs_sb_version_hascrc(&mp->m_sb))
> > + return NULL;
> > +
> > + ptr = bp->b_addr;
> > + bno = bp->b_bn;
> > + len = BBTOB(bp->b_length);
> > + ASSERT(len >= blksize);
> > +
> > + while (len > 0) {
> > + if ((failed_at = xfs_attr3_rmt_verify(mp, ptr, blksize, bno)))
> > + return failed_at;
> > + len -= blksize;
> > + ptr += blksize;
> > + bno += BTOBB(blksize);
> > + }
> > +
> > + return NULL;
> > +}
>
> I'd much prefer to see this combined with
> xfs_attr3_rmt_read_verify() rather than having another copy of this
> iteration code. They really only vary by whether the CRC is checked
> in the loop....
>
> ....
>
> > +static xfs_failaddr_t
> > +xfs_dquot_buf_verify_struct(
> > + struct xfs_buf *bp)
> > +{
> > + struct xfs_mount *mp = bp->b_target->bt_mount;
> > +
> > + if (!xfs_dquot_buf_verify(mp, bp, 0))
> > + return __this_address;
> > + return NULL;
> > +}
>
> I can't remember what happened exactly with dquot buffers earlire in
> the patchset, but why isn't it returning a failaddr like all the
> other structure verifiers?
The dquot verifiers are sufficiently different from everything else
(verbose error reporting, some ability to zap garbage data) that I was
going to send that as a separate cleanup series.
I /think/ the solution is to disentangle xfs_dqcheck into a separate
check routine that returns xfs_failaddr_t like everything else (at a
cost of the removal of all the xfs_alert calls) so that the error
reports become "xfs: quota buffer XXX error at xfs_dqcheck+0x74" like
everything else. The caller becomes directly responsible for printing a
warning message (instead of XFS_QMOPT_DOWARN).
Then, the quota repair piece becomes a separate function which the
XFS_QMOPT_DQREPAIR callers can call directly.
Sound good?
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
