kernel - Fix rare vref() assertion
authorMatthew Dillon <dillon@apollo.backplane.com>
Sat, 8 Dec 2018 22:34:51 +0000 (14:34 -0800)
committerMatthew Dillon <dillon@apollo.backplane.com>
Sat, 8 Dec 2018 22:34:51 +0000 (14:34 -0800)
* The VREF_TERMINATE flag gets cleared when a vnode is reactivated.
  However, concurrent LK_SHARED locks on vnodes can race the v_state
  test.  Thus the code cannot assume that VREF_TERMINATE has been cleared
  when v_state is VS_ACTIVE.

  To avoid the race, we simply unconditionally clear VREF_TERMINATE on
  a successful vget().

* Could be reproduced by running blogbench and synth together, both of
  which generate extreme filesystem-intensive loads.

sys/kern/vfs_lock.c

index 607595c..85567a9 100644 (file)
@@ -560,6 +560,12 @@ vget(struct vnode *vp, int flags)
                 * changing.  Since the vnode is not in a VRECLAIMED state,
                 * we can safely clear VINACTIVE.
                 *
+                * It is possible for a shared lock to cause a race with
+                * another thread that is also in the process of clearing
+                * VREF_TERMINATE, meaning that we might return with it still
+                * set and then assert in a later vref().  The solution is to
+                * unconditionally clear VREF_TERMINATE here as well.
+                *
                 * NOTE! Multiple threads may clear VINACTIVE if this is
                 *       shared lock.  This race is allowed.
                 */
@@ -568,6 +574,7 @@ vget(struct vnode *vp, int flags)
                if (vp->v_act > VACT_MAX)       /* SMP race ok */
                        vp->v_act = VACT_MAX;
                error = 0;
+               atomic_clear_int(&vp->v_refcnt, VREF_TERMINATE);
        } else {
                /*
                 * If the vnode is not VS_ACTIVE it must be reactivated
@@ -585,6 +592,12 @@ vget(struct vnode *vp, int flags)
                 * the refcnt is non-zero and the vnode has not been
                 * reclaimed.  This also means that the transitions do
                 * not affect cachedvnodes.
+                *
+                * It is possible for a shared lock to cause a race with
+                * another thread that is also in the process of clearing
+                * VREF_TERMINATE, meaning that we might return with it still
+                * set and then assert in a later vref().  The solution is to
+                * unconditionally clear VREF_TERMINATE here as well.
                 */
                _vclrflags(vp, VINACTIVE);
                vp->v_act += VACT_INC;
@@ -606,7 +619,8 @@ vget(struct vnode *vp, int flags)
                        spin_unlock(&vp->v_spin);
                        break;
                case VS_ACTIVE:
-                       atomic_clear_int(&vp->v_refcnt, VREF_FINALIZE);
+                       atomic_clear_int(&vp->v_refcnt, VREF_FINALIZE |
+                                                       VREF_TERMINATE);
                        spin_unlock(&vp->v_spin);
                        break;
                case VS_DYING: