On Mon, Jan 20, 2025 at 05:29:01PM +0300, Vitaliy Makkoveev wrote:
> On Mon, Jan 20, 2025 at 01:37:24PM +0100, Alexander Bluhm wrote:
> > Hi,
> > 
> > I want to make UNIX and TCP socket locking more similar.
> > 
> > One difference is that UNIX sockets unlock in soabort() while TCP
> > does not do that.  in_pcbdetach() does keep the lock, so let's
> > change uipc_abort().  This also gives symetric lock and unlock in
> > the caller.  Refcount is needed to call unlock on an aborted socket.
> > 
> > The queue 0 in soclose() is only used by UNIX sockets, so replace
> > the "if" with "kassert".
> > 
> > ok?
> > 
> 
> The diff is ok mvs@, but do we really need this assertion?
> 
> > +		if (!TAILQ_EMPTY(&so->so_q0))
> > +			KASSERT(persocket);

Not really, I can remove it.  It passed regress once with assertion,
that should be enough.  If you want, I can also remove the same
assertion in soisconnected().

bluhm

> > Index: kern/uipc_socket.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket.c,v
> > diff -u -p -r1.360 uipc_socket.c
> > --- kern/uipc_socket.c	13 Jan 2025 18:10:20 -0000	1.360
> > +++ kern/uipc_socket.c	18 Jan 2025 16:40:02 -0000
> > @@ -399,23 +399,27 @@ drop:
> >  	if (so->so_options & SO_ACCEPTCONN) {
> >  		int persocket = solock_persocket(so);
> >  
> > +		if (!TAILQ_EMPTY(&so->so_q0))
> > +			KASSERT(persocket);
> >  		while ((so2 = TAILQ_FIRST(&so->so_q0)) != NULL) {
> > -			if (persocket)
> > -				solock(so2);
> > +			soref(so2);
> > +			solock(so2);
> >  			(void) soqremque(so2, 0);
> > -			if (persocket)
> > -				sounlock(so);
> > +			sounlock(so);
> >  			soabort(so2);
> > -			if (persocket)
> > -				solock(so);
> > +			sounlock(so2);
> > +			sorele(so2);
> > +			solock(so);
> >  		}
> >  		while ((so2 = TAILQ_FIRST(&so->so_q)) != NULL) {
> > -			if (persocket)
> > -				solock(so2);
> > +			soref(so2);
> > +			solock_nonet(so2);
> >  			(void) soqremque(so2, 1);
> >  			if (persocket)
> >  				sounlock(so);
> >  			soabort(so2);
> > +			sounlock_nonet(so2);
> > +			sorele(so2);
> >  			if (persocket)
> >  				solock(so);
> >  		}
> > Index: kern/uipc_usrreq.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_usrreq.c,v
> > diff -u -p -r1.212 uipc_usrreq.c
> > --- kern/uipc_usrreq.c	1 Jan 2025 13:44:22 -0000	1.212
> > +++ kern/uipc_usrreq.c	18 Jan 2025 16:24:10 -0000
> > @@ -656,7 +656,7 @@ uipc_abort(struct socket *so)
> >  	struct unpcb *unp = sotounpcb(so);
> >  
> >  	unp_detach(unp);
> > -	sofree(so, 0);
> > +	sofree(so, 1);
> >  }
> >  
> >  int
> >