panic in vget()

[Kostik Belousov]

On Fri, Apr 16, 2010 at 01:23:17PM -0700, Matthew Fleming wrote:
> 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)

Both the analysis and the patch look good.

Did you considered locking the vnode even when no locking flags were
given, as is done for VI_OWEINACT handling ? Your solution is better,
esp. for old lockmgr, but acquiring vnode lock might be safer.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20100416/4cc2f7d2/attachment.pgp