xfs
[Top] [All Lists]

Re: review: fsblock zero - don't panic

To: Nathan Scott <nathans@xxxxxxx>
Subject: Re: review: fsblock zero - don't panic
From: David Chinner <dgc@xxxxxxx>
Date: Fri, 11 Aug 2006 13:26:26 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20060810155851.C2591606@xxxxxxxxxxxxxxxxxxxxxxxx>
References: <20060810155851.C2591606@xxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.4.2.1i
On Thu, Aug 10, 2006 at 03:58:51PM +1000, Nathan Scott wrote:
> As part of attempting to understand what happened in a corruption
> problem awhile back, and generally be a bit more defensive of our
> precious primary superblock, some code was added to XFS to detect
> (and panic) on any inode extents that start at block zero.
> 
> This has happened once or twice now, and when it does, we panic the
> kernel.  This is not at all nice, as it means we take out the whole
> system due to ondisk corruption.  This patch makes that code issue
> a warning now, and fail whatever operation was in progress.

Looks OK.

FWIW:

> -     if ( !(io->io_flags & XFS_IOCORE_RT)  && !ret_imap->br_startblock) {
.....
> +     if (unlikely(
> +         !(io->io_flags & XFS_IOCORE_RT) && !ret_imap->br_startblock)) {

If you are hinting this branch to be unlikely, then we should also
check the start block first before checking the io_flags.  We only
need to check the io_flags if we are actually accessing block zero.
i.e.

+       if (unlikely(
+           !ret_imap->br_startblock && !(io->io_flags & XFS_IOCORE_RT))) {

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group


<Prev in Thread] Current Thread [Next in Thread>