Request for review: 7166048: remove the embedded epoll data structure

[Alan Bateman]

On 04/05/2012 08:35, Sean Chou wrote:
> Hi all,
>
>     I found the src/solaris/native/sun/nio/ch/EPollArrayWrapper.c 
> embedded the epoll data structure epoll_data and epoll_event . It is 
> duplicated with the definition in sys/epoll.h . This redundancy would 
> cause failure if the underlying system has different epoll data 
> structure (eg. different alignment) .
>
>     I reported 7166048 for it and made a patch: 
> http://cr.openjdk.java.net/~zhouyx/7166048/webrev.00/ 
> <http://cr.openjdk.java.net/%7Ezhouyx/7166048/webrev.00/>  .
>
>
As background, when the epoll Selector was added then the JDK had to be 
build on distributions that were still 2.4 based and didn't have 
epoll.h. We had meant to go back and clear this up in JDK7 but didn't 
get around to it. So thanks for taking this on now. I looked at your 
patch but it doesn't seem to be complete as it doesn't change the usages 
to call the epoll functions directly. Attached is the patch that I had 
for cleaning this up, it should still be current because this code has 
not changed. The only thing I notice is that it doesn't remove the 
include of dlfcn.h, that shouldn't be needed now.

On the RFE that you submitted, I've moved this to the right place as:

7166048: (se) EPollArrayWrapper.c no longer needs to define epoll data 
structures

-Alan.



diff --git a/src/solaris/native/sun/nio/ch/EPollArrayWrapper.c 
b/src/solaris/native/sun/nio/ch/EPollArrayWrapper.c
--- a/src/solaris/native/sun/nio/ch/EPollArrayWrapper.c
+++ b/src/solaris/native/sun/nio/ch/EPollArrayWrapper.c
@@ -34,55 +34,13 @@
  #include <unistd.h>
  #include <sys/resource.h>
  #include <sys/time.h>
-
-#ifdef  __cplusplus
-extern "C" {
-#endif
-
-/* epoll_wait(2) man page */
-
-typedef union epoll_data {
-    void *ptr;
-    int fd;
-    __uint32_t u32;
-    __uint64_t u64;
-} epoll_data_t;
-
-
-/* x86-64 has same alignment as 32-bit */
-#ifdef __x86_64__
-#define EPOLL_PACKED __attribute__((packed))
-#else
-#define EPOLL_PACKED
-#endif
-
-struct epoll_event {
-    __uint32_t events;  /* Epoll events */
-    epoll_data_t data;  /* User data variable */
-} EPOLL_PACKED;
-
-#ifdef  __cplusplus
-}
-#endif
+#include <sys/epoll.h>

  #define RESTARTABLE(_cmd, _result) do { \
    do { \
      _result = _cmd; \
    } while((_result == -1) && (errno == EINTR)); \
  } while(0)
-
-/*
- * epoll event notification is new in 2.6 kernel. As the offical build
- * platform for the JDK is on a 2.4-based distribution then we must
- * obtain the addresses of the epoll functions dynamically.
- */
-typedef int (*epoll_create_t)(int size);
-typedef int (*epoll_ctl_t)   (int epfd, int op, int fd, struct 
epoll_event *event);
-typedef int (*epoll_wait_t)  (int epfd, struct epoll_event *events, int 
maxevents, int timeout);
-
-static epoll_create_t epoll_create_func;
-static epoll_ctl_t    epoll_ctl_func;
-static epoll_wait_t   epoll_wait_func;

  static int
  iepoll(int epfd, struct epoll_event *events, int numfds, jlong timeout)
@@ -96,7 +54,7 @@ iepoll(int epfd, struct epoll_event *eve
      start = t.tv_sec * 1000 + t.tv_usec / 1000;

      for (;;) {
-        int res = (*epoll_wait_func)(epfd, events, numfds, timeout);
+        int res = epoll_wait(epfd, events, numfds, timeout);
          if (res < 0 && errno == EINTR) {
              if (remaining >= 0) {
                  gettimeofday(&t, NULL);
@@ -117,14 +75,6 @@ JNIEXPORT void JNICALL
  JNIEXPORT void JNICALL
  Java_sun_nio_ch_EPollArrayWrapper_init(JNIEnv *env, jclass this)
  {
-    epoll_create_func = (epoll_create_t) dlsym(RTLD_DEFAULT, 
"epoll_create");
-    epoll_ctl_func    = (epoll_ctl_t)    dlsym(RTLD_DEFAULT, "epoll_ctl");
-    epoll_wait_func   = (epoll_wait_t)   dlsym(RTLD_DEFAULT, "epoll_wait");
-
-    if ((epoll_create_func == NULL) || (epoll_ctl_func == NULL) ||
-        (epoll_wait_func == NULL)) {
-        JNU_ThrowInternalError(env, "unable to get address of epoll 
functions, pre-2.6 kernel?");
-    }
  }

  JNIEXPORT jint JNICALL
@@ -134,7 +84,7 @@ Java_sun_nio_ch_EPollArrayWrapper_epollC
       * epoll_create expects a size as a hint to the kernel about how to
       * dimension internal structures. We can't predict the size in 
advance.
       */
-    int epfd = (*epoll_create_func)(256);
+    int epfd = epoll_create(256);
      if (epfd < 0) {
         JNU_ThrowIOExceptionWithLastError(env, "epoll_create failed");
      }
@@ -173,7 +123,7 @@ Java_sun_nio_ch_EPollArrayWrapper_epollC
      event.events = events;
      event.data.fd = fd;

-    RESTARTABLE((*epoll_ctl_func)(epfd, (int)opcode, (int)fd, &event), 
res);
+    RESTARTABLE(epoll_ctl(epfd, (int)opcode, (int)fd, &event), res);

      /*
       * A channel may be registered with several Selectors. When each 
Selector
@@ -199,7 +149,7 @@ Java_sun_nio_ch_EPollArrayWrapper_epollW
      int res;

      if (timeout <= 0) {           /* Indefinite or no wait */
-        RESTARTABLE((*epoll_wait_func)(epfd, events, numfds, timeout), 
res);
+        RESTARTABLE(epoll_wait(epfd, events, numfds, timeout), res);
      } else {                      /* Bounded wait; bounded restarts */
          res = iepoll(epfd, events, numfds, timeout);
      }