Re: fdrop_locked() and FILE_LOCK() vs. Giant

2003-06-17 Thread Robert Watson

On Tue, 17 Jun 2003, Don Lewis wrote:

 The FILE_LOCK() implementation uses pool mutex under the hood, which
 means it should only be used as a leaf level mutex.  The fdrop_locked() 
 code wants to be called with FILE_LOCK() held, but the fdrop_locked() 
 implementation calls mtx_lock(Giant) before calling FILE_UNLOCK().  In
 addition to violating the proper usage of the pool mutex, there is
 also the potential for a lock order violation.  The close() 
 implementation grabs Giant and eventually calls fdrop(), which calls
 FILE_LOCK() immediately before calling fdrop_locked().  If another
 caller of fdrop_locked() calls FILE_LOCK() without grabbing Giant first,
 then the lock order will be reversed when fdrop_locked() grabs Giant. 
 
 It looks like fdrop_locked() should require that Giant be grabbed by the
 caller before fdrop_locked() is called. 

I've also noticed that the file descriptor lock is held over per-object
calls to fo_poll(), which currently isn't a big deal for most objects, but
may be in the future if we have to grab other locks in order to test the
poll status inside the object.  I run into this problem with the MAC work
because the vnode label is protected by the vnode lock, which is a
sleepable lock.  We may want to change label locking in the future to
avoid this, but in the mean time I get a lot of witness warnings, and
using a pool mutex for the fd lock may cause lock order problems if there
is more complex locking.

Robert N M Watson FreeBSD Core Team, TrustedBSD Projects
[EMAIL PROTECTED]  Network Associates Laboratories


___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: fdrop_locked() and FILE_LOCK() vs. Giant

2003-06-17 Thread Don Lewis
On 17 Jun, Robert Watson wrote:
 
 On Tue, 17 Jun 2003, Don Lewis wrote:
 
 The FILE_LOCK() implementation uses pool mutex under the hood, which
 means it should only be used as a leaf level mutex.  The fdrop_locked() 
 code wants to be called with FILE_LOCK() held, but the fdrop_locked() 
 implementation calls mtx_lock(Giant) before calling FILE_UNLOCK().  In
 addition to violating the proper usage of the pool mutex, there is
 also the potential for a lock order violation.  The close() 
 implementation grabs Giant and eventually calls fdrop(), which calls
 FILE_LOCK() immediately before calling fdrop_locked().  If another
 caller of fdrop_locked() calls FILE_LOCK() without grabbing Giant first,
 then the lock order will be reversed when fdrop_locked() grabs Giant. 
 
 It looks like fdrop_locked() should require that Giant be grabbed by the
 caller before fdrop_locked() is called. 
 
 I've also noticed that the file descriptor lock is held over per-object
 calls to fo_poll(), which currently isn't a big deal for most objects, but
 may be in the future if we have to grab other locks in order to test the
 poll status inside the object.  I run into this problem with the MAC work
 because the vnode label is protected by the vnode lock, which is a
 sleepable lock.  We may want to change label locking in the future to
 avoid this, but in the mean time I get a lot of witness warnings, and
 using a pool mutex for the fd lock may cause lock order problems if there
 is more complex locking.

Does witness even keep track of the pool mutex stuff?  It doesn't seem
to report any lock order problems in the fdrop_locked() case.  I'm
attempting to debug a deadlock problem for someone, and one process is
hung on FILE_LOCK() in fdrop(), but show witness in ddb doesn't
mention any pool mutex holders.  The process hung in fdrop() got there
by calling close(), which grabs Giant.  Once it wedge, then everything
else on the system stacked up waiting for Giant.

Alfred mentioned that fdrop_locked() can be easily fixed by dropping the
file lock a bit sooner.
___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to [EMAIL PROTECTED]