Subject: [PATCH] EPOLL_CTL_SET operatin for epoll_ctl when using EPOLLONESHOT

Simplify userspace by eliminating the need to track whether a specific
filehandle was added to epoll already and only needs to be re-arm
after being internally disabled by EPOLLONESHOT.

EPOLL_CLT_SET can be used to add or modify a filehandle in epoll
using the kernel's internal tracking to apply the right equivalent
operation to EPOLL_CTL_ADD or EPOLL_CTL_MOD.

Signed-off-by: Carlo Marcelo Arenas Belon <[email protected]>
---
fs/eventpoll.c | 9 ++++++++-
include/linux/eventpoll.h | 1 +
2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 085c5c0..afa6972 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -799,7 +799,7 @@ static int ep_poll_callback(wait_queue_t *wait, unsigned mode, int sync, void *k
* If the event mask does not contain any poll(2) event, we consider the
* descriptor to be disabled. This condition is likely the effect of the
* EPOLLONESHOT bit that disables the descriptor when an event is received,
- * until the next EPOLL_CTL_MOD will be issued.
+ * until the next EPOLL_CTL_MOD or EPOLL_CTL_SET will be issued.
*/
if (!(epi->event.events & ~EP_PRIVATE_BITS))
goto out_unlock;
@@ -1302,6 +1302,13 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
} else
error = -ENOENT;
break;
+ case EPOLL_CTL_SET:
+ epds.events |= POLLERR | POLLHUP;
+ if (epi)
+ error = ep_modify(ep, epi, &epds);
+ else
+ error = ep_insert(ep, &epds, tfile, fd);
+ break;
}
mutex_unlock(&ep->mtx);

diff --git a/include/linux/eventpoll.h b/include/linux/eventpoll.h
index f6856a5..59758c0 100644
--- a/include/linux/eventpoll.h
+++ b/include/linux/eventpoll.h
@@ -25,6 +25,7 @@
#define EPOLL_CTL_ADD 1
#define EPOLL_CTL_DEL 2
#define EPOLL_CTL_MOD 3
+#define EPOLL_CTL_SET 4

/* Set the One Shot behaviour for the target file descriptor */
#define EPOLLONESHOT (1 << 30)
--
1.6.3.3


2009-09-24 23:46:34

by Davide Libenzi

[permalink] [raw]
Subject: Re: [PATCH] EPOLL_CTL_SET operatin for epoll_ctl when using EPOLLONESHOT

On Thu, 24 Sep 2009, Carlo Marcelo Arenas Belon wrote:

> Simplify userspace by eliminating the need to track whether a specific
> filehandle was added to epoll already and only needs to be re-arm
> after being internally disabled by EPOLLONESHOT.
>
> EPOLL_CLT_SET can be used to add or modify a filehandle in epoll
> using the kernel's internal tracking to apply the right equivalent
> operation to EPOLL_CTL_ADD or EPOLL_CTL_MOD.

Applications (except poorly written ones) usually know what's in the
epoll set and what's not, in their bookkeeping.
I'm sorry, but I see no use of this code.



- Davide

Subject: Re: [PATCH] EPOLL_CTL_SET operatin for epoll_ctl when using EPOLLONESHOT

On Thu, Sep 24, 2009 at 04:46:20PM -0700, Davide Libenzi wrote:
> On Thu, 24 Sep 2009, Carlo Marcelo Arenas Belon wrote:
>
> > Simplify userspace by eliminating the need to track whether a specific
> > filehandle was added to epoll already and only needs to be re-arm
> > after being internally disabled by EPOLLONESHOT.
> >
> > EPOLL_CLT_SET can be used to add or modify a filehandle in epoll
> > using the kernel's internal tracking to apply the right equivalent
> > operation to EPOLL_CTL_ADD or EPOLL_CTL_MOD.
>
> Applications (except poorly written ones) usually know what's in the
> epoll set and what's not, in their bookkeeping.

agree, but since the kernel is also doing that bookkeeping using
ep_find it is possible by using this new operation to avoid doing
that in userspace, saving some cycles (and probably some contention in
multithreaded applications if the epoll set is global) in the hot path.

> I'm sorry, but I see no use of this code.

a very simple, probe of concept echoserver using this (once #defined)
is available from :

http://sajino.sajinet.com.pe/echoserver.c

Carlo

2009-09-25 00:34:26

by Davide Libenzi

[permalink] [raw]
Subject: Re: [PATCH] EPOLL_CTL_SET operatin for epoll_ctl when using EPOLLONESHOT

On Fri, 25 Sep 2009, Carlo Marcelo Arenas Belon wrote:

> On Thu, Sep 24, 2009 at 04:46:20PM -0700, Davide Libenzi wrote:
> > On Thu, 24 Sep 2009, Carlo Marcelo Arenas Belon wrote:
> >
> > > Simplify userspace by eliminating the need to track whether a specific
> > > filehandle was added to epoll already and only needs to be re-arm
> > > after being internally disabled by EPOLLONESHOT.
> > >
> > > EPOLL_CLT_SET can be used to add or modify a filehandle in epoll
> > > using the kernel's internal tracking to apply the right equivalent
> > > operation to EPOLL_CTL_ADD or EPOLL_CTL_MOD.
> >
> > Applications (except poorly written ones) usually know what's in the
> > epoll set and what's not, in their bookkeeping.
>
> agree, but since the kernel is also doing that bookkeeping using
> ep_find it is possible by using this new operation to avoid doing
> that in userspace, saving some cycles (and probably some contention in
> multithreaded applications if the epoll set is global) in the hot path.
>
> > I'm sorry, but I see no use of this code.
>
> a very simple, probe of concept echoserver using this (once #defined)
> is available from :
>
> http://sajino.sajinet.com.pe/echoserver.c

As I said, a poorly designed code. You could easily have done an ADD in
one place, and then have your multishot function handle only MOD.



- Davide