Re: Request for testers - socket locking diff

[Matt Thomas <matt%3am-software.com@localhost>]

On Apr 13, 2008, at 10:17 AM, Andrew Doran wrote:
> On Sat, Apr 12, 2008 at 10:41:18PM +0100, Andrew Doran wrote:
>
> > Here's an updated patch.
> >
> >         http://www.netbsd.org/~ad/socklock-2008041201.diff
> >
> > - Rudimentary tests on inet6 show it working.
> > - The bluetooth patch is merged in.
> > - More bugfixes, and assertions.
> > - The sbcheck change is bogus (sbcheck appears to be bogus, anyway)
> > - I think that I would prefer to move acquire of kernel_lock into the
> >   individual protocols, instead of always having the wrappers which
> >   will be unneeded baggage in the future.
>
> One problem I know of: I think there is a race when sending SCM_RIGHTS
> messages through a Unix socket. The symptom I see is gnome-terminal  
> hanging
> due to a corrupted sockbuf. I think that the loop around  
> unp_externalize in
> soreceive needs to be atomic. I'm surprised we never got bitten by  
> this
> before since it could block, but whatever.
>
> Aside from the below I think the patch is nearly ready:
>
> - Make protosw::pr_lock mandatory.

You mean pr_getlock?

> - Make protocol methods take kernel_lock directly, and remove  
> wrappers.
> - Profiling and further testing.
> - In netisrs, always lock in the socket -> kernel_lock direction,  
> instead
>   of letting the softint code take kernel_lock. This is just for  
> efficiency.
>
> Any comments on it?


In sys/unpcb.h, shouldn't unp_streamlock be kmutex_t * instead of  
void *?

sonewconn should have an extra param which is the kmutex_t * for the new
socket.  Actually, I want two new params, the kmutex_t * and a void *  
for
initializing so_pcb but that's in the future.

pr_getlock() just seems wrong to me.  How hard is it going to be change
the so_lock of a socket?  (which would happen in PRU_ATTACH)?

I guess I think PRU_ATTACH should be called with so_lock NULL by default
and unlocked.

I don't you handle the case where head_so != so in the soq* routines.