svn commit: r312600 - head/sys/kern

[Konstantin Belousov]

On Sat, Jan 21, 2017 at 06:38:17PM +0000, Mateusz Guzik wrote:
> Author: mjg
> Date: Sat Jan 21 18:38:16 2017
> New Revision: 312600
> URL: https://svnweb.freebsd.org/changeset/base/312600
> 
> Log:
>   vfs: refactor _vn_lock
>   
>   Stop testing for LK_RETRY and error multiple times. Also postpone the
>   VI_DOOMED until after LK_RETRY was seen as it reads from the vnode.
>   
>   No functional changes.
> 
> Modified:
>   head/sys/kern/vfs_vnops.c
> 
> Modified: head/sys/kern/vfs_vnops.c
> ==============================================================================
> --- head/sys/kern/vfs_vnops.c	Sat Jan 21 17:39:10 2017	(r312599)
> +++ head/sys/kern/vfs_vnops.c	Sat Jan 21 18:38:16 2017	(r312600)
> @@ -1539,27 +1539,24 @@ _vn_lock(struct vnode *vp, int flags, ch
>  
>  	VNASSERT((flags & LK_TYPE_MASK) != 0, vp,
>  	    ("vn_lock called with no locktype."));
> -	do {
>  #ifdef DEBUG_VFS_LOCKS
> -		KASSERT(vp->v_holdcnt != 0,
> -		    ("vn_lock %p: zero hold count", vp));
> +	KASSERT(vp->v_holdcnt != 0,
> +			("vn_lock %p: zero hold count", vp));
This line also has wrong inde seems to not be fixed by later commit.

>  #endif
> -		error = VOP_LOCK1(vp, flags, file, line);
> -		flags &= ~LK_INTERLOCK;	/* Interlock is always dropped. */
> -		KASSERT((flags & LK_RETRY) == 0 || error == 0,
> -		    ("LK_RETRY set with incompatible flags (0x%x) or an error occurred (%d)",
> -		    flags, error));
> -		/*
> -		 * Callers specify LK_RETRY if they wish to get dead vnodes.
> -		 * If RETRY is not set, we return ENOENT instead.
> -		 */
> -		if (error == 0 && vp->v_iflag & VI_DOOMED &&
> -		    (flags & LK_RETRY) == 0) {
> +retry:
> +	error = VOP_LOCK1(vp, flags, file, line);
> +	flags &= ~LK_INTERLOCK;	/* Interlock is always dropped. */
> +	KASSERT((flags & LK_RETRY) == 0 || error == 0,
> +			("LK_RETRY set with incompatible flags (0x%x) or an error occurred (%d)",
> +			 flags, error));
> +	if (flags & LK_RETRY) {
Stylish test is
	if ((flags & LK_RETRY) != 0) {
> +		if ((error != 0))
Too many ().

> +			goto retry;
> +		if ((vp->v_iflag & VI_DOOMED)) {
Too many braces again, or missed != 0.
>  			VOP_UNLOCK(vp, 0);
>  			error = ENOENT;
> -			break;
Also, this does the functional change, it seems to completely break LK_RERY
logic.  If LK_RETRY is specified, VI_DOOMED must not result in ENOENT.

>  		}
> -	} while (flags & LK_RETRY && error != 0);
> +	}
>  	return (error);
>  }
>