On 05/12/17(Tue) 14:37, Alexander Bluhm wrote: > On Mon, Dec 04, 2017 at 03:33:56PM +0100, Martin Pieuchot wrote: > > Diff below change the usage of `sb_flags' and `sb_flagsintr'. The > > former will be protected by the socket lock while the latter will > > be using atomic operations. > > I like this plan. > > > @@ -381,17 +381,18 @@ sbunlock(struct socket *so, struct sockb > > void > > sowakeup(struct socket *so, struct sockbuf *sb) > > { > > - KERNEL_ASSERT_LOCKED(); > > soassertlocked(so); > > > > - selwakeup(&sb->sb_sel); > > - sb->sb_flagsintr &= ~SB_SEL; > > - if (sb->sb_flagsintr & SB_WAIT) { > > - sb->sb_flagsintr &= ~SB_WAIT; > > + sb->sb_flags &= ~SB_SEL; > > + if (sb->sb_flags & SB_WAIT) { > > + sb->sb_flags &= ~SB_WAIT; > > wakeup(&sb->sb_cc); > > } > > + KERNEL_LOCK(); > > if (so->so_state & SS_ASYNC) > > csignal(so->so_pgid, SIGIO, so->so_siguid, so->so_sigeuid); > > + selwakeup(&sb->sb_sel); > > + KERNEL_UNLOCK(); > > } > > Why do you move the selwakeup() after the csignal() ?
To make it similar to sohasoutofband(). > > > @@ -340,6 +342,7 @@ fifo_poll(void *v) > > wso->so_snd.sb_flagsintr |= SB_SEL; > > } > > } > > + sounlock(s); > > return (revents); > > } > > Here you missed to convert sb_flagsintr |= SB_SEL to sb_flags. > > And in filt_fifowdetach() is still a sb_flags &= ~SB_KNOTE. Right, updated diff below. Index: kern/uipc_socket.c =================================================================== RCS file: /cvs/src/sys/kern/uipc_socket.c,v retrieving revision 1.209 diff -u -p -r1.209 uipc_socket.c --- kern/uipc_socket.c 23 Nov 2017 13:45:46 -0000 1.209 +++ kern/uipc_socket.c 4 Dec 2017 14:26:24 -0000 @@ -1054,9 +1054,9 @@ sorflush(struct socket *so) aso.so_rcv = *sb; memset(sb, 0, sizeof (*sb)); /* XXX - the memset stomps all over so_rcv */ - if (aso.so_rcv.sb_flags & SB_KNOTE) { + if (aso.so_rcv.sb_flagsintr & SB_KNOTE) { sb->sb_sel.si_note = aso.so_rcv.sb_sel.si_note; - sb->sb_flags = SB_KNOTE; + sb->sb_flagsintr = SB_KNOTE; } if (pr->pr_flags & PR_RIGHTS && pr->pr_domain->dom_dispose) (*pr->pr_domain->dom_dispose)(aso.so_rcv.sb_mb); @@ -1178,8 +1178,8 @@ sosplice(struct socket *so, int fd, off_ * we sleep, the socket buffers are not marked as spliced yet. */ if (somove(so, M_WAIT)) { - so->so_rcv.sb_flagsintr |= SB_SPLICE; - sosp->so_snd.sb_flagsintr |= SB_SPLICE; + so->so_rcv.sb_flags |= SB_SPLICE; + sosp->so_snd.sb_flags |= SB_SPLICE; } release: @@ -1196,8 +1196,8 @@ sounsplice(struct socket *so, struct soc task_del(sosplice_taskq, &so->so_splicetask); timeout_del(&so->so_idleto); - sosp->so_snd.sb_flagsintr &= ~SB_SPLICE; - so->so_rcv.sb_flagsintr &= ~SB_SPLICE; + sosp->so_snd.sb_flags &= ~SB_SPLICE; + so->so_rcv.sb_flags &= ~SB_SPLICE; so->so_sp->ssp_socket = sosp->so_sp->ssp_soback = NULL; if (wakeup && soreadable(so)) sorwakeup(so); @@ -1210,7 +1210,7 @@ soidle(void *arg) int s; s = solock(so); - if (so->so_rcv.sb_flagsintr & SB_SPLICE) { + if (so->so_rcv.sb_flags & SB_SPLICE) { so->so_error = ETIMEDOUT; sounsplice(so, so->so_sp->ssp_socket, 1); } @@ -1224,7 +1224,7 @@ sotask(void *arg) int s; s = solock(so); - if (so->so_rcv.sb_flagsintr & SB_SPLICE) { + if (so->so_rcv.sb_flags & SB_SPLICE) { /* * We may not sleep here as sofree() and unsplice() may be * called from softnet interrupt context. This would remove @@ -1527,7 +1527,7 @@ sorwakeup(struct socket *so) soassertlocked(so); #ifdef SOCKET_SPLICE - if (so->so_rcv.sb_flagsintr & SB_SPLICE) { + if (so->so_rcv.sb_flags & SB_SPLICE) { /* * TCP has a sendbuffer that can handle multiple packets * at once. So queue the stream a bit to accumulate data. @@ -1555,7 +1555,7 @@ sowwakeup(struct socket *so) soassertlocked(so); #ifdef SOCKET_SPLICE - if (so->so_snd.sb_flagsintr & SB_SPLICE) + if (so->so_snd.sb_flags & SB_SPLICE) task_add(sosplice_taskq, &so->so_sp->ssp_soback->so_splicetask); #endif sowakeup(so, &so->so_snd); @@ -1871,9 +1871,10 @@ sogetopt(struct socket *so, int level, i void sohasoutofband(struct socket *so) { - KERNEL_ASSERT_LOCKED(); + KERNEL_LOCK(); csignal(so->so_pgid, SIGURG, so->so_siguid, so->so_sigeuid); selwakeup(&so->so_rcv.sb_sel); + KERNEL_UNLOCK(); } int @@ -1901,7 +1902,7 @@ soo_kqfilter(struct file *fp, struct kno } SLIST_INSERT_HEAD(&sb->sb_sel.si_note, kn, kn_selnext); - sb->sb_flags |= SB_KNOTE; + sb->sb_flagsintr |= SB_KNOTE; return (0); } @@ -1915,7 +1916,7 @@ filt_sordetach(struct knote *kn) SLIST_REMOVE(&so->so_rcv.sb_sel.si_note, kn, knote, kn_selnext); if (SLIST_EMPTY(&so->so_rcv.sb_sel.si_note)) - so->so_rcv.sb_flags &= ~SB_KNOTE; + so->so_rcv.sb_flagsintr &= ~SB_KNOTE; } int @@ -1958,7 +1959,7 @@ filt_sowdetach(struct knote *kn) SLIST_REMOVE(&so->so_snd.sb_sel.si_note, kn, knote, kn_selnext); if (SLIST_EMPTY(&so->so_snd.sb_sel.si_note)) - so->so_snd.sb_flags &= ~SB_KNOTE; + so->so_snd.sb_flagsintr &= ~SB_KNOTE; } int Index: kern/sys_socket.c =================================================================== RCS file: /cvs/src/sys/kern/sys_socket.c,v retrieving revision 1.34 diff -u -p -r1.34 sys_socket.c --- kern/sys_socket.c 14 Nov 2017 16:01:55 -0000 1.34 +++ kern/sys_socket.c 4 Dec 2017 14:20:55 -0000 @@ -166,11 +166,11 @@ soo_poll(struct file *fp, int events, st if (revents == 0) { if (events & (POLLIN | POLLPRI | POLLRDNORM | POLLRDBAND)) { selrecord(p, &so->so_rcv.sb_sel); - so->so_rcv.sb_flagsintr |= SB_SEL; + so->so_rcv.sb_flags |= SB_SEL; } if (events & (POLLOUT | POLLWRNORM)) { selrecord(p, &so->so_snd.sb_sel); - so->so_snd.sb_flagsintr |= SB_SEL; + so->so_snd.sb_flags |= SB_SEL; } } sounlock(s); Index: kern/uipc_socket2.c =================================================================== RCS file: /cvs/src/sys/kern/uipc_socket2.c,v retrieving revision 1.87 diff -u -p -r1.87 uipc_socket2.c --- kern/uipc_socket2.c 23 Nov 2017 13:42:53 -0000 1.87 +++ kern/uipc_socket2.c 4 Dec 2017 14:22:21 -0000 @@ -329,12 +329,12 @@ sosleep(struct socket *so, void *ident, int sbwait(struct socket *so, struct sockbuf *sb) { + int prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH; + soassertlocked(so); - sb->sb_flagsintr |= SB_WAIT; - return (sosleep(so, &sb->sb_cc, - (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH, "netio", - sb->sb_timeo)); + sb->sb_flags |= SB_WAIT; + return (sosleep(so, &sb->sb_cc, prio, "netio", sb->sb_timeo)); } int @@ -381,17 +381,18 @@ sbunlock(struct socket *so, struct sockb void sowakeup(struct socket *so, struct sockbuf *sb) { - KERNEL_ASSERT_LOCKED(); soassertlocked(so); - selwakeup(&sb->sb_sel); - sb->sb_flagsintr &= ~SB_SEL; - if (sb->sb_flagsintr & SB_WAIT) { - sb->sb_flagsintr &= ~SB_WAIT; + sb->sb_flags &= ~SB_SEL; + if (sb->sb_flags & SB_WAIT) { + sb->sb_flags &= ~SB_WAIT; wakeup(&sb->sb_cc); } + KERNEL_LOCK(); if (so->so_state & SS_ASYNC) csignal(so->so_pgid, SIGIO, so->so_siguid, so->so_sigeuid); + selwakeup(&sb->sb_sel); + KERNEL_UNLOCK(); } /* Index: miscfs/fifofs/fifo_vnops.c =================================================================== RCS file: /cvs/src/sys/miscfs/fifofs/fifo_vnops.c,v retrieving revision 1.59 diff -u -p -r1.59 fifo_vnops.c --- miscfs/fifofs/fifo_vnops.c 4 Nov 2017 14:13:53 -0000 1.59 +++ miscfs/fifofs/fifo_vnops.c 7 Dec 2017 12:31:07 -0000 @@ -307,10 +307,12 @@ fifo_poll(void *v) struct socket *wso = ap->a_vp->v_fifoinfo->fi_writesock; int events = 0; int revents = 0; + int s; /* * FIFOs don't support out-of-band or high priority data. */ + s = solock(rso); if (ap->a_fflag & FREAD) events |= ap->a_events & (POLLIN | POLLRDNORM); if (ap->a_fflag & FWRITE) @@ -333,13 +335,14 @@ fifo_poll(void *v) events = POLLIN; if (events & (POLLIN | POLLRDNORM)) { selrecord(ap->a_p, &rso->so_rcv.sb_sel); - rso->so_rcv.sb_flagsintr |= SB_SEL; + rso->so_rcv.sb_flags |= SB_SEL; } if (events & (POLLOUT | POLLWRNORM)) { selrecord(ap->a_p, &wso->so_snd.sb_sel); - wso->so_snd.sb_flagsintr |= SB_SEL; + wso->so_snd.sb_flags |= SB_SEL; } } + sounlock(s); return (revents); } @@ -526,7 +529,7 @@ fifo_kqfilter(void *v) ap->a_kn->kn_hook = so; SLIST_INSERT_HEAD(&sb->sb_sel.si_note, ap->a_kn, kn_selnext); - sb->sb_flags |= SB_KNOTE; + sb->sb_flagsintr |= SB_KNOTE; return (0); } @@ -538,7 +541,7 @@ filt_fifordetach(struct knote *kn) SLIST_REMOVE(&so->so_rcv.sb_sel.si_note, kn, knote, kn_selnext); if (SLIST_EMPTY(&so->so_rcv.sb_sel.si_note)) - so->so_rcv.sb_flags &= ~SB_KNOTE; + so->so_rcv.sb_flagsintr &= ~SB_KNOTE; } int @@ -570,7 +573,7 @@ filt_fifowdetach(struct knote *kn) SLIST_REMOVE(&so->so_snd.sb_sel.si_note, kn, knote, kn_selnext); if (SLIST_EMPTY(&so->so_snd.sb_sel.si_note)) - so->so_snd.sb_flags &= ~SB_KNOTE; + so->so_snd.sb_flagsintr &= ~SB_KNOTE; } int