On Fri, Dec 15, 2017 at 02:09:10PM +1100, Dave Chinner wrote:
> 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...
Ok, but I'll do this as a separate patch.
--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
