Re: More duplicate code, vnode locking question

[Antti Kantee <pooka%cs.hut.fi@localhost>]

On Thu Apr 23 2009 at 00:36:20 +0300, Elad Efrat wrote:
> On Thu, Apr 23, 2009 at 12:24 AM, Antti Kantee <pooka%cs.hut.fi@localhost> 
> wrote:
> > On Wed Apr 22 2009 at 23:45:01 +0300, Elad Efrat wrote:
> >> >> Also, it seems that some file-systems call VOP_OPEN() without devvp
> >> >> being locked. Compare, for example, ffs_mount() and efs_mount(). Is
> >> >> this possible, or am I missing something? :)
> >> >
> >> > This is a bug.
> >>
> >> Okay. Unless someone beats me to it, once I'm done with some other stuff
> >> I'll go over the file-systems and make a patch to add locking where it's
> >> missing (in this regard only! :) and post it.
> >>
> >> What I have in mind is adding vn_lock() before the VOP_OPEN() call, and
> >> adjust error paths to vput() where the vnode is locked and vrele()
> >> otherwise. Is this correct?
> >
> > I already explained why locking it is a bug also.  Why are you replacing
> > a bug with a theoretically more severe bug?  Did you disagree with
> > my explanation?
> 
> First of all, I'm not replacing anything. All I did was bring into the
> discussion what was on my mind to fix the bug. Apparently, I
> misunderstood you (I phrase things differently when I disagree).
> 
> Also, I have two other questions:
> 
>   - How does locking devvp violate the locking order of vnodes? (note
>     that I'm not suggesting it doesn't)

I wrote that in the previous email.  You always lock from root down.
If you don't know the relationship of two vnodes, you cannot lock
them both at the same time.  In this case you are already holding the
mountpoint lock.

Like I also wrote, the mount path locking is special because of recursive
locks (which combined with namei already is a deadlock waiting to
happen!).  No point in fussing around if you can't fix anything.

>   - How do we weigh what is a more severe bug? :)

i decide ;)