On Thu, May 28, 2020 at 12:26:39PM +0200, Martin Pieuchot wrote:
> On 27/05/20(Wed) 11:54, Vitaliy Makkoveev wrote:
> > pipex(4) is simultaneously protected by KERNEL_LOCK() and NET_LOCK() and
> > the last is not required. I guess to start remove NET_LOCK(). Diff below
> > drops NET_LOCK() in pipex_ioctl() and underlaying routines. At least
> > this helps to kill unlock/lock mess in pppx_add_session() and
> > pppx_if_destroy().
>
> Getting rid of the NET_LOCK() might indeed help to untangle the current
> locking mess. However we should aim towards removing the KERNEL_LOCK()
> from, at least, the packet processing path starting with pipexintr().
I guess we can made `pipexoutq' processing NET_LOCK() free. Also
`pipexinq' processing requires a little amount of places where NET_LOCK()
is required. So we can implement special locks for pipex(4).
>
> ok mpi@
>
> > Index: sys/net/pipex.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/pipex.c,v
> > retrieving revision 1.113
> > diff -u -p -r1.113 pipex.c
> > --- sys/net/pipex.c 7 Apr 2020 07:11:22 -0000 1.113
> > +++ sys/net/pipex.c 27 May 2020 08:46:32 -0000
> > @@ -205,7 +205,8 @@ pipex_ioctl(struct pipex_iface_context *
> > {
> > int pipexmode, ret = 0;
> >
> > - NET_LOCK();
> > + KERNEL_ASSERT_LOCKED();
> > +
> > switch (cmd) {
> > case PIPEXSMODE:
> > pipexmode = *(int *)data;
> > @@ -250,7 +251,6 @@ pipex_ioctl(struct pipex_iface_context *
> > ret = ENOTTY;
> > break;
> > }
> > - NET_UNLOCK();
> >
> > return (ret);
> > }
> > @@ -269,6 +269,8 @@ pipex_add_session(struct pipex_session_r
> > struct ifnet *over_ifp = NULL;
> > #endif
> >
> > + KERNEL_ASSERT_LOCKED();
> > +
> > /* Checks requeted parameters. */
> > if (!iface->pipexmode)
> > return (ENXIO);
> > @@ -423,7 +425,6 @@ pipex_add_session(struct pipex_session_r
> > }
> > #endif
> >
> > - NET_ASSERT_LOCKED();
> > /* commit the session */
> > if (!in_nullhost(session->ip_address.sin_addr)) {
> > if (pipex_lookup_by_ip_address(session->ip_address.sin_addr)
> > @@ -473,7 +474,7 @@ pipex_add_session(struct pipex_session_r
> > int
> > pipex_notify_close_session(struct pipex_session *session)
> > {
> > - NET_ASSERT_LOCKED();
> > + KERNEL_ASSERT_LOCKED();
> > session->state = PIPEX_STATE_CLOSE_WAIT;
> > session->stat.idle_time = 0;
> >
>
