On Tue, Jun 17, 2008 at 11:24:17AM +1000, Lachlan McIlroy wrote:
> Dave Chinner wrote:
>> On Sunday 15 June 2008 11:41 pm, Lachlan McIlroy wrote:
>>>> ASSERT? How about a WANT_CORRUPTED_GOTO()?
>>> I was just being consistent with the rest of the code. If you think this
>>> ASSERT should be changed then what about all of them?
>> Well, the ASSERT means silent failure on a production system.
>> Given that failure here indicates a corrupt btree, then we really
>> should be treating it as such. i.e. shut down the filesystem. This
>> might have saved us a whole heap of trouble tracking down these
>> problems as a shutdown here would have pointed us right at the
>> source of the problem...
> Dave, what I was asking is what about the rest of the ASSERTs in
> this file - should we change them too? There's a lot of them.
> After this change they are all equally likely to trigger so if
> it makes sense to change one then the same argument applies to
> all of them.
In the past we've only changed the bits of the code that have been
needed for debugging problems. e.g. there's WANT_CORRUPTED
throughout the alloc code, but only some bits of the bmbt code and
almost none in the inobt. Really we should be consistent with
our catching and handling of errors.
IOWs, I'd say we probably should change them all, but it's going
to touch lots of code. For the btree code, I'd say it should be done
with the factoring (get them all in one go), but xfs_bmap.c code is
separate and could be done separately.