xfs
[Top] [All Lists]

Re: [PATCH 5/9] repair: detect CRC errors in AG headers

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH 5/9] repair: detect CRC errors in AG headers
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 2 May 2014 09:27:20 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140429140619.GC59046@xxxxxxxxxxxxxxx>
References: <1398719099-19194-1-git-send-email-david@xxxxxxxxxxxxx> <1398719099-19194-6-git-send-email-david@xxxxxxxxxxxxx> <20140429140619.GC59046@xxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Apr 29, 2014 at 10:06:19AM -0400, Brian Foster wrote:
> On Tue, Apr 29, 2014 at 07:04:55AM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > 
> > repair doesn't currently detect verifier errors in AG header
> > blocks - apart from the primary superblock they are not detected.
> > They are, fortunately, corrected in the important cases (AGF, AGI
> > and AGFL) because these structures are rebuilt in phase 5, but if
> > you run xfs_repair in checking mode it won't report them as bad.
> > 
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
.....
> > @@ -1312,21 +1302,34 @@ scan_ag(
> >     validate_agi(agi, agno, agcnts);
> >  
> >     ASSERT(agi_dirty == 0 || (agi_dirty && !no_modify));
> > +   ASSERT(agf_dirty == 0 || (agf_dirty && !no_modify));
> > +   ASSERT(sb_dirty == 0 || (sb_dirty && !no_modify));
> > +
> > +   /*
> > +    * Only pay attention to CRC/verifier errors if we can correct them.
> > +    * While there, ensure that we corrected a corruption error if the
> > +    * verifier detected one.
> > +    */
> > +   if (!no_modify) {
> > +           ASSERT(agi_dirty || agibuf->b_error != EFSCORRUPTED);
> > +           ASSERT(agf_dirty || agfbuf->b_error != EFSCORRUPTED);
> > +           ASSERT(sb_dirty || sbbuf->b_error != EFSCORRUPTED);
> > +
> > +           agi_dirty += (agibuf->b_error == EFSBADCRC);
> > +           agf_dirty += (agfbuf->b_error == EFSBADCRC);
> > +           sb_dirty += (sbbuf->b_error == EFSBADCRC);
> > +   }
> 
> So we'll detect and correct the CRC error in normal mode, but no longer
> issue the preceding warnings ("would reset bad ...") for CRC errors in
> no_modify mode. Is that desired?

It will still throw a bad CRC error, so the user is still told that
there is a problem.

> I ask because it looks like a departure from previous versions.
> Otherwise, the code looks fine to me.

Slightly, but I don't think it makes much difference...

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [PATCH 5/9] repair: detect CRC errors in AG headers, Dave Chinner <=