cvs commit: src/sys/kern vnode_if.src

[Don Lewis]

On 28 May, Diomidis Spinellis wrote:
> dds         2006-05-28 07:24:12 UTC
> 
>   FreeBSD src repository
> 
>   Modified files:
>     sys/kern             vnode_if.src 
>   Log:
>   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
  fixes:
    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]