Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: Merge UNP_BINDING and UNP_CONNECTING flags to UNP_BUSY
To:
Alexander Bluhm <alexander.bluhm@gmx.net>
Cc:
tech@openbsd.org
Date:
Tue, 19 Mar 2024 17:22:11 +0300

Download raw body.

Thread
On Tue, Mar 19, 2024 at 02:49:49PM +0100, Alexander Bluhm wrote:
> On Tue, Mar 19, 2024 at 01:44:45PM +0300, Vitaliy Makkoveev wrote:
> > They are redundant because always checked together. These flags are
> > not used outside unix(4) PCB layer, so the change is local.
> 
> Change is correct.  It makes no difference if one or two bits are
> used.  But is it better?  Before you could see, why it wa busy.
> Don't know if the diff is an improvement.  But also no objections.
> 

I don't remember us ever being interested in the reason of unp busy.
However, I doubt the `unp_flags' checks will grow, so no problem to
drop this diff.

> bluhm
> 
> > Index: sys/kern/uipc_usrreq.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> > retrieving revision 1.201
> > diff -u -p -r1.201 uipc_usrreq.c
> > --- sys/kern/uipc_usrreq.c	17 Mar 2024 19:47:08 -0000	1.201
> > +++ sys/kern/uipc_usrreq.c	19 Mar 2024 10:37:21 -0000
> > @@ -329,14 +329,14 @@ uipc_bind(struct socket *so, struct mbuf
> >  	struct nameidata nd;
> >  	size_t pathlen;
> >  
> > -	if (unp->unp_flags & (UNP_BINDING | UNP_CONNECTING))
> > +	if (unp->unp_flags & UNP_BUSY)
> >  		return (EINVAL);
> >  	if (unp->unp_vnode != NULL)
> >  		return (EINVAL);
> >  	if ((error = unp_nam2sun(nam, &soun, &pathlen)))
> >  		return (error);
> >  
> > -	unp->unp_flags |= UNP_BINDING;
> > +	unp->unp_flags |= UNP_BUSY;
> >  
> >  	/*
> >  	 * Enforce `i_lock' -> `solock' because fifo subsystem
> > @@ -405,7 +405,7 @@ uipc_bind(struct socket *so, struct mbuf
> >  	VOP_UNLOCK(vp);
> >  out:
> >  	KERNEL_UNLOCK();
> > -	unp->unp_flags &= ~UNP_BINDING;
> > +	unp->unp_flags &= ~UNP_BUSY;
> >  
> >  	return (error);
> >  }
> > @@ -415,7 +415,7 @@ uipc_listen(struct socket *so)
> >  {
> >  	struct unpcb *unp = sotounpcb(so);
> >  
> > -	if (unp->unp_flags & (UNP_BINDING | UNP_CONNECTING))
> > +	if (unp->unp_flags & UNP_BUSY)
> >  		return (EINVAL);
> >  	if (unp->unp_vnode == NULL)
> >  		return (EINVAL);
> > @@ -820,7 +820,7 @@ unp_connect(struct socket *so, struct mb
> >  	int error;
> >  
> >  	unp = sotounpcb(so);
> > -	if (unp->unp_flags & (UNP_BINDING | UNP_CONNECTING))
> > +	if (unp->unp_flags & UNP_BUSY)
> >  		return (EISCONN);
> >  	if ((error = unp_nam2sun(nam, &soun, NULL)))
> >  		return (error);
> > @@ -829,7 +829,7 @@ unp_connect(struct socket *so, struct mb
> >  	nd.ni_pledge = PLEDGE_UNIX;
> >  	nd.ni_unveil = UNVEIL_WRITE;
> >  
> > -	unp->unp_flags |= UNP_CONNECTING;
> > +	unp->unp_flags |= UNP_BUSY;
> >  
> >  	/*
> >  	 * Enforce `i_lock' -> `solock' because fifo subsystem
> > @@ -921,7 +921,7 @@ put:
> >  unlock:
> >  	KERNEL_UNLOCK();
> >  	solock(so);
> > -	unp->unp_flags &= ~UNP_CONNECTING;
> > +	unp->unp_flags &= ~UNP_BUSY;
> >  
> >  	/*
> >  	 * The peer socket could be closed by concurrent thread
> > Index: sys/sys/unpcb.h
> > ===================================================================
> > RCS file: /cvs/src/sys/sys/unpcb.h,v
> > retrieving revision 1.45
> > diff -u -p -r1.45 unpcb.h
> > --- sys/sys/unpcb.h	26 Nov 2022 17:51:18 -0000	1.45
> > +++ sys/sys/unpcb.h	19 Mar 2024 10:37:21 -0000
> > @@ -91,8 +91,7 @@ struct	unpcb {
> >   */
> >  #define UNP_FEIDS	0x01		/* unp_connid contains information */
> >  #define UNP_FEIDSBIND	0x02		/* unp_connid was set by a bind */
> > -#define UNP_BINDING	0x04		/* unp is binding now */
> > -#define UNP_CONNECTING	0x08		/* unp is connecting now */
> > +#define UNP_BUSY	0x04		/* unp is binding or connecting now */
> >  
> >  /*
> >   * flag bits in unp_gcflags