cvs commit: src/sys/kern vnode_if.src
truckman at FreeBSD.org
Mon May 29 15:03:30 PDT 2006
On 28 May, Diomidis Spinellis wrote:
> dds 2006-05-28 07:24:12 UTC
> FreeBSD src repository
> Modified files:
> sys/kern vnode_if.src
> Add missing % signs in the lock annotations of the functions:
> lookup, rename, strategy, islocked
> The missing % sign meant that the lines were processed as plain
> comments and the corresponding assertions were never generated.
> Revision Changes Path
> 1.80 +8 -8 src/sys/kern/vnode_if.src
[appearing from the void]
The additional sanity checking generated by this patch made my system
unbootable with DEBUG_VFS_LOCKS enabled.
ASSERT_VI_UNLOCKED() calls are added to VOP_ISLOCKED_APV(), but there
is an explicit call to VOP_ISLOCKED() in vput() with the vnode
interlock held. This could be fixed by re-ordering the code in
vput(), but there are also a number of places in lower level code that
are called with the vnode interlock held that contain calls to
ASSERT_VOP_LOCKED(), which in turn calls VOP_ISLOCKED(). Possible
Specify the locking value as "-" for vop_islocked() in vnode_if.src
Introduce a new locking value, similar to "=", but which skips the
interlock check. Note: vnode_if.awk does not implement the "="
vnode lock assertion.
The fdvp and fvp initial vnode lock state assertions can't be checked
by the generated wrapper. If fdvp and fvp happen to be references to
tdvp and/or tvp, which must be locked, then fdvp and/or fvp will also
be locked. They should only be unlocked if they are different vnodes
than tdvp and tvp. The proper sanity checking appears to be done in
vop_rename_pre(). The initial lock value for fdvp and fvp should
probably be set to "-" in vnode_if.src, or to another value that only
generates the interlock assertion.
VOP_RENAME_APV() calls ASSERT_VI_UNLOCKED() on tvp before and after
the vop->vop_rename() call, even though tvp may be NULL. vnode_if.src
specifies the initial lock state as "X", which indicates that the
check should be skipped if the vnode pointer is NULL, but vnode_if.awk
does not handle the "X" value at all, and adds an unconditional
interlock check. vop_rename_pre() does the conditional vnode lock
check, so maybe the interlock check could be done there as well and
omitted from the wrapper. The final state is specified as "U", though
this should also be a conditional check, but there is no vnode locking
value that generates a conditional unlock assertion.
vop_rename_post() does not do any lock state sanity checks.
BTW, the VOP_ISLOCKED(9) man page does not document that the return
value is the shared/exclusive lock type.
[disappearing into the void]
More information about the cvs-src