epoll interface change and glibc bits ...

Quoted 1 line

epoll does hook f_op->poll() and hence uses the asm/poll.h bits.

It does today. We are talking about "you promise that this will be the case ever after or we'll cut your head off". I have no idea why you're so reluctant since you don't have to maintain any of the user-level bits. And it is not you who has to deal with the fallout of a change when it happens.

If epoll is so different from poll (and this is what I've been told frmo Davide) then there should be a clear separation of the interfaces and all those arguing to unify the data types and constants better should rethink there understanding.

Quoted 8 lines · Ulrich

Quoted 1 line

epoll does hook f_op->poll() and hence uses the asm/poll.h bits.

It does today. We are talking about "you promise that this will be the case ever after or we'll cut your head off". I have no idea why you're so reluctant since you don't have to maintain any of the user-level bits. And it is not you who has to deal with the fallout of a change when it happens.

Quoted 4 lines

If epoll is so different from poll (and this is what I've been told frmo Davide) then there should be a clear separation of the interfaces and all those arguing to unify the data types and constants better should rethink there understanding.

epoll is not really that different from poll, is it?
It delivers edge-triggered versions of the same events poll uses. Or is there something epoll does I'm not aware of?

Quoted 15 lines · Dan Kegel

Quoted 12 lines · Ulrich

Quoted 1 line

epoll does hook f_op->poll() and hence uses the asm/poll.h bits.

It does today. We are talking about "you promise that this will be the case ever after or we'll cut your head off". I have no idea why you're so reluctant since you don't have to maintain any of the user-level bits. And it is not you who has to deal with the fallout of a change when it happens.

If epoll is so different from poll (and this is what I've been told frmo Davide) then there should be a clear separation of the interfaces and all those arguing to unify the data types and constants better should rethink there understanding.

Quoted 3 lines

epoll is not really that different from poll, is it?
It delivers edge-triggered versions of the same events poll uses. Or is there something epoll does I'm not aware of?

The interface ( edge-triggered ) is quite different and we saw in the previous experience how this might lead to confusion for the user. Putting epoll bits inside poll.h will IMHO increase this.

Quoted 3 lines

The interface ( edge-triggered ) is quite different and we saw in the previous experience how this might lead to confusion for the user. Putting epoll bits inside poll.h will IMHO increase this.

The only difference is the edge-triggered nature, though, right?

Quoted 5 lines · Dan Kegel

Quoted 3 lines

The interface ( edge-triggered ) is quite different and we saw in the previous experience how this might lead to confusion for the user. Putting epoll bits inside poll.h will IMHO increase this.

The only difference is the edge-triggered nature, though, right?

Yes, but we have seen that it's enough :)

Quoted 9 lines · Davide Libenzi

Quoted 5 lines · Dan Kegel

Quoted 3 lines

The interface ( edge-triggered ) is quite different and we saw in the previous experience how this might lead to confusion for the user. Putting epoll bits inside poll.h will IMHO increase this.

The only difference is the edge-triggered nature, though, right?

Yes, but we have seen that it's enough :)

I'm not so sure. If the epoll documentation were clear enough (which at the moment, frankly, it isn't), I think there's a good chance users would not be confused
by the difference between level-triggered and edge-triggered events.

I'd be happy to contribute better doc... has the man page for sys_epoll been written yet?

Quoted 2 lines

I'd be happy to contribute better doc... has the man page for sys_epoll been written yet?

http://www.xmailserver.org/linux-patches/epoll.2
http://www.xmailserver.org/linux-patches/epoll_create.2
http://www.xmailserver.org/linux-patches/epoll_ctl.2
http://www.xmailserver.org/linux-patches/epoll_wait.2

it is going to change though with the latest talks about the interface.

Quoted 9 lines · Davide Libenzi

Quoted 2 lines

I'd be happy to contribute better doc... has the man page for sys_epoll been written yet?

http://www.xmailserver.org/linux-patches/epoll.2
http://www.xmailserver.org/linux-patches/epoll_create.2
http://www.xmailserver.org/linux-patches/epoll_ctl.2
http://www.xmailserver.org/linux-patches/epoll_wait.2

it is going to change though with the latest talks about the interface.

Hmm. Right off the bat, I see a terminology problem. The man page says

.SH NAME
epoll \- edge triggered asynchronous I/O facility

That's going to confuse some users. They might think epoll can actually initiate I/O. Better to say

epoll \- edge triggered I/O readiness notification facility

Second, epoll_ctl(2) doesn't define the meaning of the event mask. It should give the allowed bits and define their meanings. If we use the traditional POLLIN etc, we can say
POLLIN - the fd has become ready for reading POLLOUT - the fd has become ready for writing
Note: If epoll tells you e.g. POLLIN, it means that
poll will tell you the same thing, since poll gives the current status, and epoll gives changes in status.

Quoted 10 lines · Dan Kegel

Second, epoll_ctl(2) doesn't define the meaning of the event mask. It should give the allowed bits and define their meanings. If we use the traditional POLLIN etc, we can say
POLLIN - the fd has become ready for reading POLLOUT - the fd has become ready for writing
Note: If epoll tells you e.g. POLLIN, it means that
poll will tell you the same thing, since poll gives the current status, and epoll gives changes in status.

I will have to change man pages also to fit EPOLL* definitions.

Quoted 13 lines · Davide Libenzi

Quoted 10 lines · Dan Kegel

Second, epoll_ctl(2) doesn't define the meaning of the event mask. It should give the allowed bits and define their meanings. If we use the traditional POLLIN etc, we can say
POLLIN - the fd has become ready for reading POLLOUT - the fd has become ready for writing
Note: If epoll tells you e.g. POLLIN, it means that
poll will tell you the same thing, since poll gives the current status, and epoll gives changes in status.

Quoted 1 line

I will have to change man pages also to fit EPOLL* definitions.

IMHO changing from using POLLIN etc. to EPOLLIN will obscure the essential relationship between epoll and poll (namely, that the epoll bits are the time derivative of the poll bits).

I would prefer to continue using POLLIN, etc.

http://www.xmailserver.org/linux-patches/epoll.2
http://www.xmailserver.org/linux-patches/epoll_create.2
http://www.xmailserver.org/linux-patches/epoll_ctl.2
http://www.xmailserver.org/linux-patches/epoll_wait.2

it is going to change though with the latest talks about the interface.

I'm not so sure. If the epoll documentation were clear enough (which at the moment, frankly, it isn't), I think there's a good chance users would not be confused
by the difference between level-triggered and edge-triggered events.

Quoted 7 lines · Davide Libenzi

http://www.xmailserver.org/linux-patches/epoll.2
http://www.xmailserver.org/linux-patches/epoll_create.2
http://www.xmailserver.org/linux-patches/epoll_ctl.2
http://www.xmailserver.org/linux-patches/epoll_wait.2

it is going to change though with the latest talks about the interface.

Remove the waitqueue stuff from epoll.2. It has meaning only to linux kernel developers and noone else.

What about adding an fd twice to the epoll-set? Do you get an error, will it override the previous settings for that fd, will it be ignored, or is it registered twice and you get two results for that fd?

Can two epoll-sets wait for the same fd?

Are events reported to both epoll-fds?

Is the epoll-fd itself poll/epoll/selectable?

Can I build cluster of epoll-sets?

What happens if the epollfd is put into its own fd set?

Can I send the epoll-fd over a unix-socket to another process?

Then, please add more details of how events are generated. You say, that an inactive-to-active transition causes an event. What is the starting point of the collection? (I guess, all transitions

between two epoll_wait calls.) There could be a couple of transi- tions on an fd between two epoll_wait calls. Are these events com- bined into a single reported event or is each single edge reported?

Does an operation on an fd effect the already collected but not yet reported events?

About epoll_wait: it looks like a "read with timeout" call. Is that really necessary or wouldn't a normal read(2) work as well? Similar for epoll_ctl: couldn't a write(2) to the epoll-fd do the same?

Quoted 4 lines · Edgar Toernig

What about adding an fd twice to the epoll-set? Do you get an error, will it override the previous settings for that fd, will it be ignored, or is it registered twice and you get two results for that fd?

You get EEXIST
Well, there's the remote possibility, trying very badly from two threads, to add the same fd twice. It is an harmless condition though.

Quoted 1 line

Is the epoll-fd itself poll/epoll/selectable?

Yes.

Quoted 2 lines

Can I build cluster of epoll-sets?

Uh ?!

Quoted 1 line

What happens if the epollfd is put into its own fd set?

You might find your machine a little bit frozen :)
Either 1) I remove the read lock from poll() or 2) I check the condition at insetion time to avoid it. I very much prefer 2)

Quoted 2 lines

Can I send the epoll-fd over a unix-socket to another process?

I'd say yes. SCM_RIGHTS should simply do an in-kernel file* to remote task descriptor mapping.

Quoted 3 lines

Then, please add more details of how events are generated. You say, that an inactive-to-active transition causes an event. What is the starting point of the collection? (I guess, all transitions

The starting point are the bits found at insertion time.

Quoted 2 lines

Does an operation on an fd effect the already collected but not yet reported events?

You can do two operations on an existing fd. Remove is meaninless for this case. Modify will re-read available bits.

Quoted 4 lines · Davide Libenzi

Quoted 1 line

What happens if the epollfd is put into its own fd set?

You might find your machine a little bit frozen :)
Either 1) I remove the read lock from poll() or 2) I check the condition at insetion time to avoid it. I very much prefer 2)

Hehe, sure. But could become tricky: someone may build a circular chain of epoll-fd-sets.

Quoted 7 lines · Edgar Toernig

Quoted 4 lines · Davide Libenzi

Quoted 1 line

What happens if the epollfd is put into its own fd set?

You might find your machine a little bit frozen :)
Either 1) I remove the read lock from poll() or 2) I check the condition at insetion time to avoid it. I very much prefer 2)

Hehe, sure. But could become tricky: someone may build a circular chain of epoll-fd-sets.

This could be an indication that epoll of an epoll fd should not be allowed. This kind of sucks as epoll event hierarchies appear, at least on the surface, very natural, and better than poll()/epoll.

Another option would be to allow a reasonable depth (2? 3?). The extra checking (depth calculation) only needs to be performed if a file is added or removed where f_op == epoll_f_op.

Quoted 3 lines

You get EEXIST
Well, there's the remote possibility, trying very badly from two threads, to add the same fd twice. It is an harmless condition though.

Just IMHO: I would prefer a different behaviour:

int epoll_ctl(int epfd, int fd, int events)

which registers interest for "events" on "fd" and retuns previous registered events for that fd (implies that the fd is removed when "events" is 0) or -1 for error.

If you don't like it, at least an EP_CTL_GET should be added though.

Btw, what errno for an invalid fd (not epfd)?

Quoted 3 lines

You might find your machine a little bit frozen :)
Either 1) I remove the read lock from poll() or 2) I check the condition at insetion time to avoid it. I very much prefer 2)

Hehe, sure. But could become tricky: someone may build a circular chain of epoll-fd-sets.

Quoted 2 lines

I'd say yes. SCM_RIGHTS should simply do an in-kernel file* to remote task descriptor mapping.

And what happens then? Will the set refers to the fds from the sender process or of fds of the receiving process (which may not even have all those fds open)?

Another btw, what happens on close of an fd? Will it get removed from all epoll-fd-sets automatically?

struct epoll_fd {
	int fd;
	unsigned short events;
	unsigned short revents;
	__uint64_t obj;
};

Quoted 3 lines

And what happens then? Will the set refers to the fds from the sender process or of fds of the receiving process (which may not even have all those fds open)?

Uhm !? It'll refer to files opened on the other process. To handle this correctly we should prevent an epoll file to be passed with SCM_RIGHTS in net/core/scm.c. I mean, no catastrophic things should happen, only the interface won't work correctly. I don't know if the extra handling code is worth, but we should definitely put this inside epoll(2).

Quoted 2 lines

Hehe, sure. But could become tricky: someone may build a circular chain of epoll-fd-sets.

It'll be possible to add epfd1 inside epfd2, not epfd1 inside epfd1.

I have a question about:

Quoted 6 lines

struct epoll_fd {
	int fd;
	unsigned short events;
	unsigned short revents;
	__uint64_t obj;
};

What value does the `fd' field have when a file descriptor being polled has been renumbered (by dup/close or dup2/close or fcntl(F_DUPFD)/close or passing through a unix domain socket)?

If we are honest, the `obj' field is absolutely essential as its the only value which uniquely identifies the file descriptor if you have done anything unusual with the fds.

The fd' field, on the other hand, is not guaranteed to correspond with the correct file descriptor number. So.... perhaps the structure should contain an obj' field and _no_ `fd' field?

This doesn't affect applications. Those which use obj' for something interesting (i.e. a pointer) will have the fd' value stored in the pointed-to data structure, while simple applications can just store the original fd' value in obj' in the first place.

Quoted 1 line

It'll be possible to add epfd1 inside epfd2, not epfd1 inside epfd1.

Beware of overflowing the kernel stack. If epfd4 becomes readable, and wakes up epfd3, which wakes up epfd2, which wakes up epfd1... If that is implemented recursively than I can write malicious code which will crash the kernel. Note that this isn't a cycle. It's possible to code the wakeups so this cannot happen but still have the expected behaviour.

A circular arrangement should be fine, if silly. The semantics are quite logical and don't require special cases: epfd2 becoming readable will trigger epfd1 to become readable if it isn't already. If you make a cycle, that's silly but still behaves as you'd expect. If epfd1 becomes readable, it wakes up epfd1... which is already readable so nothing further happens. Similarly with larger cycles. Assuming you've avoided stack overflow for acyclic graphs, there won't be any problem with cyclic ones.

I have a question about:

Quoted 6 lines

struct epoll_fd {
	int fd;
	unsigned short events;
	unsigned short revents;
	__uint64_t obj;
};

What value does the `fd' field have when a file descriptor being polled has been renumbered (by dup/close or dup2/close or fcntl(F_DUPFD)/close or passing through a unix domain socket)?

If we are honest, the `obj' field is absolutely essential as its the only value which uniquely identifies the file descriptor if you have done anything unusual with the fds.

The fd' field, on the other hand, is not guaranteed to correspond with the correct file descriptor number. So.... perhaps the structure should contain an obj' field and _no_ `fd' field?

This doesn't affect applications. Those which use obj' for something interesting (i.e. a pointer) will have the fd' value stored in the pointed-to data structure, while simple applications can just store the original fd' value in obj' in the first place.

[ ... regarding 'struct epollfd' ('struct epoll_fd'?) ... ] What value does the `fd' field have when a file descriptor being polled has been renumbered (by dup/close or dup2/close or fcntl(F_DUPFD)/close or passing through a unix domain socket)?

The fd' field, on the other hand, is not guaranteed to correspond with the correct file descriptor number. So.... perhaps the structure should contain an obj' field and _no_ `fd' field?

Quoted 4 lines · Jamie Lokier

The fd' field, on the other hand, is not guaranteed to correspond with the correct file descriptor number. So.... perhaps the structure should contain an obj' field and _no_ `fd' field?
...

It's OK. I agree. We can remove the fd from inside the structure and have :

    struct epoll_event {
        unsigned short events;
        unsigned short revents;
        __uint64_t obj;
    };

And the lower size of the structure will help to reduce the amount of memory transfered to userspace. I just saw that adding the extra "obj" member lowered performance of about 15% with crazy tests like Ben's pipetest. This because it creates, on my machine, more than 400000 events per second, and saving memory bandwidth on such conditions is a must. With the "more human" http test performance are about the same.

Quoted 6 lines · Davide Libenzi

And the lower size of the structure will help to reduce the amount of memory transfered to userspace. I just saw that adding the extra "obj" member lowered performance of about 15% with crazy tests like Ben's pipetest. This because it creates, on my machine, more than 400000 events per second, and saving memory bandwidth on such conditions is a must. With the "more human" http test performance are about the same.

I'd be quite surprised if 400,000 word/sec of memory bandwidth can explain a 15% time difference, especially considering all the other things that are done to communicate over a pipe (wakeups etc).

Quoted 11 lines · Davide Libenzi

Quoted 4 lines · Jamie Lokier

The fd' field, on the other hand, is not guaranteed to correspond with the correct file descriptor number. So.... perhaps the structure should contain an obj' field and _no_ `fd' field?
...

It's OK. I agree. We can remove the fd from inside the structure and have :

    struct epoll_event {
        unsigned short events;
        unsigned short revents;
        __uint64_t obj;
    };

Forget any argument I had against removing 'fd'. This sounds good.

Perhaps 'obj' should be named 'userdata'?

     struct epoll_event {
         unsigned short   events;
         unsigned short   revents;
         __uint64_t       userdata;
     };

Quoted 7 lines

Quoted 6 lines

And the lower size of the structure will help to reduce the amount of memory transfered to userspace. I just saw that adding the extra "obj" member lowered performance of about 15% with crazy tests like Ben's pipetest. This because it creates, on my machine, more than 400000 events per second, and saving memory bandwidth on such conditions is a must. With the "more human" http test performance are about the same.

Quoted 3 lines

I'd be quite surprised if 400,000 word/sec of memory bandwidth can explain a 15% time difference, especially considering all the other things that are done to communicate over a pipe (wakeups etc).

Jamie, they were 16 bytes * 400000, and the token passed through the pipe was 12 bytes.

Quoted 11 lines · Davide Libenzi

Quoted 10 lines

Quoted 6 lines

And the lower size of the structure will help to reduce the amount of memory transfered to userspace. I just saw that adding the extra "obj" member lowered performance of about 15% with crazy tests like Ben's pipetest. This because it creates, on my machine, more than 400000 events per second, and saving memory bandwidth on such conditions is a must. With the "more human" http test performance are about the same.

I'd be quite surprised if 400,000 word/sec of memory bandwidth can explain a 15% time difference, especially considering all the other things that are done to communicate over a pipe (wakeups etc).

Quoted 2 lines

Jamie, they were 16 bytes * 400000, and the token passed through the pipe was 12 bytes.

However, it's 4 bytes (1 word) * 400000 _difference_ between the two tests, yes?

Quoted 5 lines · Mark Mielke

Quoted 5 lines

    struct epoll_event {
        unsigned short events;
        unsigned short revents;
        __uint64_t obj;
    };

Quoted 7 lines

Forget any argument I had against removing 'fd'. This sounds good. Perhaps 'obj' should be named 'userdata'?

     struct epoll_event {
         unsigned short   events;
         unsigned short   revents;
         __uint64_t       userdata;
     };

Do we want to have a union instead of a direct 64bit int ?

Quoted 6 lines · Davide Libenzi

Quoted 5 lines · Mark Mielke

Quoted 5 lines

    struct epoll_event {
        unsigned short events;
        unsigned short revents;
        __uint64_t obj;
    };

Quoted 8 lines

Quoted 7 lines

Forget any argument I had against removing 'fd'. This sounds good. Perhaps 'obj' should be named 'userdata'?

     struct epoll_event {
         unsigned short   events;
         unsigned short   revents;
         __uint64_t       userdata;
     };

Do we want to have a union instead of a direct 64bit int ?

I was going to suggest this, except I couldn't figure out what to suggest that it look like... I finally figured that the value could be cast, or wrapped in a union by userspace (although theoretically, this might mean more words than absolutely necessary to initialize on a 32-bit CPU...)

What were you thinking? 1X64 bit or 2X32 bit?

Quoted 1 line

What were you thinking? 1X64 bit or 2X32 bit?

typedef union epoll_obj {
	void *ptr;
	__uint32_t u32[2];
	__uint64_t u64;
} epoll_obj_t;

I'm open to suggestions though. The "ptr" enable me to avoid wierd casts to avoid gcc screaming.

typedef union epoll_obj {
	void *ptr;
	__uint32_t u32[2];
	__uint64_t u64;
} epoll_obj_t;

I'm open to suggestions though. The "ptr" enable me to avoid wierd casts to avoid gcc screaming.

It looks fine to me for as long as we can guarantee that sizeof(void *) will be less than or equal to sizeof(__uint64_t) (relatively safe).

I still prefer 'userdata' over 'obj', but the name of thing is not very important to me.

I'm not sure if this is wise or not, but an 'fd' member might be useful as well:
[...]
For applications that wish to store fd's (probably common due to poll() roots), this would help them avoid casting magic as well. Also, it allows for 64 bit file descriptors if that ever became necessary.

That is a good idea, if we go for the union.

Quoted 7 lines · Davide Libenzi

typedef union epoll_obj {
	void *ptr;
	__uint32_t u32[2];
	__uint64_t u64;
} epoll_obj_t;

I'm open to suggestions though. The "ptr" enable me to avoid wierd casts to avoid gcc screaming.

That makes more sense to me, because it will be fine to use `ptr' even on 128-bit pointer machines when they arrive, yet preserves the property that 64<->32 bit conversion functions don't need to reformat the buffer when running 32-bit applications on a 64-bit CPU... even if the 32-bit application uses the `ptr' field.
Did I just write that? :)

Quoted 8 lines · Jamie Lokier

Quoted 7 lines · Davide Libenzi

typedef union epoll_obj {
	void *ptr;
	__uint32_t u32[2];
	__uint64_t u64;
} epoll_obj_t;

I'm open to suggestions though. The "ptr" enable me to avoid wierd casts to avoid gcc screaming.

Quoted 6 lines

That makes more sense to me, because it will be fine to use `ptr' even on 128-bit pointer machines when they arrive, yet preserves the property that 64<->32 bit conversion functions don't need to reformat the buffer when running 32-bit applications on a 64-bit CPU... even if the 32-bit application uses the `ptr' field.
Did I just write that? :)

The problem with sizeof(void *) being >= sizeof(__uint64_t) is that the data structure is the wrong length. Binary compatibility would not be maintained.

Still... I believe that the days of 128-bit pointers are comfortably far enough away that it does not cause any concerns at all for me. (I suspect the kernel will need quite a few more changes than just this to support 128-bit poitners...)

Quoted 9 lines · Davide Libenzi

Quoted 1 line

What were you thinking? 1X64 bit or 2X32 bit?

typedef union epoll_obj {
	void *ptr;
	__uint32_t u32[2];
	__uint64_t u64;
} epoll_obj_t;

I'm open to suggestions though. The "ptr" enable me to avoid wierd casts to avoid gcc screaming.

It looks fine to me for as long as we can guarantee that sizeof(void *) will be less than or equal to sizeof(__uint64_t) (relatively safe).

I still prefer 'userdata' over 'obj', but the name of thing is not very important to me.

I'm not sure if this is wise or not, but an 'fd' member might be useful as well:

  typedef union epoll_obj {
  	void *ptr;
        int fd;
  	__uint32_t u32[2];
  	__uint64_t u64;
  } epoll_obj_t;

Quoted 6 lines

  typedef union epoll_obj {
  	void *ptr;
        int fd;
  	__uint32_t u32[2];
  	__uint64_t u64;
  } epoll_obj_t;

u32 and u64 sounds more like type name. d32 / d64 ?

Quoted 6 lines · Denis Vlasenko

Quoted 6 lines

  typedef union epoll_obj {
  	void *ptr;
        int fd;
  	__uint32_t u32[2];
  	__uint64_t u64;
  } epoll_obj_t;

Quoted 1 line

u32 and u64 sounds more like type name. d32 / d64 ?

d32 isn't as descriptive as u32 (unsigned). Also if every field had a 'd' in front of it, it would be 'dptr', 'dfd', ...

u32 is fine to me.