Return-Path: Delivered-To: apmail-apr-dev-archive@www.apache.org Received: (qmail 98263 invoked from network); 6 Jun 2004 21:25:33 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur-2.apache.org with SMTP; 6 Jun 2004 21:25:33 -0000 Received: (qmail 69287 invoked by uid 500); 6 Jun 2004 21:25:29 -0000 Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 69126 invoked by uid 500); 6 Jun 2004 21:25:28 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 69100 invoked by uid 99); 6 Jun 2004 21:25:27 -0000 Date: Sun, 06 Jun 2004 14:25:15 -0700 From: Justin Erenkrantz To: Paul Querna , dev@apr.apache.org Subject: Re: [PATCH] KQueue AND sys_epoll Support for apr_pollset Message-ID: <00B8F2A82DC70EDCC557B931@[10.0.1.8]> In-Reply-To: <1086508923.13755.35.camel@localhost> References: <1086502102.13777.30.camel@localhost> <1086502178.13776.32.camel@localhost> <121A4593423DA887E198A60A@[10.0.1.7]> <1086508923.13755.35.camel@localhost> X-Mailer: Mulberry/3.1.5 (Mac OS X) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="==========DF16E55AAC5DE9C08784==========" X-Spam-Status: No, score=-3.0 required=5.0 tests=ALL_TRUSTED,RCS_FORMAT_PATCH autolearn=unavailable version=3.0.0-r10630 X-Spam-Checker-Version: SpamAssassin 3.0.0-r10630 (2004-05-13) on scotch.ics.uci.edu X-Virus-Scanned: clamd / ClamAV version 0.70, clamav-milter version 0.70j X-Virus-Checked: Checked X-Spam-Rating: minotaur-2.apache.org 1.6.2 0/1000/N --==========DF16E55AAC5DE9C08784========== Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline --On Sunday, June 6, 2004 1:02 AM -0700 Paul Querna wrote: >> Other than that, your patch looks sane. +1 on the concept... -- justin > > sweet. Just tested on FreeBSD 5.2.1 - looks fine. +1. I'll give others a day or so to look at it - if no one complains, I'll commit it. FWIW, I fixed up a bunch of style nits - see attached. -- justin --==========DF16E55AAC5DE9C08784========== Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline Index: CHANGES =================================================================== RCS file: /home/cvs/apr/CHANGES,v retrieving revision 1.471 diff -u -r1.471 CHANGES --- CHANGES 1 Jun 2004 10:03:47 -0000 1.471 +++ CHANGES 6 Jun 2004 21:23:48 -0000 @@ -7,6 +7,8 @@ Changes with APR 1.0 + *) Add support for KQueue and sys_epoll to apr_pollset. [Paul Querna] + *) Add apr_shm_remove() function for removing a named shared memory segment. [Amit Athavale ] Index: configure.in =================================================================== RCS file: /home/cvs/apr/configure.in,v retrieving revision 1.585 diff -u -r1.585 configure.in --- configure.in 5 Jun 2004 11:52:43 -0000 1.585 +++ configure.in 6 Jun 2004 21:23:48 -0000 @@ -637,6 +637,20 @@ AC_CHECK_FUNCS(poll) +haveepoll=no +AC_CHECK_FUNCS(epoll_ctl, [haveepoll=yes], ) +if test "x$haveepoll" = "xyes" ; then + AC_DEFINE(HAVE_EPOLL, 1, + [Define if your system supports the epoll system calls]) +fi + +havekqueue=no +AC_CHECK_FUNCS(kevent, [havekqueue=yes], [havekqueue=no]) +if test "x$havekqueue" = "xyes" ; then + AC_DEFINE(HAVE_KQUEUE, 1, + [Define if your system supports the kqueue system calls]) +fi + dnl ----------------------------- Checking for missing POSIX thread functions AC_CHECK_FUNCS([getpwnam_r getpwuid_r getgrnam_r getgrgid_r]) Index: poll/unix/poll.c =================================================================== RCS file: /home/cvs/apr/poll/unix/poll.c,v retrieving revision 1.44 diff -u -r1.44 poll.c --- poll/unix/poll.c 13 Feb 2004 09:38:33 -0000 1.44 +++ poll/unix/poll.c 6 Jun 2004 21:23:49 -0000 @@ -26,12 +26,77 @@ #include #endif +#ifdef HAVE_KQUEUE +#include +#include +#include +#endif + +#ifdef HAVE_EPOLL +#include +#endif #ifdef NETWARE #define HAS_SOCKETS(dt) (dt == APR_POLL_SOCKET) ? 1 : 0 #define HAS_PIPES(dt) (dt == APR_POLL_FILE) ? 1 : 0 #endif +#ifdef HAVE_KQUEUE +static apr_int16_t get_kqueue_revent(apr_int16_t event, apr_int16_t flags) +{ + apr_int16_t rv = 0; + + if (event & EVFILT_READ) + rv |= APR_POLLIN; + if (event & EVFILT_WRITE) + rv |= APR_POLLOUT; + if (flags & EV_ERROR || flags & EV_EOF) + rv |= APR_POLLERR; + + return rv; +} + +#endif + +#ifdef HAVE_EPOLL +static apr_int16_t get_epoll_event(apr_int16_t event) +{ + apr_int16_t rv = 0; + + if (event & APR_POLLIN) + rv |= EPOLLIN; + if (event & APR_POLLPRI) + rv |= EPOLLPRI; + if (event & APR_POLLOUT) + rv |= EPOLLOUT; + if (event & APR_POLLERR) + rv |= EPOLLERR; + if (event & APR_POLLHUP) + rv |= EPOLLHUP; + /* APR_POLLNVAL is not handled by epoll. */ + + return rv; +} + +static apr_int16_t get_epoll_revent(apr_int16_t event) +{ + apr_int16_t rv = 0; + + if (event & EPOLLIN) + rv |= APR_POLLIN; + if (event & EPOLLPRI) + rv |= APR_POLLPRI; + if (event & EPOLLOUT) + rv |= APR_POLLOUT; + if (event & EPOLLERR) + rv |= APR_POLLERR; + if (event & EPOLLHUP) + rv |= APR_POLLHUP; + /* APR_POLLNVAL is not handled by epoll. */ + + return rv; +} +#endif #ifdef HAVE_POLL /* We can just use poll to do our socket polling. */ @@ -286,7 +351,14 @@ struct apr_pollset_t { apr_uint32_t nelts; apr_uint32_t nalloc; -#ifdef HAVE_POLL +#ifdef HAVE_KQUEUE + int kqueue_fd; + struct kevent kevent; + struct kevent* ke_set; +#elif defined(HAVE_EPOLL) + int epoll_fd; + struct epoll_event *pollset; +#elif defined(HAVE_POLL) struct pollfd *pollset; #else fd_set readset, writeset, exceptset; @@ -305,7 +377,7 @@ apr_pool_t *p, apr_uint32_t flags) { -#if !defined(HAVE_POLL) && defined(FD_SETSIZE) +#if !defined(HAVE_KQUEUE) && !defined(HAVE_EPOLL) && !defined(HAVE_POLL) && defined(FD_SETSIZE) if (size > FD_SETSIZE) { *pollset = NULL; return APR_EINVAL; @@ -314,7 +386,17 @@ *pollset = apr_palloc(p, sizeof(**pollset)); (*pollset)->nelts = 0; (*pollset)->nalloc = size; -#ifdef HAVE_POLL +#ifdef HAVE_KQUEUE + (*pollset)->ke_set = (struct kevent*)apr_palloc(p, size * sizeof(struct kevent)); + memset((*pollset)->ke_set, 0, size * sizeof(struct kevent)); + (*pollset)->kqueue_fd = kqueue(); + if ((*pollset)->kqueue_fd == -1) { + return APR_ENOMEM; + } +#elif defined(HAVE_EPOLL) + (*pollset)->epoll_fd = epoll_create(size); + (*pollset)->pollset = apr_palloc(p, size * sizeof(struct epoll_event)); +#elif defined(HAVE_POLL) (*pollset)->pollset = apr_palloc(p, size * sizeof(struct pollfd)); #else FD_ZERO(&((*pollset)->readset)); @@ -333,17 +415,26 @@ APR_DECLARE(apr_status_t) apr_pollset_destroy(apr_pollset_t *pollset) { - /* A no-op function for now. If we later implement /dev/poll - * support, we'll need to close the /dev/poll fd here - */ +#ifdef HAVE_KQUEUE + close(pollset->kqueue_fd); +#elif defined(HAVE_EPOLL) + close(pollset->epoll_fd); +#endif return APR_SUCCESS; } APR_DECLARE(apr_status_t) apr_pollset_add(apr_pollset_t *pollset, const apr_pollfd_t *descriptor) { -#ifndef HAVE_POLL +#ifdef HAVE_KQUEUE apr_os_sock_t fd; +#elif defined(HAVE_EPOLL) + struct epoll_event ev; + int ret = -1; +#else +#if !defined(HAVE_POLL) + apr_os_sock_t fd; +#endif #endif if (pollset->nelts == pollset->nalloc) { @@ -351,7 +442,49 @@ } pollset->query_set[pollset->nelts] = *descriptor; -#ifdef HAVE_POLL + +#ifdef HAVE_KQUEUE + if (descriptor->desc_type == APR_POLL_SOCKET) { + fd = descriptor->desc.s->socketdes; + } + else { + fd = descriptor->desc.f->filedes; + } + + if (descriptor->reqevents & APR_POLLIN) { + EV_SET(&pollset->kevent, fd, EVFILT_READ, EV_ADD, 0, 0, NULL); + + if (kevent(pollset->kqueue_fd, &pollset->kevent, 1, NULL, 0, + NULL) == -1) { + return APR_ENOMEM; + } + } + + if(descriptor->reqevents & APR_POLLOUT) { + EV_SET(&pollset->kevent, fd, EVFILT_WRITE, EV_ADD, 0, 0, NULL); + + if (kevent(pollset->kqueue_fd, &pollset->kevent, 1, NULL, 0, + NULL) == -1) { + return APR_ENOMEM; + } + } + +#elif defined(HAVE_EPOLL) + ev.events = get_epoll_event(descriptor->reqevents); + if (descriptor->desc_type == APR_POLL_SOCKET) { + ev.data.fd = descriptor->desc.s->socketdes; + ret = epoll_ctl(pollset->epoll_fd, EPOLL_CTL_ADD, + descriptor->desc.s->socketdes, &ev); + } + else { + ev.data.fd = descriptor->desc.f->filedes; + ret = epoll_ctl(pollset->epoll_fd, EPOLL_CTL_ADD, + descriptor->desc.f->filedes, &ev); + } + if(0 != ret) { + return APR_EBADF; + } +#elif defined(HAVE_POLL) if (descriptor->desc_type == APR_POLL_SOCKET) { pollset->pollset[pollset->nelts].fd = descriptor->desc.s->socketdes; @@ -420,11 +553,97 @@ const apr_pollfd_t *descriptor) { apr_uint32_t i; -#ifndef HAVE_POLL +#ifdef HAVE_KQUEUE + apr_os_sock_t fd; +#elif defined(HAVE_EPOLL) + struct epoll_event ev; + int ret = -1; +#elif defined(HAVE_POLL) apr_os_sock_t fd; #endif -#ifdef HAVE_POLL +#ifdef HAVE_KQUEUE + for (i = 0; i < pollset->nelts; i++) { + if (descriptor->desc.s == pollset->query_set[i].desc.s) { + /* Found an instance of the fd: remove this and any other copies */ + apr_uint32_t dst = i; + apr_uint32_t old_nelts = pollset->nelts; + pollset->nelts--; + for (i++; i < old_nelts; i++) { + if (descriptor->desc.s == pollset->query_set[i].desc.s) { + pollset->nelts--; + } + else { + pollset->query_set[dst] = pollset->query_set[i]; + dst++; + } + } + + if (descriptor->desc_type == APR_POLL_SOCKET) { + fd = descriptor->desc.s->socketdes; + } + else { + fd = descriptor->desc.f->filedes; + } + + if(descriptor->reqevents & APR_POLLIN) { + EV_SET(&pollset->kevent, fd, + EVFILT_READ, EV_DELETE, 0, 0, NULL); + + if(kevent(pollset->kqueue_fd, &pollset->kevent, 1, NULL, 0, + NULL) == -1) { + return APR_EBADF; + } + } + + if(descriptor->reqevents & APR_POLLOUT) { + EV_SET(&pollset->kevent, fd, + EVFILT_WRITE, EV_DELETE, 0, 0, NULL); + + if(kevent(pollset->kqueue_fd, &pollset->kevent, 1, NULL, 0, + NULL) == -1) { + return APR_EBADF; + } + } + + return APR_SUCCESS; + } + } +#elif defined(HAVE_EPOLL) + for (i = 0; i < pollset->nelts; i++) { + if (descriptor->desc.s == pollset->query_set[i].desc.s) { + /* Found an instance of the fd: remove this and any other copies */ + apr_uint32_t dst = i; + apr_uint32_t old_nelts = pollset->nelts; + pollset->nelts--; + for (i++; i < old_nelts; i++) { + if (descriptor->desc.s == pollset->query_set[i].desc.s) { + pollset->nelts--; + } + else { + pollset->query_set[dst] = pollset->query_set[i]; + dst++; + } + } + ev.events = get_epoll_event(descriptor->reqevents); + if (descriptor->desc_type == APR_POLL_SOCKET) { + ev.data.fd = descriptor->desc.s->socketdes; + ret = epoll_ctl(pollset->epoll_fd, EPOLL_CTL_DEL, + descriptor->desc.s->socketdes, &ev); + } + else { + ev.data.fd = descriptor->desc.f->filedes; + ret = epoll_ctl(pollset->epoll_fd, EPOLL_CTL_DEL, + descriptor->desc.f->filedes, &ev); + } + if (ret < 0) { + return APR_EBADF; + } + + return APR_SUCCESS; + } + } +#elif defined(HAVE_POLL) for (i = 0; i < pollset->nelts; i++) { if (descriptor->desc.s == pollset->query_set[i].desc.s) { /* Found an instance of the fd: remove this and any other copies */ @@ -485,8 +704,119 @@ return APR_NOTFOUND; } +#ifdef HAVE_KQUEUE +APR_DECLARE(apr_status_t) apr_pollset_poll(apr_pollset_t *pollset, + apr_interval_time_t timeout, + apr_int32_t *num, + const apr_pollfd_t **descriptors) +{ + int rv; + apr_uint32_t i, j, r = 0; + struct timespec tv, *tvptr; -#ifdef HAVE_POLL + if (timeout < 0) { + tvptr = NULL; + } + else { + tv.tv_sec = (long)apr_time_sec(timeout); + tv.tv_nsec = (long)apr_time_msec(timeout); + tvptr = &tv; + } + + rv = kevent(pollset->kqueue_fd, NULL, 0, pollset->ke_set, pollset->nelts, + tvptr); + (*num) = rv; + if (rv < 0) { + return apr_get_netos_error(); + } + if (rv == 0) { + return APR_TIMEUP; + } + + /* TODO: Is there a better way to re-associate our data? */ + for (i = 0; i < pollset->nelts; i++) { + apr_os_sock_t fd; + if (pollset->query_set[i].desc_type == APR_POLL_SOCKET) { + fd = pollset->query_set[i].desc.s->socketdes; + } + else { + fd = pollset->query_set[i].desc.f->filedes; + } + for (j = 0; j < rv; j++) { + if (pollset->ke_set[j].ident == fd ) { + pollset->result_set[r] = pollset->query_set[i]; + pollset->result_set[r].rtnevents = + get_kqueue_revent(pollset->ke_set[j].filter, + pollset->ke_set[j].flags); + r++; + } + } + } + + (*num) = r; + + if (descriptors) { + *descriptors = pollset->result_set; + } + + return APR_SUCCESS; +} + +#elif defined(HAVE_EPOLL) + +APR_DECLARE(apr_status_t) apr_pollset_poll(apr_pollset_t *pollset, + apr_interval_time_t timeout, + apr_int32_t *num, + const apr_pollfd_t **descriptors) +{ + int rv; + apr_uint32_t i, j, k; + + if (timeout > 0) { + timeout /= 1000; + } + + rv = epoll_wait(pollset->epoll_fd, pollset->pollset , pollset->nelts, + timeout); + (*num) = rv; + if (rv < 0) { + return apr_get_netos_error(); + } + if (rv == 0) { + return APR_TIMEUP; + } + j = 0; + for (i = 0; i < pollset->nelts; i++) { + if (pollset->pollset[i].events != 0) { + /* TODO: Is there a better way to re-associate our data? */ + for(k = 0; k < pollset->nelts; k++) { + if(pollset->query_set[k].desc_type == APR_POLL_SOCKET && + pollset->query_set[k].desc.s->socketdes == + pollset->pollset[i].data.fd) { + pollset->result_set[j] = pollset->query_set[k]; + pollset->result_set[j].rtnevents = + get_epoll_revent(pollset->pollset[i].events); + j++; + break; + } + else if(pollset->query_set[k].desc_type == APR_POLL_FILE && + pollset->query_set[k].desc.f->filedes == + pollset->pollset[i].data.fd) { + pollset->result_set[j] = pollset->query_set[k]; + pollset->result_set[j].rtnevents = + get_epoll_revent(pollset->pollset[i].events); + j++; + break; + } + } + } + } + if (descriptors) { + *descriptors = pollset->result_set; + } + return APR_SUCCESS; +} +#elif defined(HAVE_POLL) APR_DECLARE(apr_status_t) apr_pollset_poll(apr_pollset_t *pollset, apr_interval_time_t timeout, apr_int32_t *num, Index: test/testpoll.c =================================================================== RCS file: /home/cvs/apr/test/testpoll.c,v retrieving revision 1.33 diff -u -r1.33 testpoll.c --- test/testpoll.c 26 May 2004 14:50:27 -0000 1.33 +++ test/testpoll.c 6 Jun 2004 21:23:49 -0000 @@ -489,10 +489,14 @@ rv = apr_pollset_poll(pollset, 1000, &num, &hot_files); ABTS_INT_EQUAL(tc, APR_SUCCESS, rv); ABTS_INT_EQUAL(tc, 2, num); - ABTS_PTR_EQUAL(tc, (void *)1, hot_files[0].client_data); - ABTS_PTR_EQUAL(tc, s[0], hot_files[0].desc.s); - ABTS_PTR_EQUAL(tc, (void *)4, hot_files[1].client_data); - ABTS_PTR_EQUAL(tc, s[3], hot_files[1].desc.s); + ABTS_ASSERT(tc, "Incorrect socket in result set", + ((hot_files[0].desc.s == s[0]) && (hot_files[1].desc.s == s[3])) || + ((hot_files[0].desc.s == s[3]) && (hot_files[1].desc.s == s[0]))); + ABTS_ASSERT(tc, "Incorrect client data in result set", + ((hot_files[0].client_data == (void *)1) && + (hot_files[1].client_data == (void *)4)) || + ((hot_files[0].client_data == (void *)4) && + (hot_files[1].client_data == (void *)1))); } abts_suite *testpoll(abts_suite *suite) --==========DF16E55AAC5DE9C08784==========--