Re: kern/38135 (vfs_busy/vfs_trybusy confusion)

[yamt%mwd.biglobe.ne.jp@localhost (YAMAMOTO Takashi)]

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

From: yamt%mwd.biglobe.ne.jp@localhost (YAMAMOTO Takashi)
To: ad%NetBSD.org@localhost
Cc: gnats-bugs%NetBSD.org@localhost
Subject: Re: kern/38135 (vfs_busy/vfs_trybusy confusion)
Date: Thu,  1 May 2008 21:16:28 +0900 (JST)

 > On Thu, May 01, 2008 at 11:07:25AM +0900, YAMAMOTO Takashi wrote:
 > 
 > > can you take a look at PR/38141 as well?
 > > 
 > > as vfs_trybusy now does rw_enter rather than rw_tryenter,
 > > my box deadlocks frequently due to the bug described in the PR.
 > 
 > Do you know of an easy way to trigger the problem?
 > 
 > Andrew
 
 my test case is ./build.sh -j128 on a system with 4 cpus.
 /tmp is tmpfs and everything else is nfs.
 
 the following patch is to detect the recursive read-lock condition.
 with this, simply mounting a tmpfs is probably enough to trigger a panic.
 
 YAMAMOTO Takashi
 
 
 Index: subr_lockdebug.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/subr_lockdebug.c,v
 retrieving revision 1.31
 diff -u -p -r1.31 subr_lockdebug.c
 --- subr_lockdebug.c   28 Apr 2008 20:24:04 -0000      1.31
 +++ subr_lockdebug.c   30 Apr 2008 12:58:06 -0000
 @@ -476,7 +476,7 @@ lockdebug_wantlock(volatile void *lock, 
        if ((ld = lockdebug_lookup(lock, &lk)) == NULL)
                return;
  
 -      if ((ld->ld_flags & LD_LOCKED) != 0) {
 +      if ((ld->ld_flags & LD_LOCKED) != 0 || ld->ld_shares != 0) {
                if ((ld->ld_flags & LD_SLEEPER) != 0) {
                        if (ld->ld_lwp == l)
                                recurse = true;
 @@ -538,8 +538,6 @@ lockdebug_locked(volatile void *lock, ui
  
                ld->ld_flags |= LD_LOCKED;
                ld->ld_locked = where;
 -              ld->ld_cpu = (uint16_t)cpu_number();
 -              ld->ld_lwp = l;
                ld->ld_exwant--;
  
                if ((ld->ld_flags & LD_SLEEPER) != 0) {
 @@ -550,6 +548,8 @@ lockdebug_locked(volatile void *lock, ui
                        TAILQ_INSERT_TAIL(&ld_spinners, ld, ld_chain);
                }
        }
 +      ld->ld_cpu = (uint16_t)cpu_number();
 +      ld->ld_lwp = l;
  
        lockdebug_unlock(lk);
  }
 @@ -585,6 +585,10 @@ lockdebug_unlocked(volatile void *lock, 
                }
                l->l_shlocks--;
                ld->ld_shares--;
 +              if (ld->ld_lwp == l)
 +                      ld->ld_lwp = NULL;
 +              if (ld->ld_cpu == (uint16_t)cpu_number())
 +                      ld->ld_cpu = (uint16_t)-1;
        } else {
                if ((ld->ld_flags & LD_LOCKED) == 0) {
                        lockdebug_abort1(ld, lk, __func__, "not locked",