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
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to