Re: Help with the NET_LOCK() - socreate
On 25/01/17(Wed) 15:52, Theo de Raadt wrote:
> I have the same diff.
>
> It does not help the pr_usrreq path though.
Sure but, let's do baby step. :)
> >On Wed, Jan 25, 2017 at 11:02:22AM -0500, David Hill wrote:
> >> splassert: yield: want 0 have 1
> >> Starting stack trace...
> >> yield() at yield+0xac
> >> pool_get() at pool_get+0x1ca
> >> socreate() at socreate+0xba
> >> sys_socket() at sys_socket+0x135
> >> syscall() at syscall+0x27b
> >> --- syscall (number 97) ---
> >> end of kernel
> >> end trace frame: 0x1b1a16c05800, count: 252
> >> 0x1b197d23277a:
> >> End of stack trace.
> >
> >This one looks easy. We do not need a lock to setup the still
> >private so structure.
> >
> >ok?
> >
> >bluhm
> >
> >Index: kern/uipc_socket.c
> >===
> >RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> >retrieving revision 1.173
> >diff -u -p -r1.173 uipc_socket.c
> >--- kern/uipc_socket.c 25 Jan 2017 16:45:50 - 1.173
> >+++ kern/uipc_socket.c 25 Jan 2017 18:01:06 -
> >@@ -123,7 +123,6 @@ socreate(int dom, struct socket **aso, i
> > return (EPROTONOSUPPORT);
> > if (prp->pr_type != type)
> > return (EPROTOTYPE);
> >-NET_LOCK(s);
> > so = pool_get(_pool, PR_WAITOK | PR_ZERO);
> > TAILQ_INIT(>so_q0);
> > TAILQ_INIT(>so_q);
> >@@ -136,6 +135,7 @@ socreate(int dom, struct socket **aso, i
> > so->so_egid = p->p_ucred->cr_gid;
> > so->so_cpid = p->p_p->ps_pid;
> > so->so_proto = prp;
> >+NET_LOCK(s);
> > error = (*prp->pr_usrreq)(so, PRU_ATTACH, NULL,
> > (struct mbuf *)(long)proto, NULL, p);
> > if (error) {
> >
> >
>
Re: Help with the NET_LOCK() - socreate
I have the same diff.
It does not help the pr_usrreq path though.
>On Wed, Jan 25, 2017 at 11:02:22AM -0500, David Hill wrote:
>> splassert: yield: want 0 have 1
>> Starting stack trace...
>> yield() at yield+0xac
>> pool_get() at pool_get+0x1ca
>> socreate() at socreate+0xba
>> sys_socket() at sys_socket+0x135
>> syscall() at syscall+0x27b
>> --- syscall (number 97) ---
>> end of kernel
>> end trace frame: 0x1b1a16c05800, count: 252
>> 0x1b197d23277a:
>> End of stack trace.
>
>This one looks easy. We do not need a lock to setup the still
>private so structure.
>
>ok?
>
>bluhm
>
>Index: kern/uipc_socket.c
>===
>RCS file: /cvs/src/sys/kern/uipc_socket.c,v
>retrieving revision 1.173
>diff -u -p -r1.173 uipc_socket.c
>--- kern/uipc_socket.c 25 Jan 2017 16:45:50 - 1.173
>+++ kern/uipc_socket.c 25 Jan 2017 18:01:06 -
>@@ -123,7 +123,6 @@ socreate(int dom, struct socket **aso, i
> return (EPROTONOSUPPORT);
> if (prp->pr_type != type)
> return (EPROTOTYPE);
>- NET_LOCK(s);
> so = pool_get(_pool, PR_WAITOK | PR_ZERO);
> TAILQ_INIT(>so_q0);
> TAILQ_INIT(>so_q);
>@@ -136,6 +135,7 @@ socreate(int dom, struct socket **aso, i
> so->so_egid = p->p_ucred->cr_gid;
> so->so_cpid = p->p_p->ps_pid;
> so->so_proto = prp;
>+ NET_LOCK(s);
> error = (*prp->pr_usrreq)(so, PRU_ATTACH, NULL,
> (struct mbuf *)(long)proto, NULL, p);
> if (error) {
>
>
Re: Help with the NET_LOCK() - socreate
On 25/01/17(Wed) 19:04, Alexander Bluhm wrote:
> On Wed, Jan 25, 2017 at 11:02:22AM -0500, David Hill wrote:
> > splassert: yield: want 0 have 1
> > Starting stack trace...
> > yield() at yield+0xac
> > pool_get() at pool_get+0x1ca
> > socreate() at socreate+0xba
> > sys_socket() at sys_socket+0x135
> > syscall() at syscall+0x27b
> > --- syscall (number 97) ---
> > end of kernel
> > end trace frame: 0x1b1a16c05800, count: 252
> > 0x1b197d23277a:
> > End of stack trace.
>
> This one looks easy. We do not need a lock to setup the still
> private so structure.
>
> ok?
It's right, in the end what we want is to protect the insertion of the
newly created socket to a global list.
ok mpi@
> Index: kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.173
> diff -u -p -r1.173 uipc_socket.c
> --- kern/uipc_socket.c25 Jan 2017 16:45:50 - 1.173
> +++ kern/uipc_socket.c25 Jan 2017 18:01:06 -
> @@ -123,7 +123,6 @@ socreate(int dom, struct socket **aso, i
> return (EPROTONOSUPPORT);
> if (prp->pr_type != type)
> return (EPROTOTYPE);
> - NET_LOCK(s);
> so = pool_get(_pool, PR_WAITOK | PR_ZERO);
> TAILQ_INIT(>so_q0);
> TAILQ_INIT(>so_q);
> @@ -136,6 +135,7 @@ socreate(int dom, struct socket **aso, i
> so->so_egid = p->p_ucred->cr_gid;
> so->so_cpid = p->p_p->ps_pid;
> so->so_proto = prp;
> + NET_LOCK(s);
> error = (*prp->pr_usrreq)(so, PRU_ATTACH, NULL,
> (struct mbuf *)(long)proto, NULL, p);
> if (error) {
>
Re: Help with the NET_LOCK() - socreate
On Wed, Jan 25, 2017 at 07:04:19PM +0100, Alexander Bluhm wrote: > On Wed, Jan 25, 2017 at 11:02:22AM -0500, David Hill wrote: > > splassert: yield: want 0 have 1 > > Starting stack trace... > > yield() at yield+0xac > > pool_get() at pool_get+0x1ca > > socreate() at socreate+0xba > > sys_socket() at sys_socket+0x135 > > syscall() at syscall+0x27b > > --- syscall (number 97) --- > > end of kernel > > end trace frame: 0x1b1a16c05800, count: 252 > > 0x1b197d23277a: > > End of stack trace. > > This one looks easy. We do not need a lock to setup the still > private so structure. > > ok? > > bluhm > Which uncovers: splassert: yield: want 0 have 1 Starting stack trace... yield() at yield+0xac malloc() at malloc+0x406 route_usrreq() at route_usrreq+0xb0 socreate() at socreate+0x15f sys_socket() at sys_socket+0x135 syscall() at syscall+0x27b --- syscall (number 97) --- end of kernel end trace frame: 0x1b171444a720, count: 251 0x1b19e6cfd5ca: End of stack trace.
Re: Help with the NET_LOCK() - socreate
On Wed, Jan 25, 2017 at 11:02:22AM -0500, David Hill wrote:
> splassert: yield: want 0 have 1
> Starting stack trace...
> yield() at yield+0xac
> pool_get() at pool_get+0x1ca
> socreate() at socreate+0xba
> sys_socket() at sys_socket+0x135
> syscall() at syscall+0x27b
> --- syscall (number 97) ---
> end of kernel
> end trace frame: 0x1b1a16c05800, count: 252
> 0x1b197d23277a:
> End of stack trace.
This one looks easy. We do not need a lock to setup the still
private so structure.
ok?
bluhm
Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.173
diff -u -p -r1.173 uipc_socket.c
--- kern/uipc_socket.c 25 Jan 2017 16:45:50 - 1.173
+++ kern/uipc_socket.c 25 Jan 2017 18:01:06 -
@@ -123,7 +123,6 @@ socreate(int dom, struct socket **aso, i
return (EPROTONOSUPPORT);
if (prp->pr_type != type)
return (EPROTOTYPE);
- NET_LOCK(s);
so = pool_get(_pool, PR_WAITOK | PR_ZERO);
TAILQ_INIT(>so_q0);
TAILQ_INIT(>so_q);
@@ -136,6 +135,7 @@ socreate(int dom, struct socket **aso, i
so->so_egid = p->p_ucred->cr_gid;
so->so_cpid = p->p_p->ps_pid;
so->so_proto = prp;
+ NET_LOCK(s);
error = (*prp->pr_usrreq)(so, PRU_ATTACH, NULL,
(struct mbuf *)(long)proto, NULL, p);
if (error) {
Re: Help with the NET_LOCK() - socreate
On Wed, Jan 25, 2017 at 04:32:25PM +1000, Martin Pieuchot wrote: > I just enabled the NET_LOCK() again and I'm looking for test reports. > Please go build a kernel from sources or wait for the next snapshot, > run it and report back. > > If you're looking for some small coding tasks related to the NET_LOCK() > just do: > > # sysctl kern.splassert=2 > # sysctl kern.pool_debug=2 > > Then watch for the traces on your console. > > You'll see something like: > > Starting stack trace... > yield(0,1,d09dac52,f5549dbc,d94e9378) at yield+0xa4 > yield(d0bc8f40,1,f5549e18,80,14) at yield+0xa4 > pool_get(d0bc8f40,1,f5549ec8,d03ecbfb,d97815f4) at pool_get+0x1ba > m_get(1,3,f5549ec0,d03a9362,d0bc22e0) at m_get+0x30 > doaccept(d977e6c4,3,cf7ee4f8,cf7ee4ec,2000) at doaccept+0x193 > sys_accept(d977e6c4,f5549f5c,f5549f7c,0,f5549fa8) at sys_accept+0x37 > syscall() at syscall+0x250 > > This means accept(2) is doing a memory allocation that can sleep, here > with m_get(9), while holding the NET_LOCK(). Even if these should be > ok, it is easy to avoid them. In the case of doaccept() a mbuf could > be allocated beforehand or simply use the stack for that. > > Cheers, > Martin > splassert: yield: want 0 have 1 Starting stack trace... yield() at yield+0xac pool_get() at pool_get+0x1ca socreate() at socreate+0xba sys_socket() at sys_socket+0x135 syscall() at syscall+0x27b --- syscall (number 97) --- end of kernel end trace frame: 0x1b1a16c05800, count: 252 0x1b197d23277a: End of stack trace.
