On Thu, Apr 05, 2007 at 08:41:58PM -0400, Greg Troxel wrote:
> 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

I would expect this to be,
    if ..
	ref child, ref parent, unlock parent,
	lock child, lock parent, unref parent

Otherwise we may lose the parent vnode if we happen to block while
trying to lock the child. (since the parent is unlocked and possibly
unreferenced). Unless of course the parent is not just locked, but also
has a ref when we entered lookup.

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

Beginning of vproc::lookup (vproc_vfscalls.cc)

    /* Get the target object. */
    if (STREQ(name, ".")) {
	/* Don't get the same object twice! */
	target_fso = parent_fso;
	parent_fso = 0; /* Fake a FSDB->Put(&parent_fso); */
    }

So yes, it is guaranteed.

> > It looks like the patch already got committed to CVS, so I guess my
> > other worry is unfounded.
> 
> Commited doesn't imply correct....

Ah, different developer model, my Coda patches for Linux often have to
pass through one or two subsystem maintainers and end up in Andrew
Morton's -mm tree for a bit before they hit the main kernel. Unless it
is an critical fix.

> 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?

We lock file objects based on fid ordering, although there is also a
per-volume read/write lock so we can only have a single writer for any
given volume. However I don't think we can have deadlocks as a result of
different ordering. Venus only holds locks for the duration of handling
an upcall, and we are guaranteed that at least one worker thread is able
to make progress.

Jan