cvs commit: src/sys/kern sys_socket.c

[Robert Watson]

On Tue, 30 Mar 2004, Nate Lawson wrote:

> On Sun, 28 Mar 2004, Robert Watson wrote:
> >   Modified files:
> >     sys/kern             sys_socket.c
> >   Log:
> >   Conditionally acquire Giant when entering the socket layer via file
> >   descriptor operations based on debug.mpsafenet, rather than acquiring
> >   Giant unconditionally.
> >
> >   Revision  Changes    Path
> >   1.56      +6 -6      src/sys/kern/sys_socket.c
> 
> This adds a small race if someone happens to disable debug.mpsafenet
> while a thread is holding Giant.  This may not be an issue since this is
> only a debugging sysctl and shouldn't be changed during network
> operation.  Still, it's important to note this. 

debug.mpsafenet is only modifiable as a tunable; as a sysctl, it's
read-only.  It's critical that this remain the case, or Very Bad Things
Might Happen, especially given that accesses to debug.mpsafenet aren't
synchronized so you can get stale values if it's changed, as well as the
sorts of races you've identified. :-)

> I've been finishing up ACPI locking and revisited a similar issue in the
> embedded controller driver.  When changing use of the global lock, I
> acquire the outer lock so I am certain no one is holding the inner lock
> and neglects to release it on the way out of a function because the
> global lock flag changed. 

For the purposes of the network stack locking work, I think supporting
run-time change of the policy isn't really useful to us -- at least, not
at the cost of finding a more flexible solution that's also more
expensive.

Robert N M Watson             FreeBSD Core Team, TrustedBSD Projects
robert at fledge.watson.org      Senior Research Scientist, McAfee Research