Re: LOR with filedesc structure and Giant

On Sat, 16 Aug 2003, Poul-Henning Kamp wrote:

> >> The problem seems to be due to select() being called on the /dev/null
> >> device, and it is holding the filedesc lock when it reaches
> >> PICKUP_GIANT() in spec_poll.
> >
> >Yeah, this is pretty much the same issue you've been bumping into for a
> >bit -- we hold filedesc lock over select(), which means every object we
> >poll can't grab a lock that either comes before the file descriptor lockin
> >the lock order, or that might sleep.
> 
> Doesn't this effectively doom any attempt at getting rid af Giant from
> below ? 

I have mixed feelings about our current strategy.  On the one hand, it's a
very simple strategy to understand and implement -- it's also a reasonable
argument that poll operations for status might return "quickly" -- i.e.,
be safe while holding a mutex to prevent the filedesc array from changing.
On the other hand, the lock order and sleep implications are pretty
alarming, and have already caused a substantial number of problems.  It
would be interesting to know what consistency guarantees are provided for
the user app on other platforms with fine-grained kernel locking.

Approaches that come to mind include making a copy of the filedesc array
to prevent it from changing (sounds expensive for a select() call) to
avoid holding the mutex for long; we could move to an sx lock which would
fix the sleep issue at a slightly increased locking cost (but not solve
the lock order problem); if we push Giant past the file descriptor code in
one big throw that would resolve the lock order issue (but not the sleep
problem).  In a recent pass, I identified some of the locks with order
relationships with the file descriptor lock, and many of those will be
non-trivial to resolve.  For example, we grab file descriptor locks during
execve() to clean up the file descriptor array, and kevent interacts with
file descriptor locks.  Pushing Giant off further off execve() might have
a fair number of interactions with VFS and VM we'd want to watch out for
(on the other hand, we are probably close..)  Most of the changes to push
Giant behind the filedesc lock are not too bad, though.  I think it would
be worth a concerted effort by an interested and competent party to push
Giant behind the file descriptor lock.

Robert N M Watson             FreeBSD Core Team, TrustedBSD Projects
robert_at_fledge.watson.org      Network Associates Laboratories