Re: kern/40525: panic: ffs_valloc: dup alloc

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

The following reply was made to PR kern/40525; it has been noted by GNATS.

From: David Holland <dholland-bugs%netbsd.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/40525: panic: ffs_valloc: dup alloc
Date: Sun, 22 Feb 2009 05:50:43 +0000

 On Sun, Feb 08, 2009 at 10:45:29AM +0100, Juergen Hannken-Illjes wrote:
  > > But it sounds like you agree that my patch fixes *a* bug, even if it's
  > > not the same bug reported on current-users. Right?
  > 
  > Please get a second OK then.
 
 Discussion at some length with pooka and ad has produced the following
 conclusions:
 
   1. It's a real problem, and the proposed patch does fix it.
   2. BC_INVAL has to be removed from the corresponding brelse call
      below "fail:" too.
   3. We think the buffer cache will DTRT if the call to getblk returns
      a fresh buffer with no real data and we call brelse(bp, 0) on it
      (that is, don't explicitly invalidate it). I was worried about
      this, and still am a bit, and will look into it further later.
   4. We're Concerned(TM) about other uses of BC_INVAL in FS code.
      There are quite a few that look suspect.
 
 ad said this afternoon he was going to commit the patch, but hasn't
 yet; I will if he doesn't in the next day or two.
 
 This is the full patch.
 
 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        22 Feb 2009 05:49:33 -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;
        }
        /*
 @@ -1396,7 +1396,7 @@ gotit:
        if (bp != NULL)
                brelse(bp, 0);
        if (ibp != NULL)
 -              brelse(ibp, BC_INVAL);
 +              brelse(ibp, 0);
        mutex_enter(&ump->um_lock);
        return (0);
  }
 
 
 -- 
 David A. Holland
 dholland%netbsd.org@localhost