panic in vget()

Matthew Fleming matthew.fleming at isilon.com
Fri Apr 16 20:23:19 UTC 2010


I'm looking at this panic in vget() on stable/7:

	if (vp->v_iflag & VI_DOOMED && (flags & LK_RETRY) == 0)
		panic("vget: vn_lock failed to return ENOENT\n");

It seems to me that this is not a correct assertion, because if the
caller passed in no lock flags (i.e. just checking the vnode for
validity) then there is a window between the VI_UNLOCK() in _vn_lock(9)
and the subsequent VI_LOCK() in vget() where another thread could have
set VI_DOOMED.

This isn't a problem on CURRENT because the code has been changed to not
allow an empty lock flags.

I believe the following is a potential fix is:

 	vholdl(vp);
 	if ((error = vn_lock(vp, flags | LK_INTERLOCK, td)) != 0) {
 		vdrop(vp);
 		return (error);
 	}
 	VI_LOCK(vp);
+	/*
+	 * Deal with a timing window when the interlock is not held
+	 * and VI_DOOMED can be set, since we only have a holdcnt,
+	 * not a usecount.
+	 */
+	if (vp->v_iflag & VI_DOOMED && (flags & LK_RETRY) == 0) {
+		KASSERT((flags & LK_TYPE_MASK) == 0, ("Unexpected flags
%x", flags));
+		vdropl(vp);
+		return (ENOENT);
+	}
 	/* Upgrade our holdcnt to a usecount. */
 	v_upgrade_usecount(vp);
-	if (vp->v_iflag & VI_DOOMED && (flags & LK_RETRY) == 0)
-		panic("vget: vn_lock failed to return ENOENT\n");
 	if (oweinact) {
 		if (vp->v_iflag & VI_OWEINACT)
 			vinactive(vp, td);
 		VI_UNLOCK(vp);
 		if ((oldflags & LK_TYPE_MASK) == 0)

Thanks,
matthew


More information about the freebsd-stable mailing list