Bug #796: epoll fixes for 100% CPU usage

by Gonzalo Arana

This fixes the (100% CPU usage) problem with this platform:
kernel 2.4.21 patched with
http://www.xmailserver.org/linux-patches/epoll-lt-2.4.21-0.18.diff,
epoll-lib-0.10, pcl-1.2.

diff --git a/src/comm_epoll.cc b/src/comm_epoll.cc
index 3e9029f82a79015058f1acd2b4cde8533153b6e0..a61fa493d8501ecaf3d8893d238d9a38290e9e13 100644 (file)
--- a/src/comm_epoll.cc
+++ b/src/comm_epoll.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: comm_epoll.cc,v 1.4 2003/08/03 21:38:15 robertc Exp $
+ * $Id: comm_epoll.cc,v 1.5 2003/11/09 17:11:10 hno Exp $
  *
  * DEBUG: section 5    Socket functions
  *
@@ -94,6 +94,24 @@ comm_select_init(void)
     }
 }
 
+static const char* epolltype_atoi(int x)
+{
+    switch(x) {
+
+    case EPOLL_CTL_ADD:
+        return "EPOLL_CTL_ADD";
+
+    case EPOLL_CTL_DEL:
+        return "EPOLL_CTL_DEL";
+
+    case EPOLL_CTL_MOD:
+        return "EPOLL_CTL_MOD";
+
+    default:
+        return "UNKNOWN_EPOLLCTL_OP";
+    }
+}
+
 /*
  * comm_setselect
  *
@@ -106,10 +124,7 @@ commSetSelect(int fd, unsigned int type, PF * handler,
               void *client_data, time_t timeout)
 {
     fde *F = &fd_table[fd];
-    int change = 0;
-    int events = 0;
-    int pollin = 0;
-    int pollout = 0;
+    int epoll_ctl_type = 0;
 
     struct epoll_event ev;
     assert(fd >= 0);
@@ -117,70 +132,52 @@ commSetSelect(int fd, unsigned int type, PF * handler,
     debug(5, DEBUG_EPOLL ? 0 : 8) ("commSetSelect(fd=%d,type=%u,handler=%p,client_data=%p,timeout=%ld)\n",
                                    fd,type,handler,client_data,timeout);
 
-    if(F->read_handler != NULL)
-        pollin = 1;
+    ev.events = 0;
+    ev.data.fd = fd;
 
-    if(F->write_handler != NULL)
-        pollout = 1;
+    // If read is an interest
 
     if (type & COMM_SELECT_READ) {
-        if(F->read_handler != handler)
-            change = 1;
-
-        if(handler == NULL)
-            pollin = 0;
-        else
-            pollin = 1;
+        if (handler)
+            ev.events |= EPOLLIN;
 
         F->read_handler = handler;
 
         F->read_data = client_data;
+
+        // Otherwise, use previously stored value
+    } else if (F->epoll_state & EPOLLIN) {
+        ev.events |= EPOLLIN;
     }
 
+    // If write is an interest
     if (type & COMM_SELECT_WRITE) {
-        if(F->write_handler != handler)
-            change = 1;
-
-        if(handler == NULL)
-            pollout = 0;
-        else
-            pollout = 1;
+        if (handler)
+            ev.events |= EPOLLOUT;
 
         F->write_handler = handler;
 
         F->write_data = client_data;
-    }
 
-    if(pollin)
-        events |= EPOLLIN;
-
-    if(pollout)
-        events |= EPOLLOUT;
-
-    if(events)
-        events |= EPOLLHUP | EPOLLERR;
+        // Otherwise, use previously stored value
+    } else if (F->epoll_state & EPOLLOUT) {
+        ev.events |= EPOLLOUT;
+    }
 
-    ev.data.fd = fd;
+    if (ev.events)
+        ev.events |= EPOLLHUP | EPOLLERR;
 
-    ev.events = events;
+    if (ev.events != F->epoll_state) {
+        if (F->epoll_state) // already monitoring something.
+            epoll_ctl_type = ev.events ? EPOLL_CTL_MOD : EPOLL_CTL_DEL;
+        else
+            epoll_ctl_type = EPOLL_CTL_ADD;
 
-    if(events) {
-        if (epoll_ctl(kdpfd, EPOLL_CTL_MOD, fd, &ev) < 0) {
-            if(errno == ENOENT) {
-                debug(5,4) ("commSetSelect: epoll_ctl(,EPOLL_CTL_MOD,,) failed on fd=%d: entry does not exist\n",fd);
+        F->epoll_state = ev.events;
 
-                if (epoll_ctl(kdpfd, EPOLL_CTL_ADD, fd, &ev) < 0)
-                    debug(5,1) ("commSetSelect: cpoll_ctl(,EPOLL_CTL_ADD,,) failed on fd=%d!: %s\n",fd,xstrerror());
-            } else {
-                debug(5,1) ("commSetSelect: cpoll_ctl(,EPOLL_CTL_MOD,,) failed on fd=%d!: %s\n",fd,xstrerror());
-            }
-        }
-    } else if(change) {
-        if(epoll_ctl(kdpfd,EPOLL_CTL_DEL,fd,&ev) < 0) {
-            if(errno != ENOENT)
-                debug(5,1) ("commSetSelect: cpoll_ctl(,EPOLL_CTL_DEL,,) failed on fd=%d!: %s\n",fd,xstrerror());
-            else
-                debug(5,4) ("commSetSelect: epoll_ctl(,EPOLL_CTL_DEL,,) failed on fd=%d: entry does not exist\n",fd);
+        if (epoll_ctl(kdpfd, epoll_ctl_type, fd, &ev) < 0) {
+            debug(5, DEBUG_EPOLL ? 0 : 8) ("commSetSelect: epoll_ctl(,%s,,): failed on fd=%d: %s\n",
+                                           epolltype_atoi(epoll_ctl_type), fd, xstrerror());
         }
     }
 
@@ -238,32 +235,56 @@ comm_select(int msec)
 
     getCurrentTime();
 
+    statHistCount(&statCounter.select_fds_hist, num);
+
     if (num == 0)
-        return COMM_OK;                /* No error.. */
+        return COMM_TIMEOUT;           /* No error.. */
+
+    PROF_start(comm_handle_ready_fd);
 
     for (i = 0, cevents = pevents; i < num; i++, cevents++) {
         fd = cevents->data.fd;
         F = &fd_table[fd];
-        debug(5, DEBUG_EPOLL ? 0 : 8) ("comm_select(): got fd=%d events=%d F->read_handler=%p F->write_handler=%p\n",
-                                       fd,cevents->events,F->read_handler,F->write_handler);
+        debug(5, DEBUG_EPOLL ? 0 : 8) ("comm_select(): got fd=%d events=%x monitoring=%x F->read_handler=%p F->write_handler=%p\n",
+                                       fd,cevents->events,F->epoll_state,F->read_handler,F->write_handler);
+
+        // TODO: add EPOLLPRI??
 
-        if(cevents->events & (EPOLLIN|EPOLLHUP|EPOLLERR)) {
+        if (cevents->events & (EPOLLIN|EPOLLHUP|EPOLLERR)) {
             if((hdl = F->read_handler) != NULL) {
                 debug(5, DEBUG_EPOLL ? 0 : 8) ("comm_select(): Calling read handler on fd=%d\n",fd);
+                PROF_start(comm_write_handler);
                 F->read_handler = NULL;
                 hdl(fd, F->read_data);
+                PROF_stop(comm_write_handler);
+                statCounter.select_fds++;
+            } else {
+                debug(5, DEBUG_EPOLL ? 0 : 8) ("comm_select(): no read handler for fd=%d\n",fd);
+                fd_table[fd].flags.read_pending = 1;
+                // remove interest since no handler exist for this event.
+                commSetSelect(fd, COMM_SELECT_READ, NULL, NULL, 0);
             }
         }
 
-        if(cevents->events & (EPOLLOUT|EPOLLHUP|EPOLLERR)) {
+        if (cevents->events & (EPOLLOUT|EPOLLHUP|EPOLLERR)) {
             if((hdl = F->write_handler) != NULL) {
                 debug(5, DEBUG_EPOLL ? 0 : 8) ("comm_select(): Calling write handler on fd=%d\n",fd);
+                PROF_start(comm_read_handler);
                 F->write_handler = NULL;
                 hdl(fd, F->write_data);
+                PROF_stop(comm_read_handler);
+                statCounter.select_fds++;
+            } else {
+                fd_table[fd].flags.write_pending = 1;
+                debug(5, DEBUG_EPOLL ? 0 : 8) ("comm_select(): no write handler for fd=%d\n",fd);
+                // remove interest since no handler exist for this event.
+                commSetSelect(fd, COMM_SELECT_WRITE, NULL, NULL, 0);
             }
         }
     }
 
+    PROF_stop(comm_handle_ready_fd);
+
     return COMM_OK;
 }
 

diff --git a/src/fd.cc b/src/fd.cc
index a9042981dc89de2d5d54a131c1222e82917eb4ba..77b47f1593c4fd0d871d720e3f46a8e6cc5d5a15 100644 (file)
--- a/src/fd.cc
+++ b/src/fd.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: fd.cc,v 1.48 2003/02/21 22:50:08 robertc Exp $
+ * $Id: fd.cc,v 1.49 2003/11/09 17:11:11 hno Exp $
  *
  * DEBUG: section 51    Filedescriptor Functions
  * AUTHOR: Duane Wessels
@@ -163,6 +163,7 @@ fd_open(int fd, unsigned int type, const char *desc)
     debug(51, 3) ("fd_open FD %d %s\n", fd, desc);
     F->type = type;
     F->flags.open = 1;
+    F->epoll_state = 0;
 #ifdef _SQUID_MSWIN_
 
     switch (type) {

diff --git a/src/fde.h b/src/fde.h
index 85c311e927b5beabd80a7ab622e1c137b93dd4ac..6cf981b43bb53eb930209717f6725a019a2f4b10 100644 (file)
--- a/src/fde.h
+++ b/src/fde.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: fde.h,v 1.5 2003/07/15 06:50:42 robertc Exp $
+ * $Id: fde.h,v 1.6 2003/11/09 17:11:11 hno Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -93,12 +93,16 @@ unsigned int close_on_exec:
 
 unsigned int read_pending:
         1;
+
+unsigned int write_pending:
+        1;
     }
 
     flags;
     int bytes_read;
     int bytes_written;
     int uses;                   /* ie # req's over persistent conn */
+    unsigned epoll_state;
 
     struct _fde_disk disk;
     PF *read_handler;