tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Vnode API change: VOP_LOOKUP

   Date: Mon, 3 Feb 2014 12:04:27 +0100
   From: J. Hannken-Illjes <>

   As announced some weeks ago here comes the API change to VOP_LOOKUP:

     Change vnode operation lookup to return the resulting vnode *vpp locked.
     Change cache_lookup() to return an unlocked vnode.

Sounds like the right thing to me.  I skimmed through the patch, and I
have one question and a few little nits.

   @@ -571,8 +571,14 @@ zfs_replay_remove(zfsvfs_t *zfsvfs, lr_r
           if (error != 0) {
                   goto fail;
   +       error = vn_lock(vp, LK_EXCLUSIVE);

Why LK_EXCLUSIVE and not (LK_EXCLUSIVE | LK_RETRY)?  There are a few
other cases of this too.  Are you mixing this up with a change to
allow vn_lock to fail everywhere so we can use that for transactions
and forced unmount?  While I think that will eventually be the right
thing, that should be in a separate patch so it can be systematically
done for all vn_locks.

   @@ -464,29 +464,9 @@ cache_lookup(struct vnode *dvp, const ch
   -       }
   +       KASSERT(error == 0);
This kassert is superfluous -- the `if (error) ... return' branch is
right above it.
   @@ -380,12 +380,12 @@ layer_lookup(void *v)
                   *ap->a_vpp = dvp;
           } else if (lvp != NULL) {
   -               /* Note: dvp, ldvp and lvp are all locked. */
   +               /* Note: dvp and ldvp are all locked. */


   @@ -519,8 +519,10 @@ kernfs_lookup(void *v)

                   error = kernfs_allocvp(dvp->v_mount, vpp, kt->kt_tag, kt, 0);
   +               if (error == 0)
   +                       VOP_UNLOCK(*vpp);
                   return (error);

I would rather write this (and a few other cases like it in the patch)

error = kernfs_allocvp(...);
if (error)
        return error;
return 0;
in order to consistently set error branches apart and make the success
case the main flow of the code.

Home | Main Index | Thread Index | Old Index