On 04/06/18(Mon) 21:11, Claudio Jeker wrote:
> On Mon, Jun 04, 2018 at 03:25:17PM +0200, Martin Pieuchot wrote:
> > For pfkey and routing sockets, calling sofree() in pr_detach is a noop.
> > The reason is that `SS_NOFDREF' hasn't been set at this point so the
> > socket won't be freed.
> > 
> > So I'd like to remove the sofree() and add an assert instead.  sofree()
> > will need to change soon to be able to deal with per-socket locks as
> > well as global locks.  So having fewer of them help.
> > 
> > ok?
> 
> I'm not sure if I like it when implementations differ in use of the socket
> functions. In general I would prefer if the socket interface is used the
> same way by all protocols so that reviewing them is simpler.
> I understand that the mentioned ones are already kind of special
> snowflakes so maybe this is not an issue.

Could you please read my "Towards per-socket locks" diff?  No matter
which way we go the implementation will differ because we're using
different locks.  I'm trying to keep the number of lock-related hacks
low.

> > Index: kern/uipc_socket.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> > retrieving revision 1.221
> > diff -u -p -r1.221 uipc_socket.c
> > --- kern/uipc_socket.c      8 May 2018 15:03:27 -0000       1.221
> > +++ kern/uipc_socket.c      4 Jun 2018 10:16:30 -0000
> > @@ -282,8 +282,7 @@ drop:
> >                     error = error2;
> >     }
> >  discard:
> > -   if (so->so_state & SS_NOFDREF)
> > -           panic("soclose NOFDREF: so %p, so_type %d", so, so->so_type);
> > +   KASSERT((so->so_state & SS_NOFDREF) == 0);
> >     so->so_state |= SS_NOFDREF;
> >     sofree(so);
> >     sounlock(s);
> > @@ -306,8 +305,7 @@ soaccept(struct socket *so, struct mbuf 
> >  
> >     soassertlocked(so);
> >  
> > -   if ((so->so_state & SS_NOFDREF) == 0)
> > -           panic("soaccept !NOFDREF: so %p, so_type %d", so, so->so_type);
> > +   KASSERT((so->so_state & SS_NOFDREF) != 0);
> >     so->so_state &= ~SS_NOFDREF;
> >     if ((so->so_state & SS_ISDISCONNECTED) == 0 ||
> >         (so->so_proto->pr_flags & PR_ABRTACPTDIS) == 0)
> > Index: net/pfkeyv2.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> > retrieving revision 1.180
> > diff -u -p -r1.180 pfkeyv2.c
> > --- net/pfkeyv2.c   19 May 2018 20:04:55 -0000      1.180
> > +++ net/pfkeyv2.c   4 Jun 2018 10:17:11 -0000
> > @@ -323,8 +323,9 @@ pfkeyv2_detach(struct socket *so)
> >     refcnt_finalize(&kp->refcnt, "pfkeyrefs");
> >  
> >     so->so_pcb = NULL;
> > -   sofree(so);
> > +   KASSERT((so->so_state & SS_NOFDREF) == 0);
> >     free(kp, M_PCB, sizeof(struct keycb));
> > +
> >     return (0);
> >  }
> >  
> > Index: net/rtsock.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/rtsock.c,v
> > retrieving revision 1.265
> > diff -u -p -r1.265 rtsock.c
> > --- net/rtsock.c    14 May 2018 07:33:59 -0000      1.265
> > +++ net/rtsock.c    4 Jun 2018 10:17:10 -0000
> > @@ -296,7 +296,7 @@ route_detach(struct socket *so)
> >     refcnt_finalize(&rop->refcnt, "rtsockrefs");
> >  
> >     so->so_pcb = NULL;
> > -   sofree(so);
> > +   KASSERT((so->so_state & SS_NOFDREF) == 0);
> >     free(rop, M_PCB, sizeof(struct routecb));
> >  
> >     return (0);
> > 
> 
> -- 
> :wq Claudio
> 

Reply via email to