[PATCH] EPOLL_CTL_SET operatin for epoll_ctl when using EPOLLONESHOT

[PATCH] EPOLL_CTL_SET operatin for epoll_ctl when using EPOLLONESHOT

[Carlo Marcelo Arenas Belon]

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 <carenas@sajinet.com.pe>
---
 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

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

[Davide Libenzi]

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

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

[Carlo Marcelo Arenas Belon]

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

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

[Davide Libenzi]

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