Re: panic: ffs_valloc: dup alloc

[David Holland <dholland-current%netbsd.org@localhost>]

Reposting by request to add tech-kern, sorry about the noise...

   ------

On Mon, Jan 19, 2009 at 10:09:57AM -0500, Steven M. Bellovin wrote:
 > > > While I'm not seeing exactly that problem, I do see problems that
 > > > result in duplicate blocks after a crash.  I think it's timing- and
 > > > disk-layout dependent, but I do think there's a serious file system
 > > > problem in -current.  I'm seriously contemplating wiping my disk
 > > > and installing a system from a couple of months ago.
 > > 
 > > Do you have any ideas how one might set about replicating it? (What
 > > mount options, partition size, etc. are you using?)
 >
 > I posted the other day -- if I build devel/apr, even running
 > single-user (but only with SMP enabled), it always crashes the system,
 > generally with duplicate inodes under apr/work.

ok, I think it's a race condition introduced in -r1.112 of
ffs_alloc.c, with the following fix:

Index: ffs_alloc.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/ffs/ffs_alloc.c,v
retrieving revision 1.120
diff -u -p -r1.120 ffs_alloc.c
--- ffs_alloc.c 11 Jan 2009 02:45:56 -0000      1.120
+++ ffs_alloc.c 26 Jan 2009 03:28:52 -0000
@@ -1284,7 +1284,7 @@ retry:
        if (ibp != NULL &&
            initediblk != ufs_rw32(cgp->cg_initediblk, needswap)) {
                /* Another thread allocated more inodes so we retry the test. */
-               brelse(ibp, BC_INVAL);
+               brelse(ibp, 0);
                ibp = NULL;
        }
        /*


If we reach this point, this other thread must have already cleared
out the inodes in that block, and they had to have done it *after* we
released bp and *before* we fetched ipb. This means that ipb contains
valid data, that is, the zeroed-out inode block prepared by the other
thread. Marking it invalid will lose that data. Then when one of those
inodes is allocated (possibly by us, potentially by the other thread
or someone else) and FFS_VGET reads the inode, it will reread the
block from disk, get whatever crap was left lying there from
previously, find that its new inode's mode != 0, and panic as
observed.

I haven't tested this, only read the code, so it may be a load of
dingo's kidneys.

-- 
David A. Holland
dholland%netbsd.org@localhost