On Thu, Dec 14, 2017 at 04:04:09PM -0800, Darrick J. Wong wrote:
> On Fri, Dec 15, 2017 at 09:03:26AM +1100, Dave Chinner wrote:
> > On Wed, Dec 13, 2017 at 03:58:37PM -0800, Darrick J. Wong wrote:
> > > */
> > > void
> > > xfs_verifier_error(
> > > - struct xfs_buf *bp)
> > > + struct xfs_buf *bp,
> > > + xfs_failaddr_t fa)
> > > {
> > > struct xfs_mount *mp = bp->b_target->bt_mount;
> > >
> > > xfs_alert(mp, "Metadata %s detected at %pS, %s block 0x%llx",
> > > bp->b_error == -EFSBADCRC ? "CRC error" : "corruption",
> > > - __return_address, bp->b_ops->name, bp->b_bn);
> > > + fa ? fa : __return_address, bp->b_ops->name,
> > > + bp->b_bn);
> > >
> > > xfs_alert(mp, "Unmount and run xfs_repair");
> >
> > I'm also wondering if we should move the xfs_buf_ioerror() call
> > inside this function, too, rather than coding multiple calls in the
> > verifiers to set bp->b_error in each branch of the verifier that
> > has an error...
>
> What, something like:
>
> if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF)) {
> fa = __this_address;
> error = -EFSBADCRC;
> } else {
> fa = xfs_agfl_verify(bp);
> if (fa)
> error = -EFSCORRUPTED;
> }
>
> if (error)
> xfs_verifier_error(bp, fa, error);
>
Yeah, seems like a better approach because we don't have to
put the error on the buffer before calling xfs_verifier_error(). fa
tells us exactly where the error wasdetected, so we don't need
xfs_buf_ioerror() to tell us that...
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
