Re: Request for testers - socket locking diff

[Andrew Doran <ad%netbsd.org@localhost>]

On Wed, Apr 23, 2008 at 08:18:33AM +0100, Iain Hibbert wrote:

> On Tue, 22 Apr 2008, Andrew Doran wrote:
> 
> > Latest version of the patch, against -current today:
> >
> > http://www.netbsd.org/~ad/socklock-2008042201.diff
> 
> Sorry was unable to boot a -current kernel for a couple of weeks (acpi
> issue) but will test the bluetooth as far as I'm able soon (visually it
> looks ok)

Great, thank you.
 
> > - Make protocol entry points take kernel_lock, kill the inline wrappers.
> 
> I like that better, it would be intended that eventually locking is pushed
> right down to individual connections?

So, there are essentially two layers of locking now:

o The socket lock, which covers socket activity and the low priority
  protocol processing interrupts and timeouts. I think that should be fairly
  clear. For bluetooth I have given it a single subsystem lock (bt_lock). 
  The other network protocols share a global lock (softnet_lock) because
  they make a lot of cross calls and I haven't traced all the paths through. 
  I don't know how you would make that more fine grained for bluetooth
  because I don't know how data flows within the bluetooth stack, but I'm
  sure it would be possible to do that.

o The kernel_lock covering anything to do with the hardware, processing that
  occurs from hardware interrupts (e.g. ip_flow), and so on. For bluetooth
  I'd expect that only needs to lock against device driver interrupts, and
  could be replaced with mutexes in the hardware drivers or something
  along those lines. Again, I don't understand how it interfaces with
  hardware so for now I've left it at the protocol boundary just to be on
  the safe side. In a lot of situations it's probably not necessary to take
  kernel_lock because we are never going to touch any of that state.

What I want to do with the patch is to make the socket code completely free
of kernel_lock, and to allow network processing soft interrupts to block.
Previously, if they blocked, there were no locks involved, so the socket
code could call up through e.g. tcp_usrreq() and find half-baked PCBs and
so on, causing problems. With the patch it's safe, because the protocol
interrupt holds a lock that will prevent the socket layer from calling up
while it's blocked.

To cut a long story short, the job of taking the protocols out from under
kernel_lock and making them fine grained is a different one and not
something that I'm planning to work on. :-)

> > I think this is nearly ready to go, aside from more testing.
> 
> I think you need the below patch (GENERIC kernel does not include ISO)?

Ah, cheers. I built ALL with -O0 which seems to emit a different set of
warnings to -O2.

Thanks,
Andrew