Jan Harkes <jaharkes_at_cs.cmu.edu> writes:

> I don't know much of the finer details of NetBSD's locking, but I
> noticed that the area where the parent/child lock ordering is fixed for
> '..' lookups, it also intentionally skips '.'.

The rule is

  lookup is entered with a locked vnode ("parent")

  if regular lookup, child vnode is obtained, referenced and locked

  if ., extra ref is added, but no extra lock (locks aren't recursive,
  so this wouldn't make sense).

  if .., then ref child, unlock parent, lock child, lock parent
  This is needed to avoid deadlock - locks always start at root and go
  down.

> Just wondering if it is useful to short-circuit such lookups earlier on
> before we even bother trying the coda_nc_lookup and venus_lookup. I
> assume that we would just return the current object, still locked with
> possible an additional vref. But returning early means we avoid the
> upcall to venus, don't have to add it to the coda namecache, and avoid
> the special case near the end.

If we can guarantee that venus always returns the self vnode for ., then
this would be an optimization and make sense.  Indeed just doing vref is
what happens, since presumably coda_nc_lookup and venus_lookup return
the cnode/vnode itself.

Right now I'm fighting correctness bugs so not thinking about this.
I added a comment that this would make sense to my local copy.

> It looks like the patch already got committed to CVS, so I guess my
> other worry is unfounded.

Commited doesn't imply correct....

> I was wondering if the locked parent directory vnode had a vref or
> not, dropping the lock without having a reference could lead to a hard
> to trigger, but nasty race where the vnode could possibly be dropped
> from memory while we're blocked trying to lock the vnode we want to
> return.

Yes, the caller of coda_lookup holds the reference (meaning has a
pointer, and there's a count in the v_usecount to keep it from getting
freed).  From vnode(9):

     vref(vp)
              Increment v_usecount of the vnode vp.  Any kernel thread system
              which uses a vnode (e.g., during the operation of some algorithm
              or to store in a data structure) should call vref().

which is awkwardly written, but basically if you have a pointer
someplace it has to be vref'd.


Does venus do locking on it's internal version of vnode, and if so does
it have the follow down from root approach?  Is there any possibility of
deadlock between kernel locks and venus locks?