Re: kern/48411: repeatable SMP crashes in amd64-current

["J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>]

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

From: "J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: kern/48411: repeatable SMP crashes in amd64-current
Date: Thu, 28 Nov 2013 17:22:15 +0100

 --Apple-Mail=_05C7A77A-2DE2-44B8-A1D6-83D7F30CD1E5
 Content-Transfer-Encoding: 7bit
 Content-Type: text/plain;
        charset=us-ascii
 
 This deadlock comes from one thread being in layer_node_find(), holding
 the vnode locked and trying to vget(..., LK_NOWAIT) it.  Another thread
 (usually the vrele thread) is in vrelel() trying to get the vnode lock.
 
 Please try the attached diff where vrelel() marks the vnode as changing
 after it has aquired the vnode lock but before it runs VOP_INACTIVE().
 
 Entering layer_node_find() with a locked vnode has to go -- unfortunately
 this means VOP_LOOKUP() has to return an unlocked vnode.
 
 --
 J. Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig 
(Germany)
 
 
 --Apple-Mail=_05C7A77A-2DE2-44B8-A1D6-83D7F30CD1E5
 Content-Disposition: attachment;
        filename=vfs_vnode.c.diff
 Content-Type: application/octet-stream;
        name="vfs_vnode.c.diff"
 Content-Transfer-Encoding: 7bit
 
 Index: vfs_vnode.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/vfs_vnode.c,v
 retrieving revision 1.26
 diff -p -u -2 -r1.26 vfs_vnode.c
 --- vfs_vnode.c        23 Nov 2013 13:46:22 -0000      1.26
 +++ vfs_vnode.c        28 Nov 2013 16:03:24 -0000
 @@ -607,8 +607,4 @@ vrelel(vnode_t *vp, int flags)
  
        KASSERT((vp->v_iflag & VI_XLOCK) == 0);
 -      if ((flags & VRELEL_CHANGING_SET) == 0) {
 -              KASSERT((vp->v_iflag & VI_CHANGING) == 0);
 -              vp->v_iflag |= VI_CHANGING;
 -      }
  
  #ifdef DIAGNOSTIC
 @@ -655,11 +651,12 @@ vrelel(vnode_t *vp, int flags)
                        if (__predict_false(vtryrele(vp))) {
                                VOP_UNLOCK(vp);
 -                              KASSERT((vp->v_iflag & VI_CHANGING) != 0);
 -                              vp->v_iflag &= ~VI_CHANGING;
 -                              cv_broadcast(&vp->v_cv);
 +                              if ((flags & VRELEL_CHANGING_SET) != 0) {
 +                                      KASSERT((vp->v_iflag & VI_CHANGING) != 
0);
 +                                      vp->v_iflag &= ~VI_CHANGING;
 +                                      cv_broadcast(&vp->v_cv);
 +                              }
                                mutex_exit(vp->v_interlock);
                                return;
                        }
 -                      mutex_exit(vp->v_interlock);
                        defer = false;
                } else if ((vp->v_iflag & VI_LAYER) != 0) {
 @@ -676,8 +673,8 @@ vrelel(vnode_t *vp, int flags)
                        if (error != 0) {
                                defer = true;
 -                              mutex_enter(vp->v_interlock);
                        } else {
                                defer = false;
                        }
 +                      mutex_enter(vp->v_interlock);
                }
  
 @@ -688,6 +685,9 @@ vrelel(vnode_t *vp, int flags)
                         */
                        KASSERT(mutex_owned(vp->v_interlock));
 -                      KASSERT((vp->v_iflag & VI_CHANGING) != 0);
 -                      vp->v_iflag &= ~VI_CHANGING;
 +                      if ((flags & VRELEL_CHANGING_SET) != 0) {
 +                              KASSERT((vp->v_iflag & VI_CHANGING) != 0);
 +                              vp->v_iflag &= ~VI_CHANGING;
 +                              cv_broadcast(&vp->v_cv);
 +                      }
                        mutex_enter(&vrele_lock);
                        TAILQ_INSERT_TAIL(&vrele_list, vp, v_freelist);
 @@ -695,9 +695,14 @@ vrelel(vnode_t *vp, int flags)
                                cv_signal(&vrele_cv); 
                        mutex_exit(&vrele_lock);
 -                      cv_broadcast(&vp->v_cv);
                        mutex_exit(vp->v_interlock);
                        return;
                }
  
 +              if ((flags & VRELEL_CHANGING_SET) == 0) {
 +                      KASSERT((vp->v_iflag & VI_CHANGING) == 0);
 +                      vp->v_iflag |= VI_CHANGING;
 +              }
 +              mutex_exit(vp->v_interlock);
 +
                /*
                 * The vnode can gain another reference while being
 @@ -740,4 +745,9 @@ vrelel(vnode_t *vp, int flags)
                }
                KASSERT(vp->v_usecount > 0);
 +      } else { /* vnode was already clean */
 +              if ((flags & VRELEL_CHANGING_SET) == 0) {
 +                      KASSERT((vp->v_iflag & VI_CHANGING) == 0);
 +                      vp->v_iflag |= VI_CHANGING;
 +              }
        }
  
 
 --Apple-Mail=_05C7A77A-2DE2-44B8-A1D6-83D7F30CD1E5--