Jan Harkes <jaharkes_at_cs.cmu.edu> writes:

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

The caller has to have a ref.  You're not allowed to have a lock without
a ref, since to have a lock you have to store a pointer so you can
unlock it.  So coda_lookup is riding the caller's ref.   The parent
vnode is supposed to be returned locked, with no refcount changes.

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

Thanks, comment about future optimization adjusted.


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

Well, Linux doesn't guarantee correct either, it's just a different
review process :-) My changes were posted for public review and looked
at by one of the people most clueful about vfs.  And this is -current,
not a release - it has to be in -current for at least a week and then I
can submit a request to have it pulled up to release branches.

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

Then I don't think there will be any vfs locking/coda locking deadlocks
(if there aren't any on either side separately).

I am now running postmark on my crashbox, with default parameters.
Any other good filesystem abuse programs out there?