From owner-svn-src-all@FreeBSD.ORG Sun Jan 25 07:24:34 2009 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9A845106567A; Sun, 25 Jan 2009 07:24:34 +0000 (UTC) (envelope-from jeff@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 88D0A8FC1D; Sun, 25 Jan 2009 07:24:34 +0000 (UTC) (envelope-from jeff@FreeBSD.org) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id n0P7OYZj009646; Sun, 25 Jan 2009 07:24:34 GMT (envelope-from jeff@svn.freebsd.org) Received: (from jeff@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id n0P7OYd9009645; Sun, 25 Jan 2009 07:24:34 GMT (envelope-from jeff@svn.freebsd.org) Message-Id: <200901250724.n0P7OYd9009645@svn.freebsd.org> From: Jeff Roberson Date: Sun, 25 Jan 2009 07:24:34 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r187677 - head/sys/kern X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 25 Jan 2009 07:24:35 -0000 Author: jeff Date: Sun Jan 25 07:24:34 2009 New Revision: 187677 URL: http://svn.freebsd.org/changeset/base/187677 Log: Fix errors introduced when I rewrote select. - Restructure selscan() and selrescan() to avoid producing extra selfps when we have a fd in multiple sets. As described below multiple selfps may still exist for other reasons. - Make selrescan() tolerate multiple selfds for a given descriptor set since sockets use two selinfos per fd. If an event on each selinfo fires selrescan() will see the descriptor twice. This could result in select() returning 2x the number of fds actually existing in fd sets. Reported by: mgleason@ncftp.com Modified: head/sys/kern/sys_generic.c Modified: head/sys/kern/sys_generic.c ============================================================================== --- head/sys/kern/sys_generic.c Sun Jan 25 07:22:34 2009 (r187676) +++ head/sys/kern/sys_generic.c Sun Jan 25 07:24:34 2009 (r187677) @@ -886,6 +886,71 @@ done: return (error); } +/* + * Convert a select bit set to poll flags. + # + * The backend always returns POLLHUP/POLLERR if appropriate and we + * return this as a set bit in any set. + */ +static int select_flags[3] = { + POLLRDNORM | POLLHUP | POLLERR, + POLLWRNORM | POLLHUP | POLLERR, + POLLRDBAND | POLLHUP | POLLERR +}; + +/* + * Compute the fo_poll flags required for a fd given by the index and + * bit position in the fd_mask array. + */ +static __inline int +selflags(fd_mask **ibits, int idx, int bit) +{ + int flags; + int msk; + + flags = 0; + for (msk = 0; msk < 3; msk++) { + if (ibits[msk] == NULL) + continue; + if ((ibits[msk][idx] & (fd_mask)bit) == 0) + continue; + flags |= select_flags[msk]; + } + return (flags); +} + +/* + * Set the appropriate output bits given a mask of fired events and the + * input bits originally requested. + */ +static __inline int +selsetbits(fd_mask **ibits, fd_mask **obits, int idx, fd_mask bit, int events) +{ + int msk; + int n; + + n = 0; + for (msk = 0; msk < 3; msk++) { + if ((events & select_flags[msk]) == 0) + continue; + if (ibits[msk] == NULL) + continue; + if ((ibits[msk][idx] & bit) == 0) + continue; + /* + * XXX Check for a duplicate set. This can occur because a + * socket calls selrecord() twice for each poll() call + * resulting in two selfds per real fd. selrescan() will + * call selsetbits twice as a result. + */ + if ((obits[msk][idx] & bit) != 0) + continue; + obits[msk][idx] |= bit; + n++; + } + + return (n); +} /* * Traverse the list of fds attached to this thread's seltd and check for @@ -894,18 +959,18 @@ done: static int selrescan(struct thread *td, fd_mask **ibits, fd_mask **obits) { + struct filedesc *fdp; + struct selinfo *si; struct seltd *stp; struct selfd *sfp; struct selfd *sfn; - struct selinfo *si; struct file *fp; - int msk, fd; - int n = 0; - /* Note: backend also returns POLLHUP/POLLERR if appropriate. */ - static int flag[3] = { POLLRDNORM, POLLWRNORM, POLLRDBAND }; - struct filedesc *fdp = td->td_proc->p_fd; + int fd, ev, n; + int idx, bit; + fdp = td->td_proc->p_fd; stp = td->td_sel; + n = 0; FILEDESC_SLOCK(fdp); STAILQ_FOREACH_SAFE(sfp, &stp->st_selq, sf_link, sfn) { fd = (int)(uintptr_t)sfp->sf_cookie; @@ -918,18 +983,11 @@ selrescan(struct thread *td, fd_mask **i FILEDESC_SUNLOCK(fdp); return (EBADF); } - for (msk = 0; msk < 3; msk++) { - if (ibits[msk] == NULL) - continue; - if ((ibits[msk][fd/NFDBITS] & - ((fd_mask) 1 << (fd % NFDBITS))) == 0) - continue; - if (fo_poll(fp, flag[msk], td->td_ucred, td)) { - obits[msk][(fd)/NFDBITS] |= - ((fd_mask)1 << ((fd) % NFDBITS)); - n++; - } - } + idx = fd / NFDBITS; + bit = 1 << (fd % NFDBITS); + ev = fo_poll(fp, selflags(ibits, idx, bit), td->td_ucred, td); + if (ev != 0) + n += selsetbits(ibits, obits, idx, bit, ev); } FILEDESC_SUNLOCK(fdp); stp->st_flags = 0; @@ -947,38 +1005,32 @@ selscan(td, ibits, obits, nfd) fd_mask **ibits, **obits; int nfd; { - int msk, i, fd; - fd_mask bits; + struct filedesc *fdp; struct file *fp; - int n = 0; - /* Note: backend also returns POLLHUP/POLLERR if appropriate. */ - static int flag[3] = { POLLRDNORM, POLLWRNORM, POLLRDBAND }; - struct filedesc *fdp = td->td_proc->p_fd; + int ev, flags, end, fd; + int n, idx, bit; + fdp = td->td_proc->p_fd; + n = 0; FILEDESC_SLOCK(fdp); - for (msk = 0; msk < 3; msk++) { - if (ibits[msk] == NULL) - continue; - for (i = 0; i < nfd; i += NFDBITS) { - bits = ibits[msk][i/NFDBITS]; - /* ffs(int mask) not portable, fd_mask is long */ - for (fd = i; bits && fd < nfd; fd++, bits >>= 1) { - if (!(bits & 1)) - continue; - if ((fp = fget_locked(fdp, fd)) == NULL) { - FILEDESC_SUNLOCK(fdp); - return (EBADF); - } - selfdalloc(td, (void *)(uintptr_t)fd); - if (fo_poll(fp, flag[msk], td->td_ucred, - td)) { - obits[msk][(fd)/NFDBITS] |= - ((fd_mask)1 << ((fd) % NFDBITS)); - n++; - } + for (idx = 0, fd = 0; idx < nfd; idx++) { + end = imin(fd + NFDBITS, nfd); + for (bit = 1; fd < end; bit <<= 1, fd++) { + /* Compute the list of events we're interested in. */ + flags = selflags(ibits, idx, bit); + if (flags == 0) + continue; + if ((fp = fget_locked(fdp, fd)) == NULL) { + FILEDESC_SUNLOCK(fdp); + return (EBADF); } + selfdalloc(td, (void *)(uintptr_t)fd); + ev = fo_poll(fp, flags, td->td_ucred, td); + if (ev != 0) + n += selsetbits(ibits, obits, idx, bit, ev); } } + FILEDESC_SUNLOCK(fdp); td->td_retval[0] = n; return (0);