Re: More duplicate code, vnode locking question

[Elad Efrat <elad%NetBSD.org@localhost>]

On Wed, Apr 22, 2009 at 11:19 PM, Andrew Doran <ad%netbsd.org@localhost> wrote:
> On Tue, Apr 21, 2009 at 12:25:32AM +0300, Elad Efrat wrote:
>
>> However, this time vnode locking is involved, and I'm not sure what
>> the rules are about functions that are not in the vnode interface
>> (vnode_if?) that lock/unlock vnodes.
>
> - the vnode must always be locked for VOP_ACCESS().
> - the convention is that devvp is not held locked across mount.

Great. So can I remove the LOCKLEAF from the namei() call in
efs_vfsops.c and change the code in all file-systems to look the
snippet below?

        vn_lock(...);
        error = genfs_can_mount(...);
        VOP_UNLOCK(...);
        if (error)
        ...

(and of course add a comment above genfs_can_mount(), or
whatever the function will be named, that it requires a locked
vnode.)

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

Thanks,

-e.