public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Lachlan McIlroy <lachlan@sgi.com>
Cc: Dave Chinner <dchinner@agami.com>, xfs-dev <xfs-dev@sgi.com>,
	xfs-oss <xfs@oss.sgi.com>
Subject: Re: [PATCH] Always reset btree cursor after an insert
Date: Tue, 17 Jun 2008 15:40:17 +1000	[thread overview]
Message-ID: <20080617054017.GO3700@disturbed> (raw)
In-Reply-To: <48571241.20806@sgi.com>

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.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

      reply	other threads:[~2008-06-17  5:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-16  2:21 [PATCH] Always reset btree cursor after an insert Lachlan McIlroy
2008-06-16  5:01 ` Dave Chinner
2008-06-16  6:41   ` Lachlan McIlroy
2008-06-16 17:14     ` Dave Chinner
2008-06-17  1:24       ` Lachlan McIlroy
2008-06-17  5:40         ` Dave Chinner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080617054017.GO3700@disturbed \
    --to=david@fromorbit.com \
    --cc=dchinner@agami.com \
    --cc=lachlan@sgi.com \
    --cc=xfs-dev@sgi.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox