svn commit: r312600 - head/sys/kern

[Bruce Evans]

On Sat, 21 Jan 2017, Konstantin Belousov wrote:

> On Sat, Jan 21, 2017 at 06:38:17PM +0000, Mateusz Guzik wrote:
>> ...
>> 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.

Many style bugs.

>> 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));

This was correctly formatted except for the long line.

>> -		/*
>> -		 * 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));

This is even more grossly misreformatted than the other KASSERT().  The new
style bugs are:
- random first contuation indent (it happens to be 2 tabs)
- long line longer than before (further messed up by the indentation)
- random second contuation indent (it actually lines up with the first,
   except it uses gnu-style lining up parentheses instead of KNF)

>> +	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.

Later commits further unimproved style by adding __predict_ugly() and
long lines from blind substitution of that.   __predict_ugly() is almost
as useful as 'register', but more invasive.

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

Bruce