Deadlock between nfsd and snapshots.

[Tor Egge]

> I found some cases where i_flag, timestamps and probably other parts of the
> vnode seem to be accessed without any proper locks (maybe a refcount) :-(.
> See another (private) reply and below.
> 
> I don't understand how the vnode interlock can help much for interoperation
> with code which uses vnode locks.  Aren't these locks almost independent of 
> each other, so if the vnode lock is already held then acquiring the vnode
> interlock wouldn't block?  So to protect iflag with the interlock you
> would have to add the interlock to mounds of code that just uses the vnode
> lock now.

the vnode interlock is used to resolve between those holding a shared lock.

There are 4 locking protocols for the protection of i_flags and the
timestamps:

 1.  Previous locking protocol: Giant

 2.  Current locking protocol: vnode lock.  This breaks down when processes
     with a shared vnode lock perform changes (e.g. sets flags or timestamps).

 3.  Proposed locking protocol: exclusive vnode lock or shared
     vnode lock plus the vnode interlock.

 4.  Alternate locking protocol: vnode interlock.  Further code changes
     might be needed for this to work properly, e.g. IN_MODIFIED might
     have to be set before making changes to the inode instead of
     aftewards.

> I understand this locking :-).  ffs_sync() actually uses only uses the vnode
> interlock to access i_flag.  I think this is intentionally quick and not
> quite right -- there is a comment a few lines before it saying that we depend
> on a mntvnode lock to keep things stable enough for a quick test.  The scope
> of the comment is unclear.  I think the quick test is only good enough for
> sync(2).

The correct unoptimized version of ffs_sync() would have the vnode lock before
checking i_flag.

When this loop executes as part of suspending the file system, MNTK_SUSPEND is
set.  No code is executing within regions protected by vn_start_write() and new
calls to vn_start_write() will fail or block.  Calls to
vn_start_secondary_write() might succeed but will then trigger a retry of the
whole vnode sync loop.

While not having the vnode lock when checking i_flag in ffs_sync() opens a
race, the conseqeuences are limited due to other locking and the restart.

- Tor Egge