On 12/07/18(Thu) 17:53, Alexander Bluhm wrote: > On Thu, Jul 12, 2018 at 02:38:48PM +0200, Martin Pieuchot wrote: > > @@ -83,12 +91,6 @@ soo_ioctl(struct file *fp, u_long cmd, c > > switch (cmd) { > > > > case FIONBIO: > > - s = solock(so); > > - if (*(int *)data) > > - so->so_state |= SS_NBIO; > > - else > > - so->so_state &= ~SS_NBIO; > > - sounlock(so, s); > > break; > > man 2 ioctl documents FIONBIO. Should we set or clear FNONBLOCK > instead?
That's already done by the upper layer. After this diff we can kill a bunch of pseudo FIONBIO handlers. > > @@ -1157,8 +1154,7 @@ sosplice(struct socket *so, int fd, off_ > > } > > > > /* Lock both receive and send buffer. */ > > - if ((error = sblock(so, &so->so_rcv, > > - (so->so_state & SS_NBIO) ? M_NOWAIT : M_WAITOK)) != 0) { > > + if ((error = sblock(so, &so->so_rcv, M_WAITOK)) != 0) { > > goto frele; > > } > > if ((error = sblock(so, &sosp->so_snd, M_WAITOK)) != 0) { > > I have copied the logic from soreceive. So I think sosplice should > also use FNONBLOCK. That would mean to pass the f_flag to sosetopt(). I'm not fan of this plumbing. So unless you can explain why M_NOWAIT is needed here I don't see why we should have it. Do you have any code checking for EWOULDBLOCK after calling setsockopt() with SO_SPLICE? Plus since we have a rwlock as socket lock, most of this logic isn't relevant. > > @@ -223,12 +223,11 @@ fifo_read(void *v) > > if (uio->uio_resid == 0) > > return (0); > > if (ap->a_ioflag & IO_NDELAY) > > - rso->so_state |= SS_NBIO; > > + flags = MSG_DONTWAIT; > > I think a |= would be nicer here. Sure. > > @@ -384,8 +380,8 @@ fifo_close(void *v) > > } > > } > > if (fip->fi_readers == 0 && fip->fi_writers == 0) { > > - error1 = soclose(fip->fi_readsock); > > - error2 = soclose(fip->fi_writesock); > > + error1 = soclose(fip->fi_readsock, MSG_DONTWAIT); > > + error2 = soclose(fip->fi_writesock, MSG_DONTWAIT); > > free(fip, M_VNODE, sizeof *fip); > > vp->v_fifoinfo = NULL; > > } > > You are adding MSG_DONTWAIT to a bunch of closes. Are you sure > that you do not change behavior here? Does a blocking fifo close > with SO_LINGER wait for the other end? > [...] > NFS never sets SS_NBIO, so I think we should not use MSG_DONTWAIT. Right, '0' is what we want here. Updated diff below. Index: sys/kern/subr_log.c =================================================================== RCS file: /cvs/src/sys/kern/subr_log.c,v retrieving revision 1.55 diff -u -p -r1.55 subr_log.c --- sys/kern/subr_log.c 19 Feb 2018 08:59:52 -0000 1.55 +++ sys/kern/subr_log.c 12 Jul 2018 11:39:15 -0000 @@ -51,6 +51,7 @@ #include <sys/filedesc.h> #include <sys/socket.h> #include <sys/socketvar.h> +#include <sys/fcntl.h> #ifdef KTRACE #include <sys/ktrace.h> @@ -480,7 +481,8 @@ dosendsyslog(struct proc *p, const char len = auio.uio_resid; if (fp) { - error = sosend(fp->f_data, NULL, &auio, NULL, NULL, 0); + int flags = (fp->f_flag & FNONBLOCK) ? MSG_DONTWAIT : 0; + error = sosend(fp->f_data, NULL, &auio, NULL, NULL, flags); if (error == 0) len -= auio.uio_resid; } else if (constty || cn_devvp) { Index: sys/kern/sys_socket.c =================================================================== RCS file: /cvs/src/sys/kern/sys_socket.c,v retrieving revision 1.39 diff -u -p -r1.39 sys_socket.c --- sys/kern/sys_socket.c 10 Jul 2018 08:58:50 -0000 1.39 +++ sys/kern/sys_socket.c 12 Jul 2018 12:20:49 -0000 @@ -43,6 +43,7 @@ #include <sys/ioctl.h> #include <sys/poll.h> #include <sys/stat.h> +#include <sys/fcntl.h> #include <net/if.h> #include <net/route.h> @@ -60,18 +61,25 @@ struct fileops socketops = { int soo_read(struct file *fp, off_t *poff, struct uio *uio, struct ucred *cred) { + struct socket *so = (struct socket *)fp->f_data; + int flags = 0; + + if (fp->f_flag & FNONBLOCK) + flags |= MSG_DONTWAIT; - return (soreceive((struct socket *)fp->f_data, (struct mbuf **)NULL, - uio, (struct mbuf **)NULL, (struct mbuf **)NULL, (int *)NULL, - (socklen_t)0)); + return (soreceive(so, NULL, uio, NULL, NULL, &flags, 0)); } int soo_write(struct file *fp, off_t *poff, struct uio *uio, struct ucred *cred) { + struct socket *so = (struct socket *)fp->f_data; + int flags = 0; + + if (fp->f_flag & FNONBLOCK) + flags |= MSG_DONTWAIT; - return (sosend((struct socket *)fp->f_data, (struct mbuf *)NULL, - uio, (struct mbuf *)NULL, (struct mbuf *)NULL, 0)); + return (sosend(so, NULL, uio, NULL, NULL, flags)); } int @@ -83,12 +91,6 @@ soo_ioctl(struct file *fp, u_long cmd, c switch (cmd) { case FIONBIO: - s = solock(so); - if (*(int *)data) - so->so_state |= SS_NBIO; - else - so->so_state &= ~SS_NBIO; - sounlock(so, s); break; case FIOASYNC: @@ -210,10 +212,12 @@ soo_stat(struct file *fp, struct stat *u int soo_close(struct file *fp, struct proc *p) { - int error = 0; + int flags, error = 0; - if (fp->f_data) - error = soclose(fp->f_data); + if (fp->f_data) { + flags = (fp->f_flag & FNONBLOCK) ? MSG_DONTWAIT : 0; + error = soclose(fp->f_data, flags); + } fp->f_data = 0; return (error); } Index: sys/kern/uipc_socket.c =================================================================== RCS file: /cvs/src/sys/kern/uipc_socket.c,v retrieving revision 1.225 diff -u -p -r1.225 uipc_socket.c --- sys/kern/uipc_socket.c 5 Jul 2018 14:45:07 -0000 1.225 +++ sys/kern/uipc_socket.c 12 Jul 2018 12:21:49 -0000 @@ -244,7 +244,7 @@ sofree(struct socket *so, int s) * Free socket when disconnect complete. */ int -soclose(struct socket *so) +soclose(struct socket *so, int flags) { struct socket *so2; int s, error = 0; @@ -270,7 +270,7 @@ soclose(struct socket *so) } if (so->so_options & SO_LINGER) { if ((so->so_state & SS_ISDISCONNECTING) && - (so->so_state & SS_NBIO)) + (flags & MSG_DONTWAIT)) goto drop; while (so->so_state & SS_ISCONNECTED) { error = sosleep(so, &so->so_timeo, @@ -469,7 +469,7 @@ restart: if (space < clen || (space - clen < resid && (atomic || space < so->so_snd.sb_lowat))) { - if ((so->so_state & SS_NBIO) || (flags & MSG_DONTWAIT)) + if (flags & MSG_DONTWAIT) snderr(EWOULDBLOCK); sbunlock(so, &so->so_snd); error = sbwait(so, &so->so_snd); @@ -664,8 +664,6 @@ soreceive(struct socket *so, struct mbuf flags = *flagsp &~ MSG_EOR; else flags = 0; - if (so->so_state & SS_NBIO) - flags |= MSG_DONTWAIT; if (flags & MSG_OOB) { m = m_get(M_WAIT, MT_DATA); s = solock(so); @@ -748,7 +746,7 @@ restart: } if (uio->uio_resid == 0 && controlp == NULL) goto release; - if ((so->so_state & SS_NBIO) || (flags & MSG_DONTWAIT)) { + if (flags & MSG_DONTWAIT) { error = EWOULDBLOCK; goto release; } @@ -1128,8 +1126,7 @@ sosplice(struct socket *so, int fd, off_ /* If no fd is given, unsplice by removing existing link. */ if (fd < 0) { /* Lock receive buffer. */ - if ((error = sblock(so, &so->so_rcv, - (so->so_state & SS_NBIO) ? M_NOWAIT : M_WAITOK)) != 0) { + if ((error = sblock(so, &so->so_rcv, M_WAITOK)) != 0) { return (error); } if (so->so_sp->ssp_socket) @@ -1157,8 +1154,7 @@ sosplice(struct socket *so, int fd, off_ } /* Lock both receive and send buffer. */ - if ((error = sblock(so, &so->so_rcv, - (so->so_state & SS_NBIO) ? M_NOWAIT : M_WAITOK)) != 0) { + if ((error = sblock(so, &so->so_rcv, M_WAITOK)) != 0) { goto frele; } if ((error = sblock(so, &sosp->so_snd, M_WAITOK)) != 0) { Index: sys/kern/uipc_syscalls.c =================================================================== RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v retrieving revision 1.177 diff -u -p -r1.177 uipc_syscalls.c --- sys/kern/uipc_syscalls.c 20 Jun 2018 10:52:49 -0000 1.177 +++ sys/kern/uipc_syscalls.c 12 Jul 2018 12:18:33 -0000 @@ -109,13 +109,11 @@ sys_socket(struct proc *p, void *v, regi error = falloc(p, &fp, &fd); if (error) { fdpunlock(fdp); - soclose(so); + soclose(so, MSG_DONTWAIT); } else { fp->f_flag = fflag; fp->f_type = DTYPE_SOCKET; fp->f_ops = &socketops; - if (nonblock) - so->so_state |= SS_NBIO; so->so_state |= ss; fp->f_data = so; fdinsert(fdp, fd, cloexec, fp); @@ -302,7 +300,7 @@ doaccept(struct proc *p, int sock, struc error = EINVAL; goto out; } - if ((head->so_state & SS_NBIO) && head->so_qlen == 0) { + if ((headfp->f_flag & FNONBLOCK) && head->so_qlen == 0) { if (head->so_state & SS_CANTRCVMORE) error = ECONNABORTED; else @@ -347,10 +345,6 @@ doaccept(struct proc *p, int sock, struc error = copyaddrout(p, nam, name, namelen, anamelen); out: if (!error) { - if (nflag & FNONBLOCK) - so->so_state |= SS_NBIO; - else - so->so_state &= ~SS_NBIO; sounlock(head, s); fdplock(fdp); fp->f_data = so; @@ -416,7 +410,7 @@ sys_connect(struct proc *p, void *v, reg error = soconnect(so, nam); if (error) goto bad; - if ((so->so_state & SS_NBIO) && (so->so_state & SS_ISCONNECTING)) { + if ((fp->f_flag & FNONBLOCK) && (so->so_state & SS_ISCONNECTING)) { error = EINPROGRESS; goto out; } @@ -503,12 +497,6 @@ sys_socketpair(struct proc *p, void *v, if (KTRPOINT(p, KTR_STRUCT)) ktrfds(p, sv, 2); #endif - if (nonblock) { - (*fp1->f_ops->fo_ioctl)(fp1, FIONBIO, (caddr_t)&type, - p); - (*fp2->f_ops->fo_ioctl)(fp2, FIONBIO, (caddr_t)&type, - p); - } fdinsert(fdp, sv[0], cloexec, fp1); fdinsert(fdp, sv[1], cloexec, fp2); fdpunlock(fdp); @@ -529,10 +517,10 @@ free3: KERNEL_UNLOCK(); free2: if (so2 != NULL) - (void)soclose(so2); + (void)soclose(so2, 0); free1: if (so1 != NULL) - (void)soclose(so1); + (void)soclose(so1, 0); return (error); } @@ -626,6 +614,8 @@ sendit(struct proc *p, int s, struct msg if ((error = getsock(p, s, &fp)) != 0) return (error); so = fp->f_data; + if (fp->f_flag & FNONBLOCK) + flags |= MSG_DONTWAIT; error = pledge_sendit(p, mp->msg_name); if (error) @@ -849,6 +839,8 @@ recvit(struct proc *p, int s, struct msg } #endif len = auio.uio_resid; + if (fp->f_flag & FNONBLOCK) + mp->msg_flags |= MSG_DONTWAIT; error = soreceive(fp->f_data, &from, &auio, NULL, mp->msg_control ? &control : NULL, &mp->msg_flags, Index: sys/net/bfd.c =================================================================== RCS file: /cvs/src/sys/net/bfd.c,v retrieving revision 1.71 diff -u -p -r1.71 bfd.c --- sys/net/bfd.c 6 Jun 2018 06:55:22 -0000 1.71 +++ sys/net/bfd.c 12 Jul 2018 12:19:31 -0000 @@ -250,14 +250,14 @@ bfd_clear_task(void *arg) if (bfd->bc_so) { /* remove upcall before calling soclose or it will be called */ bfd->bc_so->so_upcall = NULL; - soclose(bfd->bc_so); + soclose(bfd->bc_so, MSG_DONTWAIT); } if (bfd->bc_soecho) { bfd->bc_soecho->so_upcall = NULL; - soclose(bfd->bc_soecho); + soclose(bfd->bc_soecho, MSG_DONTWAIT); } if (bfd->bc_sosend) - soclose(bfd->bc_sosend); + soclose(bfd->bc_sosend, MSG_DONTWAIT); rtfree(bfd->bc_rt); bfd->bc_rt = NULL; @@ -495,7 +495,7 @@ bfd_listener(struct bfd_config *bfd, uns close: m_free(m); - soclose(so); + soclose(so, MSG_DONTWAIT); return (NULL); } @@ -624,7 +624,7 @@ bfd_sender(struct bfd_config *bfd, unsig close: m_free(m); - soclose(so); + soclose(so, MSG_DONTWAIT); return (NULL); } Index: sys/net/if_pflow.c =================================================================== RCS file: /cvs/src/sys/net/if_pflow.c,v retrieving revision 1.88 diff -u -p -r1.88 if_pflow.c --- sys/net/if_pflow.c 6 Jun 2018 06:55:22 -0000 1.88 +++ sys/net/if_pflow.c 12 Jul 2018 12:19:44 -0000 @@ -286,7 +286,7 @@ pflow_clone_destroy(struct ifnet *ifp) mq_purge(&sc->sc_outputqueue); m_freem(sc->send_nam); if (sc->so != NULL) { - error = soclose(sc->so); + error = soclose(sc->so, MSG_DONTWAIT); sc->so = NULL; } if (sc->sc_flowdst != NULL) @@ -349,7 +349,7 @@ pflow_set(struct pflow_softc *sc, struct free(sc->sc_flowdst, M_DEVBUF, sc->sc_flowdst->sa_len); sc->sc_flowdst = NULL; if (sc->so != NULL) { - soclose(sc->so); + soclose(sc->so, MSG_DONTWAIT); sc->so = NULL; } } @@ -395,7 +395,7 @@ pflow_set(struct pflow_softc *sc, struct free(sc->sc_flowsrc, M_DEVBUF, sc->sc_flowsrc->sa_len); sc->sc_flowsrc = NULL; if (sc->so != NULL) { - soclose(sc->so); + soclose(sc->so, MSG_DONTWAIT); sc->so = NULL; } switch(pflowr->flowsrc.ss_family) { @@ -445,14 +445,14 @@ pflow_set(struct pflow_softc *sc, struct sounlock(so, s); m_freem(m); if (error) { - soclose(so); + soclose(so, MSG_DONTWAIT); return (error); } } sc->so = so; } } else if (!pflowvalidsockaddr(sc->sc_flowdst, 0)) { - soclose(sc->so); + soclose(sc->so, MSG_DONTWAIT); sc->so = NULL; } Index: sys/miscfs/fifofs/fifo_vnops.c =================================================================== RCS file: /cvs/src/sys/miscfs/fifofs/fifo_vnops.c,v retrieving revision 1.67 diff -u -p -r1.67 fifo_vnops.c --- sys/miscfs/fifofs/fifo_vnops.c 6 Jun 2018 06:55:22 -0000 1.67 +++ sys/miscfs/fifofs/fifo_vnops.c 12 Jul 2018 16:15:58 -0000 @@ -136,15 +136,15 @@ fifo_open(void *v) } fip->fi_readsock = rso; if ((error = socreate(AF_UNIX, &wso, SOCK_STREAM, 0)) != 0) { - (void)soclose(rso); + (void)soclose(rso, 0); free(fip, M_VNODE, sizeof *fip); vp->v_fifoinfo = NULL; return (error); } fip->fi_writesock = wso; if ((error = soconnect2(wso, rso)) != 0) { - (void)soclose(wso); - (void)soclose(rso); + (void)soclose(wso, 0); + (void)soclose(rso, 0); free(fip, M_VNODE, sizeof *fip); vp->v_fifoinfo = NULL; return (error); @@ -214,7 +214,7 @@ fifo_read(void *v) struct vop_read_args *ap = v; struct uio *uio = ap->a_uio; struct socket *rso = ap->a_vp->v_fifoinfo->fi_readsock; - int error; + int error, flags = 0; #ifdef DIAGNOSTIC if (uio->uio_rw != UIO_READ) @@ -223,12 +223,11 @@ fifo_read(void *v) if (uio->uio_resid == 0) return (0); if (ap->a_ioflag & IO_NDELAY) - rso->so_state |= SS_NBIO; + flags |= MSG_DONTWAIT; VOP_UNLOCK(ap->a_vp); - error = soreceive(rso, NULL, uio, NULL, NULL, NULL, 0); + error = soreceive(rso, NULL, uio, NULL, NULL, &flags, 0); vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY); if (ap->a_ioflag & IO_NDELAY) { - rso->so_state &= ~SS_NBIO; if (error == EWOULDBLOCK && ap->a_vp->v_fifoinfo->fi_writers == 0) error = 0; @@ -245,20 +244,17 @@ fifo_write(void *v) { struct vop_write_args *ap = v; struct socket *wso = ap->a_vp->v_fifoinfo->fi_writesock; - int error; + int error, flags = 0; #ifdef DIAGNOSTIC if (ap->a_uio->uio_rw != UIO_WRITE) panic("fifo_write mode"); #endif - /* XXXSMP changing state w/o lock isn't safe. */ if (ap->a_ioflag & IO_NDELAY) - wso->so_state |= SS_NBIO; + flags |= MSG_DONTWAIT; VOP_UNLOCK(ap->a_vp); - error = sosend(wso, NULL, ap->a_uio, NULL, NULL, 0); + error = sosend(wso, NULL, ap->a_uio, NULL, NULL, flags); vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY); - if (ap->a_ioflag & IO_NDELAY) - wso->so_state &= ~SS_NBIO; return (error); } @@ -384,8 +380,8 @@ fifo_close(void *v) } } if (fip->fi_readers == 0 && fip->fi_writers == 0) { - error1 = soclose(fip->fi_readsock); - error2 = soclose(fip->fi_writesock); + error1 = soclose(fip->fi_readsock, 0); + error2 = soclose(fip->fi_writesock, 0); free(fip, M_VNODE, sizeof *fip); vp->v_fifoinfo = NULL; } @@ -402,8 +398,8 @@ fifo_reclaim(void *v) if (fip == NULL) return (0); - soclose(fip->fi_readsock); - soclose(fip->fi_writesock); + soclose(fip->fi_readsock, 0); + soclose(fip->fi_writesock, 0); free(fip, M_VNODE, sizeof *fip); vp->v_fifoinfo = NULL; Index: sys/nfs/krpc_subr.c =================================================================== RCS file: /cvs/src/sys/nfs/krpc_subr.c,v retrieving revision 1.33 diff -u -p -r1.33 krpc_subr.c --- sys/nfs/krpc_subr.c 6 Jun 2018 06:55:22 -0000 1.33 +++ sys/nfs/krpc_subr.c 12 Jul 2018 16:16:21 -0000 @@ -469,7 +469,7 @@ krpc_call(struct sockaddr_in *sa, u_int m_freem(nam); m_freem(mhead); m_freem(from); - soclose(so); + soclose(so, 0); return error; } Index: sys/nfs/nfs_boot.c =================================================================== RCS file: /cvs/src/sys/nfs/nfs_boot.c,v retrieving revision 1.44 diff -u -p -r1.44 nfs_boot.c --- sys/nfs/nfs_boot.c 14 Nov 2017 16:01:55 -0000 1.44 +++ sys/nfs/nfs_boot.c 12 Jul 2018 16:16:24 -0000 @@ -190,7 +190,7 @@ nfs_boot_init(struct nfs_diskless *nd, s if (error) panic("nfs_boot: set if addr, error=%d", error); - soclose(so); + soclose(so, 0); TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) { if (ifa->ifa_addr->sa_family == AF_INET) Index: sys/nfs/nfs_socket.c =================================================================== RCS file: /cvs/src/sys/nfs/nfs_socket.c,v retrieving revision 1.129 diff -u -p -r1.129 nfs_socket.c --- sys/nfs/nfs_socket.c 6 Jun 2018 06:55:22 -0000 1.129 +++ sys/nfs/nfs_socket.c 12 Jul 2018 16:16:27 -0000 @@ -433,7 +433,7 @@ nfs_disconnect(struct nfsmount *nmp) so = nmp->nm_so; nmp->nm_so = NULL; soshutdown(so, SHUT_RDWR); - soclose(so); + soclose(so, 0); } } Index: sys/sys/socketvar.h =================================================================== RCS file: /cvs/src/sys/sys/socketvar.h,v retrieving revision 1.85 diff -u -p -r1.85 socketvar.h --- sys/sys/socketvar.h 10 Jul 2018 10:02:14 -0000 1.85 +++ sys/sys/socketvar.h 12 Jul 2018 12:20:10 -0000 @@ -148,7 +148,6 @@ struct socket { #define SS_ISDISCONNECTED 0x800 /* socket disconnected from peer */ #define SS_PRIV 0x080 /* privileged for broadcast, raw... */ -#define SS_NBIO 0x100 /* non-blocking ops */ #define SS_ASYNC 0x200 /* async i/o notify */ #define SS_CONNECTOUT 0x1000 /* connect, not accept, at this end */ #define SS_ISSENDING 0x2000 /* hint for lower layer */ @@ -312,7 +311,7 @@ int soaccept(struct socket *so, struct m int sobind(struct socket *so, struct mbuf *nam, struct proc *p); void socantrcvmore(struct socket *so); void socantsendmore(struct socket *so); -int soclose(struct socket *so); +int soclose(struct socket *, int); int soconnect(struct socket *so, struct mbuf *nam); int soconnect2(struct socket *so1, struct socket *so2); int socreate(int dom, struct socket **aso, int type, int proto); Index: share/man/man9/socreate.9 =================================================================== RCS file: /cvs/src/share/man/man9/socreate.9,v retrieving revision 1.9 diff -u -p -r1.9 socreate.9 --- share/man/man9/socreate.9 1 Sep 2017 15:52:03 -0000 1.9 +++ share/man/man9/socreate.9 12 Jul 2018 12:30:59 -0000 @@ -47,7 +47,7 @@ .Ft int .Fn sobind "struct socket *so" "struct mbuf *nam" "struct proc *p" .Ft void -.Fn soclose "struct socket *so" +.Fn soclose "struct socket *so" "int flags" .Ft int .Fn soconnect "struct socket *so" "struct mbuf *nam" .Ft int @@ -307,11 +307,6 @@ The caller may need to manually clear if .Fn soconnect returns an error. -.Pp -The -.Dv MSG_DONTWAIT -flag is not implemented for -.Fn sosend . .Pp This manual page does not describe how to register socket upcalls or monitor a socket for readability/writability without using blocking I/O.