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
