You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by tr...@apache.org on 2009/02/21 23:22:35 UTC
svn commit: r746597 - /apr/apr/trunk/poll/unix/epoll.c
Author: trawick
Date: Sat Feb 21 22:22:34 2009
New Revision: 746597
URL: http://svn.apache.org/viewvc?rev=746597&view=rev
Log:
simplifications and minor improvements to error reporting and
pathlength
impl_pollset_create:
use the apr_get_netos_error() macro instead of errno for
consistency with the rest of this file
impl_pollset_add:
return errno instead of APR_EBADF when we fail to add to
the pollset (it may indeed be EBADF, but it could be more
interesting)
refactor a small amount of code which was duplicated for
flags with/without APR_POLLSET_WAKEABLE
impl_pollset_remove:
axe a redundant check for APR_RING_EMPTY, as
for (var = APR_RING_FIRST(); var != APR_RING_SENTINEL(); ...)
makes essentially the same check
impl_pollset_poll():
refactor a stretch of code which was duplicated for flags
with/without APR_POLLSET_WAKEABLE, and at the same time axe
an unnecessary structure copy in the usual case that the
returned event was not the wakeup pipe
Modified:
apr/apr/trunk/poll/unix/epoll.c
Modified: apr/apr/trunk/poll/unix/epoll.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/epoll.c?rev=746597&r1=746596&r2=746597&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/epoll.c (original)
+++ apr/apr/trunk/poll/unix/epoll.c Sat Feb 21 22:22:34 2009
@@ -98,7 +98,7 @@
fd = epoll_create(size);
if (fd < 0) {
pollset->p = NULL;
- return errno;
+ return apr_get_netos_error();
}
pollset->p = apr_palloc(p, sizeof(apr_pollset_private_t));
@@ -165,14 +165,12 @@
descriptor->desc.f->filedes, &ev);
}
- if (pollset->flags & APR_POLLSET_NOCOPY) {
- if (0 != ret) {
- rv = APR_EBADF;
- }
+ if (0 != ret) {
+ rv = apr_get_netos_error();
}
- else {
- if (0 != ret) {
- rv = APR_EBADF;
+
+ if (!(pollset->flags & APR_POLLSET_NOCOPY)) {
+ if (rv != APR_SUCCESS) {
APR_RING_INSERT_TAIL(&(pollset->p->free_ring), elem, pfd_elem_t, link);
}
else {
@@ -210,18 +208,16 @@
if (!(pollset->flags & APR_POLLSET_NOCOPY)) {
pollset_lock_rings();
- if (!APR_RING_EMPTY(&(pollset->p->query_ring), pfd_elem_t, link)) {
- for (ep = APR_RING_FIRST(&(pollset->p->query_ring));
- ep != APR_RING_SENTINEL(&(pollset->p->query_ring),
- pfd_elem_t, link);
- ep = APR_RING_NEXT(ep, link)) {
+ for (ep = APR_RING_FIRST(&(pollset->p->query_ring));
+ ep != APR_RING_SENTINEL(&(pollset->p->query_ring),
+ pfd_elem_t, link);
+ ep = APR_RING_NEXT(ep, link)) {
- if (descriptor->desc.s == ep->pfd.desc.s) {
- APR_RING_REMOVE(ep, link);
- APR_RING_INSERT_TAIL(&(pollset->p->dead_ring),
- ep, pfd_elem_t, link);
- break;
- }
+ if (descriptor->desc.s == ep->pfd.desc.s) {
+ APR_RING_REMOVE(ep, link);
+ APR_RING_INSERT_TAIL(&(pollset->p->dead_ring),
+ ep, pfd_elem_t, link);
+ break;
}
}
@@ -238,7 +234,7 @@
{
int ret, i, j;
apr_status_t rv = APR_SUCCESS;
- apr_pollfd_t fd;
+ apr_pollfd_t *fdptr;
if (timeout > 0) {
timeout /= 1000;
@@ -255,47 +251,31 @@
rv = APR_TIMEUP;
}
else {
- if (pollset->flags & APR_POLLSET_NOCOPY) {
- for (i = 0, j = 0; i < ret; i++) {
- fd = *((apr_pollfd_t *)(pollset->p->pollset[i].data.ptr));
- /* Check if the polled descriptor is our
- * wakeup pipe. In that case do not put it result set.
- */
- if ((pollset->flags & APR_POLLSET_WAKEABLE) &&
- fd.desc_type == APR_POLL_FILE &&
- fd.desc.f == pollset->wakeup_pipe[0]) {
- apr_pollset_drain_wakeup_pipe(pollset);
- rv = APR_EINTR;
- }
- else {
- pollset->p->result_set[j] = fd;
- pollset->p->result_set[j].rtnevents =
- get_epoll_revent(pollset->p->pollset[i].events);
- j++;
- }
+ for (i = 0, j = 0; i < ret; i++) {
+ if (pollset->flags & APR_POLLSET_NOCOPY) {
+ fdptr = (apr_pollfd_t *)(pollset->p->pollset[i].data.ptr);
}
- if (((*num) = j))
- rv = APR_SUCCESS;
- }
- else {
- for (i = 0, j = 0; i < ret; i++) {
- fd = (((pfd_elem_t *) (pollset->p->pollset[i].data.ptr))->pfd);
- if ((pollset->flags & APR_POLLSET_WAKEABLE) &&
- fd.desc_type == APR_POLL_FILE &&
- fd.desc.f == pollset->wakeup_pipe[0]) {
- apr_pollset_drain_wakeup_pipe(pollset);
- rv = APR_EINTR;
- }
- else {
- pollset->p->result_set[j] = fd;
- pollset->p->result_set[j].rtnevents =
- get_epoll_revent(pollset->p->pollset[i].events);
- j++;
- }
+ else {
+ fdptr = &(((pfd_elem_t *) (pollset->p->pollset[i].data.ptr))->pfd);
+ }
+ /* Check if the polled descriptor is our
+ * wakeup pipe. In that case do not put it result set.
+ */
+ if ((pollset->flags & APR_POLLSET_WAKEABLE) &&
+ fdptr->desc_type == APR_POLL_FILE &&
+ fdptr->desc.f == pollset->wakeup_pipe[0]) {
+ apr_pollset_drain_wakeup_pipe(pollset);
+ rv = APR_EINTR;
+ }
+ else {
+ pollset->p->result_set[j] = *fdptr;
+ pollset->p->result_set[j].rtnevents =
+ get_epoll_revent(pollset->p->pollset[i].events);
+ j++;
}
- if (((*num) = j))
- rv = APR_SUCCESS;
}
+ if (((*num) = j))
+ rv = APR_SUCCESS;
if (descriptors) {
*descriptors = pollset->p->result_set;
Re: Re: svn commit: r746597 - /apr/apr/trunk/poll/unix/epoll.c
Posted by tr...@gmail.com.
On Feb 22, 2009 3:53am, Mladen Turk <mt...@apache.org> wrote:
> Jeff Trawick wrote:
> On Sat, Feb 21, 2009 at 5:22 PM, trawick@apache.org
> I think there may still be feedback issues in impl_pollset_poll() with
> rv, *num, and *descriptors, possibly specific to APR_POLLSET_WAKEABLE. I
> don't see a testcase for that; maybe I'll get a chance to play with that
> soon.
> The only difference is that if only the wakeup socked was signaled
> the *num is set to 0, descriptors are untouched and rv is APR_EINTR.
> Test case would be cool. I have one, but it needs a
> little bit polishing.
> Meanwhile, testpoll is crashing on Leopard, and I didn't think I changed
> any code that affects Leopard (yet) ;)
> It passes on Darwin. Does Leopard uses kqueue as well?
yes
As it turns out, I was lucky to see it crash a couple of times in a row, as
it is reproduced infrequently.
These hacks work around most intermittent testpoll failures I see on on
leopard (Darwin Kernel Version 9.6.0: Mon Nov 24 17:37:00 PST 2008 ...
i386), where kevent returns unexpectedly:
Index: testpoll.c
===================================================================
--- testpoll.c (revision 746597)
+++ testpoll.c (working copy)
@@ -382,6 +382,11 @@
send_msg(s, sa, 0, tc);
rv = apr_pollset_poll(pollset, 0, &num, &descs);
+ if (rv != APR_SUCCESS) {
+ /* rare glitch seen on Leopard; wait just a little longer */
+ printf(" \n DELAY %d\n ", __LINE__);
+ rv = apr_pollset_poll(pollset, apr_time_from_sec(1), &num, &descs);
+ }
ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
ABTS_INT_EQUAL(tc, 1, num);
ABTS_PTR_NOTNULL(tc, descs);
@@ -444,6 +449,11 @@
send_msg(s, sa, LARGE_NUM_SOCKETS - 1, tc);
rv = apr_pollset_poll(pollset, 0, &num, &descs);
+ if (rv != APR_SUCCESS) {
+ /* rare glitch seen on Leopard; wait just a little longer */
+ printf(" \n DELAY %d\n ", __LINE__);
+ rv = apr_pollset_poll(pollset, apr_time_from_sec(1), &num, &descs);
+ }
ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
ABTS_INT_EQUAL(tc, 1, num);
ABTS_PTR_NOTNULL(tc, descs);
@@ -607,7 +617,13 @@
send_msg(s, sa, 0, tc);
pcb.tc = tc;
pcb.count = 0;
- rv = apr_pollcb_poll(pollcb, 0, trigger_pollcb_cb, &pcb);
+ rv = apr_pollcb_poll(pollcb, 0, trigger_pollcb_cb, &pcb);
+ if (APR_STATUS_IS_TIMEUP(rv)) {
+ /* rare glitch seen on Leopard; wait just a little longer */
+ printf(" \n DELAY %d\n ", __LINE__);
+ rv = apr_pollcb_poll(pollcb, apr_time_from_sec(1), trigger_pollcb_cb,
+ &pcb);
+ }
ABTS_INT_EQUAL(tc, 0, APR_STATUS_IS_TIMEUP(rv));
ABTS_INT_EQUAL(tc, 1, pcb.count);
Re: svn commit: r746597 - /apr/apr/trunk/poll/unix/epoll.c
Posted by Mladen Turk <mt...@apache.org>.
Jeff Trawick wrote:
> On Sat, Feb 21, 2009 at 5:22 PM, <trawick@apache.org
>
> I think there may still be feedback issues in impl_pollset_poll() with
> rv, *num, and *descriptors, possibly specific to APR_POLLSET_WAKEABLE.
> I don't see a testcase for that; maybe I'll get a chance to play with
> that soon.
>
The only difference is that if only the wakeup socked was signaled
the *num is set to 0, descriptors are untouched and rv is APR_EINTR.
Test case would be cool. I have one, but it needs a
little bit polishing.
> Meanwhile, testpoll is crashing on Leopard, and I didn't think I changed
> any code that affects Leopard (yet) ;)
>
It passes on Darwin. Does Leopard uses kqueue as well?
Regards
--
^(TM)
Re: svn commit: r746597 - /apr/apr/trunk/poll/unix/epoll.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Sat, Feb 21, 2009 at 5:22 PM, <tr...@apache.org> wrote:
> Author: trawick
> Date: Sat Feb 21 22:22:34 2009
> New Revision: 746597
>
> URL: http://svn.apache.org/viewvc?rev=746597&view=rev
> Log:
> simplifications and minor improvements to error reporting and
> pathlength
I think there may still be feedback issues in impl_pollset_poll() with rv,
*num, and *descriptors, possibly specific to APR_POLLSET_WAKEABLE. I don't
see a testcase for that; maybe I'll get a chance to play with that soon.
Meanwhile, testpoll is crashing on Leopard, and I didn't think I changed any
code that affects Leopard (yet) ;)