Re: review: fsblock zero - don't panic

[David Chinner <dgc@sgi.com>]

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