On 23/01/17(Mon) 18:45, Philip Guenther wrote:
> On Mon, Jan 23, 2017 at 4:01 PM, Martin Pieuchot <[email protected]> wrote:
> [...]
> > @@ -277,19 +277,35 @@ doaccept(struct proc *p, int sock, struc
> > return (error);
> >
> > headfp = fp;
> > + head = headfp->f_data;
> > +
> > + fdplock(fdp);
> > + error = falloc(p, &fp, &tmpfd);
> > + if (!error && (flags & SOCK_CLOEXEC))
> > + fdp->fd_ofileflags[tmpfd] |= UF_EXCLOSE;
> > + fdpunlock(fdp);
> > + if (error) {
> > + /*
> > + * Probably ran out of file descriptors. Wakeup
> > + * so some other process might have a chance at it.
> > + */
> > + wakeup_one(&head->so_timeo);
>
> This comment and wakeup_one() call should be left out. They were
> needed in the original location because the thread had consumed the
> original wakeup from the new connection being completed at that point
> and when it gave up (because falloc() failed) it needed to recreate
> that for the next process. Now, doing the falloc() before we don't
> need or want that extra wakeup.
>
> Otherwise, looks good.
Thanks, I'm going to commit the version below then.
Index: kern/uipc_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.144
diff -u -p -r1.144 uipc_syscalls.c
--- kern/uipc_syscalls.c 29 Dec 2016 12:12:43 -0000 1.144
+++ kern/uipc_syscalls.c 24 Jan 2017 03:39:56 -0000
@@ -265,7 +265,7 @@ doaccept(struct proc *p, int sock, struc
{
struct filedesc *fdp = p->p_fd;
struct file *fp, *headfp;
- struct mbuf *nam;
+ struct mbuf *nam = NULL;
socklen_t namelen;
int error, s, tmpfd;
struct socket *head, *so;
@@ -277,19 +277,30 @@ doaccept(struct proc *p, int sock, struc
return (error);
headfp = fp;
+
+ fdplock(fdp);
+ error = falloc(p, &fp, &tmpfd);
+ if (!error && (flags & SOCK_CLOEXEC))
+ fdp->fd_ofileflags[tmpfd] |= UF_EXCLOSE;
+ fdpunlock(fdp);
+ if (error) {
+ FRELE(headfp, p);
+ return (error);
+ }
+
redo:
NET_LOCK(s);
head = headfp->f_data;
if (isdnssocket(head) || (head->so_options & SO_ACCEPTCONN) == 0) {
error = EINVAL;
- goto bad;
+ goto out;
}
if ((head->so_state & SS_NBIO) && head->so_qlen == 0) {
if (head->so_state & SS_CANTRCVMORE)
error = ECONNABORTED;
else
error = EWOULDBLOCK;
- goto bad;
+ goto out;
}
while (head->so_qlen == 0 && head->so_error == 0) {
if (head->so_state & SS_CANTRCVMORE) {
@@ -298,34 +309,19 @@ redo:
}
error = tsleep(&head->so_timeo, PSOCK | PCATCH,
"netcon", 0);
- if (error) {
- goto bad;
- }
+ if (error)
+ goto out;
}
if (head->so_error) {
error = head->so_error;
head->so_error = 0;
- goto bad;
+ goto out;
}
/* Figure out whether the new socket should be non-blocking. */
nflag = flags & SOCK_NONBLOCK_INHERIT ? (headfp->f_flag & FNONBLOCK)
: (flags & SOCK_NONBLOCK ? FNONBLOCK : 0);
- fdplock(fdp);
- error = falloc(p, &fp, &tmpfd);
- if (error == 0 && (flags & SOCK_CLOEXEC))
- fdp->fd_ofileflags[tmpfd] |= UF_EXCLOSE;
- fdpunlock(fdp);
- if (error != 0) {
- /*
- * Probably ran out of file descriptors. Wakeup
- * so some other process might have a chance at it.
- */
- wakeup_one(&head->so_timeo);
- goto bad;
- }
-
nam = m_get(M_WAIT, MT_SONAME);
/*
@@ -336,10 +332,7 @@ redo:
if (head->so_qlen == 0) {
NET_UNLOCK(s);
m_freem(nam);
- fdplock(fdp);
- fdremove(fdp, tmpfd);
- closef(fp, p);
- fdpunlock(fdp);
+ nam = NULL;
goto redo;
}
@@ -360,24 +353,20 @@ redo:
error = soaccept(so, nam);
if (!error && name != NULL)
error = copyaddrout(p, nam, name, namelen, anamelen);
-
+ if (!error) {
+ (*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t)&nflag, p);
+ FILE_SET_MATURE(fp, p);
+ *retval = tmpfd;
+ }
+out:
+ NET_UNLOCK(s);
+ m_freem(nam);
if (error) {
- /* if an error occurred, free the file descriptor */
- NET_UNLOCK(s);
- m_freem(nam);
fdplock(fdp);
fdremove(fdp, tmpfd);
closef(fp, p);
fdpunlock(fdp);
- goto out;
}
- (*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t)&nflag, p);
- FILE_SET_MATURE(fp, p);
- *retval = tmpfd;
- m_freem(nam);
-bad:
- NET_UNLOCK(s);
-out:
FRELE(headfp, p);
return (error);
}
