Request for review: 7166048: remove the embedded epoll data structure
Sean Chou
zhouyx at linux.vnet.ibm.com
Mon May 7 02:07:21 PDT 2012
Hi Alan and Charles,
Many thanks. Patch confirmed.
On Mon, May 7, 2012 at 4:48 PM, Charles Lee <littlee at linux.vnet.ibm.com>wrote:
> Hi Sean,
>
> The patch is committed @
>
> Changeset: 62557a1336c0
> Author: zhouyx
> Date: 2012-05-07 16:43 +0800
> URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/62557a1336c0
>
> 7166048: Remove the embeded epoll data structure.
> Reviewed-by: alanb
>
> Please verify it. And thanks Alan to review it.
>
>
> On 05/07/2012 03:36 PM, Sean Chou wrote:
>
> Hi Alan,
>
> The new patch is at:
> http://cr.openjdk.java.net/~zhouyx/7166048/webrev.03/ . There are some
> spaces difference between the old and new file. Please take a look again.
>
> On Fri, May 4, 2012 at 3:59 PM, Alan Bateman <Alan.Bateman at oracle.com>wrote:
>
>> 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);
>> }
>>
>>
>
>
> --
> Best Regards,
> Sean Chou
>
>
>
> --
> Yours Charles
>
>
--
Best Regards,
Sean Chou
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20120507/55517459/attachment.html
More information about the nio-dev
mailing list