Re: Request for testers - socket locking diff

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

On Wed, Apr 23, 2008 at 09:47:32PM +0100, Iain Hibbert wrote:

> On Wed, 23 Apr 2008, Andrew Doran wrote:
> 
> > 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)
> 
> Ok there was some tsleep that needed to be interlocked (during device
> setup), patch below. It seems to be working ok now though my machine is
> an old P3 laptop so no MP.

You can still run into many of the same problems on a UP machine but they
just don't occur as often.

> > 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.
> 
> I tried to make the interface between device drivers and bluetooth stack
> safe a while ago:
> 
> - data being passed upwards on device context is stored on a queue
>   protected by a per-device spin mutex with driver-specified IPL, then a
>   softint triggered which takes the data off the queue with the spin mutex
>   and handles it in the bluetooth stack context.
> 
> - data being passed downwards is handed to the driver and it is up to the
>   driver to arbitrate access. currently they all use splxxx() as
>   appropriate.

So downwards also uses a lock?
 
> perhaps this means the kernel_lock wrappers can come off the bluetooth
> code?

Does the lock also serialize access to the hardware and driver state, for
example locking out device interrupts? Otherwise it may be unsafe to call up
into the driver. Again, I don't have a good understanding of the bluetooth
code.

Thanks again,
Andrew