On Thu, Nov 11, 2021 at 12:42:36AM +0300, Vitaliy Makkoveev wrote:
> What about 'SO_DYING' flag? Anyway I want to remove it with the next
> diff.
The so_options are for user flags. The so_state is for kernel
transitions. So a SS_DYING would be better, see sys/socketvar.h.
But as we commit the other soclose() diff first, it does not matter
anymore.
> No problem, but I made "if (vp)" without NULL for the consistency with
> "if (unp->unp_vnode)" in this function. Should they have the same
> notation?
mpi@ and I converted the network stack gradually to the pointer ==
NULL or pointer != NULL idiom. If you change something, go into
this direction. But there is no need to make bulk conversions.
> OK for updated diff?
OK bluhm@ without the SO_DYING part.
> Index: sys/kern/uipc_socket.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.268
> diff -u -p -r1.268 uipc_socket.c
> --- sys/kern/uipc_socket.c 6 Nov 2021 05:26:33 -0000 1.268
> +++ sys/kern/uipc_socket.c 10 Nov 2021 21:33:01 -0000
> @@ -334,6 +334,8 @@ soclose(struct socket *so, int flags)
> /* Revoke async IO early. There is a final revocation in sofree(). */
> sigio_free(&so->so_sigio);
> if (so->so_options & SO_ACCEPTCONN) {
> + so->so_options |= SO_DYING;
> +
> while ((so2 = TAILQ_FIRST(&so->so_q0)) != NULL) {
> (void) soqremque(so2, 0);
> (void) soabort(so2);
> Index: sys/kern/uipc_usrreq.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> retrieving revision 1.154
> diff -u -p -r1.154 uipc_usrreq.c
> --- sys/kern/uipc_usrreq.c 6 Nov 2021 17:35:14 -0000 1.154
> +++ sys/kern/uipc_usrreq.c 10 Nov 2021 21:33:01 -0000
> @@ -485,20 +485,30 @@ void
> unp_detach(struct unpcb *unp)
> {
> struct socket *so = unp->unp_socket;
> - struct vnode *vp = NULL;
> + struct vnode *vp = unp->unp_vnode;
>
> rw_assert_wrlock(&unp_lock);
>
> LIST_REMOVE(unp, unp_link);
> - if (unp->unp_vnode) {
> +
> + if (vp != NULL) {
> + unp->unp_vnode = NULL;
> +
> /*
> - * `v_socket' is only read in unp_connect and
> - * unplock prevents concurrent access.
> + * Enforce `i_lock' -> `unp_lock' because fifo
> + * subsystem requires it.
> */
>
> - unp->unp_vnode->v_socket = NULL;
> - vp = unp->unp_vnode;
> - unp->unp_vnode = NULL;
> + sounlock(so, SL_LOCKED);
> +
> + VOP_LOCK(vp, LK_EXCLUSIVE);
> + vp->v_socket = NULL;
> +
> + KERNEL_LOCK();
> + vput(vp);
> + KERNEL_UNLOCK();
> +
> + solock(so);
> }
>
> if (unp->unp_conn)
> @@ -511,21 +521,6 @@ unp_detach(struct unpcb *unp)
> pool_put(&unpcb_pool, unp);
> if (unp_rights)
> task_add(systqmp, &unp_gc_task);
> -
> - if (vp != NULL) {
> - /*
> - * Enforce `i_lock' -> `unplock' because fifo subsystem
> - * requires it. The socket can't be closed concurrently
> - * because the file descriptor reference is
> - * still hold.
> - */
> -
> - sounlock(so, SL_LOCKED);
> - KERNEL_LOCK();
> - vrele(vp);
> - KERNEL_UNLOCK();
> - solock(so);
> - }
> }
>
> int
> @@ -670,7 +665,8 @@ unp_connect(struct socket *so, struct mb
> goto put_locked;
> }
> if (so->so_proto->pr_flags & PR_CONNREQUIRED) {
> - if ((so2->so_options & SO_ACCEPTCONN) == 0 ||
> + if ((so2->so_options & SO_DYING) != 0 ||
> + (so2->so_options & SO_ACCEPTCONN) == 0 ||
> (so3 = sonewconn(so2, 0)) == NULL) {
> error = ECONNREFUSED;
> goto put_locked;
> Index: sys/sys/socket.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/socket.h,v
> retrieving revision 1.101
> diff -u -p -r1.101 socket.h
> --- sys/sys/socket.h 7 Nov 2021 12:05:28 -0000 1.101
> +++ sys/sys/socket.h 10 Nov 2021 21:33:01 -0000
> @@ -97,6 +97,7 @@ typedef __sa_family_t sa_family_t; /* so
> #define SO_TIMESTAMP 0x0800 /* timestamp received dgram traffic */
> #define SO_BINDANY 0x1000 /* allow bind to any address */
> #define SO_ZEROIZE 0x2000 /* zero out all mbufs sent over socket
> */
> +#define SO_DYING 0x4000 /* socket is dying */
>
> /*
> * Additional options, not kept in so_options.
