Index | Thread | Search

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

Download raw body.

Thread
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.

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