Re: [patch/rfc] eventfd semaphore-like behavior

Re: [patch/rfc] eventfd semaphore-like behavior

[Martin Sustrik]

Hi,

Speaking of eventfd use cases, we're using it as a bitmap. Everything 
works great, the only problem being that we have to take extra care not 
to set the same bit twice:

fd_counter is 00000000
write (fd, 00001000)
fd_counter is 00001000
write (fd, 00000001)
fd_counter is 00001001
write (fd, 00001000) /* oops! */
fd_counter is 00010001

Would anyone else benefit from a new eventfd flag, say EFD_BITMAP, that 
would cause write(2) to do binary OR rather than + on the counter?

Martin

Martin Sustrik wrote:
>     
> Enter your search termsSubmit search formWeblkml.org
> Date    Wed, 4 Feb 2009 14:58:39 -0800 (PST)
> From    Davide Libenzi <>
> Subject    [patch/rfc] eventfd semaphore-like behavior
> 
> People started using eventfd in scnarios where before where using pipes.
> Many of them use eventfds in a semaphore-like way, like they were before
> with pipes. The problem with eventfd is that a read() on the fd returns
> and wipes the whole counter, making the use of it as semaphore a little
> bit more cumbersome. You can do a read() followed by a write() of
> COUNTER-1, but IMO it's pretty easy and cheap to make this work w/out
> extra steps. This patch introduces a new eventfd flag that tells eventfd
> to only dequeue 1 from the counter, allowing simple read/write to make it
> behave like a semaphore.
> Simple test here:
> 
> http://www.xmailserver.org/eventfd-sem.c
> 
> 
> Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
> 
> 
> - Davide
> 

[patch/rfc] eventfd semaphore-like behavior

[Davide Libenzi]

People started using eventfd in scnarios where before where using pipes. 
Many of them use eventfds in a semaphore-like way, like they were before 
with pipes. The problem with eventfd is that a read() on the fd returns 
and wipes the whole counter, making the use of it as semaphore a little 
bit more cumbersome. You can do a read() followed by a write() of 
COUNTER-1, but IMO it's pretty easy and cheap to make this work w/out 
extra steps. This patch introduces a new eventfd flag that tells eventfd 
to only dequeue 1 from the counter, allowing simple read/write to make it 
behave like a semaphore.
Simple test here:

http://www.xmailserver.org/eventfd-sem.c


Signed-off-by: Davide Libenzi <davidel@xmailserver.org>


- Davide


---
 fs/eventfd.c            |   20 +++++++++++---------
 include/linux/eventfd.h |   12 +++++++++++-
 2 files changed, 22 insertions(+), 10 deletions(-)

Index: linux-2.6.mod/fs/eventfd.c
===================================================================
--- linux-2.6.mod.orig/fs/eventfd.c	2009-02-03 18:13:33.000000000 -0800
+++ linux-2.6.mod/fs/eventfd.c	2009-02-04 12:16:39.000000000 -0800
@@ -28,6 +28,7 @@ struct eventfd_ctx {
 	 * issue a wakeup.
 	 */
 	__u64 count;
+	unsigned int flags;
 };
 
 /*
@@ -87,22 +88,20 @@ static ssize_t eventfd_read(struct file 
 {
 	struct eventfd_ctx *ctx = file->private_data;
 	ssize_t res;
-	__u64 ucnt;
+	__u64 ucnt = 0;
 	DECLARE_WAITQUEUE(wait, current);
 
 	if (count < sizeof(ucnt))
 		return -EINVAL;
 	spin_lock_irq(&ctx->wqh.lock);
 	res = -EAGAIN;
-	ucnt = ctx->count;
-	if (ucnt > 0)
+	if (ctx->count > 0)
 		res = sizeof(ucnt);
 	else if (!(file->f_flags & O_NONBLOCK)) {
 		__add_wait_queue(&ctx->wqh, &wait);
 		for (res = 0;;) {
 			set_current_state(TASK_INTERRUPTIBLE);
 			if (ctx->count > 0) {
-				ucnt = ctx->count;
 				res = sizeof(ucnt);
 				break;
 			}
@@ -117,8 +116,9 @@ static ssize_t eventfd_read(struct file 
 		__remove_wait_queue(&ctx->wqh, &wait);
 		__set_current_state(TASK_RUNNING);
 	}
-	if (res > 0) {
-		ctx->count = 0;
+	if (likely(res > 0)) {
+		ucnt = (ctx->flags & EFD_SEMAPHORE) ? 1 : ctx->count;
+		ctx->count -= ucnt;
 		if (waitqueue_active(&ctx->wqh))
 			wake_up_locked_poll(&ctx->wqh, POLLOUT);
 	}
@@ -166,7 +166,7 @@ static ssize_t eventfd_write(struct file
 		__remove_wait_queue(&ctx->wqh, &wait);
 		__set_current_state(TASK_RUNNING);
 	}
-	if (res > 0) {
+	if (likely(res > 0)) {
 		ctx->count += ucnt;
 		if (waitqueue_active(&ctx->wqh))
 			wake_up_locked_poll(&ctx->wqh, POLLIN);
@@ -207,7 +207,7 @@ SYSCALL_DEFINE2(eventfd2, unsigned int, 
 	BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
 	BUILD_BUG_ON(EFD_NONBLOCK != O_NONBLOCK);
 
-	if (flags & ~(EFD_CLOEXEC | EFD_NONBLOCK))
+	if (flags & ~EFD_FLAGS_SET)
 		return -EINVAL;
 
 	ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
@@ -216,13 +216,14 @@ SYSCALL_DEFINE2(eventfd2, unsigned int, 
 
 	init_waitqueue_head(&ctx->wqh);
 	ctx->count = count;
+	ctx->flags = flags;
 
 	/*
 	 * When we call this, the initialization must be complete, since
 	 * anon_inode_getfd() will install the fd.
 	 */
 	fd = anon_inode_getfd("[eventfd]", &eventfd_fops, ctx,
-			      flags & (O_CLOEXEC | O_NONBLOCK));
+			      flags & EFD_SHARED_FCNTL_FLAGS);
 	if (fd < 0)
 		kfree(ctx);
 	return fd;
@@ -232,3 +233,4 @@ SYSCALL_DEFINE1(eventfd, unsigned int, c
 {
 	return sys_eventfd2(count, 0);
 }
+
Index: linux-2.6.mod/include/linux/eventfd.h
===================================================================
--- linux-2.6.mod.orig/include/linux/eventfd.h	2009-02-03 18:13:33.000000000 -0800
+++ linux-2.6.mod/include/linux/eventfd.h	2009-02-04 12:16:32.000000000 -0800
@@ -13,10 +13,20 @@
 /* For O_CLOEXEC and O_NONBLOCK */
 #include <linux/fcntl.h>
 
-/* Flags for eventfd2.  */
+/*
+ * CAREFUL: Check include/asm-generic/fcntl.h when defining
+ * new flags, since they might collide with O_* ones. We want
+ * to re-use O_* flags that couldn't possibly have a meaning
+ * from eventfd, in order to leave a free define-space for
+ * shared O_* flags.
+ */
+#define EFD_SEMAPHORE (1 << 0)
 #define EFD_CLOEXEC O_CLOEXEC
 #define EFD_NONBLOCK O_NONBLOCK
 
+#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
+#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
+
 struct file *eventfd_fget(int fd);
 int eventfd_signal(struct file *file, int n);
 

Re: [patch/rfc] eventfd semaphore-like behavior

[Andrew Morton]

On Wed, 4 Feb 2009 14:58:39 -0800 (PST)
Davide Libenzi <davidel@xmailserver.org> wrote:

> People started using eventfd in scnarios where before where using pipes. 
> Many of them use eventfds in a semaphore-like way, like they were before 
> with pipes. The problem with eventfd is that a read() on the fd returns 
> and wipes the whole counter, making the use of it as semaphore a little 
> bit more cumbersome. You can do a read() followed by a write() of 
> COUNTER-1, but IMO it's pretty easy and cheap to make this work w/out 
> extra steps. This patch introduces a new eventfd flag that tells eventfd 
> to only dequeue 1 from the counter, allowing simple read/write to make it 
> behave like a semaphore.

Would be nice to spell out what "a sempahore-like way" is.

> Simple test here:
> 
> http://www.xmailserver.org/eventfd-sem.c
> 
> 
> Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
> 
> +/*
> + * CAREFUL: Check include/asm-generic/fcntl.h when defining
> + * new flags, since they might collide with O_* ones. We want
> + * to re-use O_* flags that couldn't possibly have a meaning
> + * from eventfd, in order to leave a free define-space for
> + * shared O_* flags.
> + */
> +#define EFD_SEMAPHORE (1 << 0)
>  #define EFD_CLOEXEC O_CLOEXEC
>  #define EFD_NONBLOCK O_NONBLOCK
>  
> +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)

How would you recommend that userspace determine whether its kernel
supports this feature, bearing in mind that someone might backport this
patch into arbitrarily earlier kernel versions?

What should be userspace's fallback strategy if that support is not
present?

Re: [patch/rfc] eventfd semaphore-like behavior

[Davide Libenzi]

On Wed, 4 Feb 2009, Andrew Morton wrote:

> On Wed, 4 Feb 2009 14:58:39 -0800 (PST)
> Davide Libenzi <davidel@xmailserver.org> wrote:
> 
> > People started using eventfd in scnarios where before where using pipes. 
> > Many of them use eventfds in a semaphore-like way, like they were before 
> > with pipes. The problem with eventfd is that a read() on the fd returns 
> > and wipes the whole counter, making the use of it as semaphore a little 
> > bit more cumbersome. You can do a read() followed by a write() of 
> > COUNTER-1, but IMO it's pretty easy and cheap to make this work w/out 
> > extra steps. This patch introduces a new eventfd flag that tells eventfd 
> > to only dequeue 1 from the counter, allowing simple read/write to make it 
> > behave like a semaphore.
> 
> Would be nice to spell out what "a sempahore-like way" is.

Counter-based resource access? Where a "wait()" returns immediately by 
decrementing the counter by one, if counter is greater than zero. 
Otherwise will wait. And where a "post(count)" will add count to the 
counter releasing the appropriate amount of waiters.
If eventfd the "post" (write) part is fine, while the "wait" (read) does 
not dequeue 1, but the whole counter value.




> > Simple test here:
> > 
> > http://www.xmailserver.org/eventfd-sem.c
> > 
> > 
> > Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
> > 
> > +/*
> > + * CAREFUL: Check include/asm-generic/fcntl.h when defining
> > + * new flags, since they might collide with O_* ones. We want
> > + * to re-use O_* flags that couldn't possibly have a meaning
> > + * from eventfd, in order to leave a free define-space for
> > + * shared O_* flags.
> > + */
> > +#define EFD_SEMAPHORE (1 << 0)
> >  #define EFD_CLOEXEC O_CLOEXEC
> >  #define EFD_NONBLOCK O_NONBLOCK
> >  
> > +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> > +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
> 
> How would you recommend that userspace determine whether its kernel
> supports this feature, bearing in mind that someone might backport this
> patch into arbitrarily earlier kernel versions?
> 
> What should be userspace's fallback strategy if that support is not
> present?

#ifdef EFD_SEMAPHORE, maybe?



- Davide

Re: [patch/rfc] eventfd semaphore-like behavior

[Andrew Morton]

On Wed, 4 Feb 2009 15:18:43 -0800 (PST)
Davide Libenzi <davidel@xmailserver.org> wrote:

> > > Simple test here:
> > > 
> > > http://www.xmailserver.org/eventfd-sem.c
> > > 
> > > 
> > > Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
> > > 
> > > +/*
> > > + * CAREFUL: Check include/asm-generic/fcntl.h when defining
> > > + * new flags, since they might collide with O_* ones. We want
> > > + * to re-use O_* flags that couldn't possibly have a meaning
> > > + * from eventfd, in order to leave a free define-space for
> > > + * shared O_* flags.
> > > + */
> > > +#define EFD_SEMAPHORE (1 << 0)
> > >  #define EFD_CLOEXEC O_CLOEXEC
> > >  #define EFD_NONBLOCK O_NONBLOCK
> > >  
> > > +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> > > +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
> > 
> > How would you recommend that userspace determine whether its kernel
> > supports this feature, bearing in mind that someone might backport this
> > patch into arbitrarily earlier kernel versions?
> > 
> > What should be userspace's fallback strategy if that support is not
> > present?
> 
> #ifdef EFD_SEMAPHORE, maybe?

That's compile-time.  People who ship binaries will probably want
to find a runtime thing for back-compatibility.

Re: [patch/rfc] eventfd semaphore-like behavior

[Davide Libenzi]

On Wed, 4 Feb 2009, Andrew Morton wrote:

> On Wed, 4 Feb 2009 15:18:43 -0800 (PST)
> Davide Libenzi <davidel@xmailserver.org> wrote:
> 
> > > > Simple test here:
> > > > 
> > > > http://www.xmailserver.org/eventfd-sem.c
> > > > 
> > > > 
> > > > Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
> > > > 
> > > > +/*
> > > > + * CAREFUL: Check include/asm-generic/fcntl.h when defining
> > > > + * new flags, since they might collide with O_* ones. We want
> > > > + * to re-use O_* flags that couldn't possibly have a meaning
> > > > + * from eventfd, in order to leave a free define-space for
> > > > + * shared O_* flags.
> > > > + */
> > > > +#define EFD_SEMAPHORE (1 << 0)
> > > >  #define EFD_CLOEXEC O_CLOEXEC
> > > >  #define EFD_NONBLOCK O_NONBLOCK
> > > >  
> > > > +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> > > > +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
> > > 
> > > How would you recommend that userspace determine whether its kernel
> > > supports this feature, bearing in mind that someone might backport this
> > > patch into arbitrarily earlier kernel versions?
> > > 
> > > What should be userspace's fallback strategy if that support is not
> > > present?
> > 
> > #ifdef EFD_SEMAPHORE, maybe?
> 
> That's compile-time.  People who ship binaries will probably want
> to find a runtime thing for back-compatibility.

I dunno. How do they actually do when we add new flags, like the O_ ones?



- Davide

Re: [patch/rfc] eventfd semaphore-like behavior

[Michael Kerrisk]

On Thu, Feb 5, 2009 at 12:27 PM, Davide Libenzi <davidel@xmailserver.org> wrote:
> On Wed, 4 Feb 2009, Andrew Morton wrote:
>
>> On Wed, 4 Feb 2009 15:18:43 -0800 (PST)
>> Davide Libenzi <davidel@xmailserver.org> wrote:
>>
>> > > > Simple test here:
>> > > >
>> > > > http://www.xmailserver.org/eventfd-sem.c
>> > > >
>> > > >
>> > > > Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
>> > > >
>> > > > +/*
>> > > > + * CAREFUL: Check include/asm-generic/fcntl.h when defining
>> > > > + * new flags, since they might collide with O_* ones. We want
>> > > > + * to re-use O_* flags that couldn't possibly have a meaning
>> > > > + * from eventfd, in order to leave a free define-space for
>> > > > + * shared O_* flags.
>> > > > + */
>> > > > +#define EFD_SEMAPHORE (1 << 0)
>> > > >  #define EFD_CLOEXEC O_CLOEXEC
>> > > >  #define EFD_NONBLOCK O_NONBLOCK
>> > > >
>> > > > +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
>> > > > +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
>> > >
>> > > How would you recommend that userspace determine whether its kernel
>> > > supports this feature, bearing in mind that someone might backport this
>> > > patch into arbitrarily earlier kernel versions?
>> > >
>> > > What should be userspace's fallback strategy if that support is not
>> > > present?
>> >
>> > #ifdef EFD_SEMAPHORE, maybe?
>>
>> That's compile-time.  People who ship binaries will probably want
>> to find a runtime thing for back-compatibility.
>
> I dunno. How do they actually do when we add new flags, like the O_ ones?

Maybe I missed something, but I think we're okay, aren't we?  Viz:

a) The glibc eventfd() wrapper invokes sys_eventfd2() (which allows a
flags arg) if it is available, and otherwise fall back to
sys_eventfd() (which does not support a flags arg).

b) If glibc falls back to sys_eventfd(), then it knows to reject
non-zero flags.  (The glibc wrapper already does this.)

c) If the old sys_eventfd2() is given a flag that it doesn't
recognize, then it fails with EINVAL.  (That check is already in the
code.)

So, userspace can determine whether EFD_SEMAPHORE is supported by not
getting an EINVAL error.  (Okay, this falls down for binaries that
bypass glibc's eventfd() wrapper, but there are unlikely to be such
binaries.)

Cheers,

Michael
-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html

Re: [patch/rfc] eventfd semaphore-like behavior

[Andrew Morton]

On Wed, 4 Feb 2009 15:27:45 -0800 (PST)
Davide Libenzi <davidel@xmailserver.org> wrote:

> On Wed, 4 Feb 2009, Andrew Morton wrote:
> 
> > On Wed, 4 Feb 2009 15:18:43 -0800 (PST)
> > Davide Libenzi <davidel@xmailserver.org> wrote:
> > 
> > > > > Simple test here:
> > > > > 
> > > > > http://www.xmailserver.org/eventfd-sem.c
> > > > > 
> > > > > 
> > > > > Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
> > > > > 
> > > > > +/*
> > > > > + * CAREFUL: Check include/asm-generic/fcntl.h when defining
> > > > > + * new flags, since they might collide with O_* ones. We want
> > > > > + * to re-use O_* flags that couldn't possibly have a meaning
> > > > > + * from eventfd, in order to leave a free define-space for
> > > > > + * shared O_* flags.
> > > > > + */
> > > > > +#define EFD_SEMAPHORE (1 << 0)
> > > > >  #define EFD_CLOEXEC O_CLOEXEC
> > > > >  #define EFD_NONBLOCK O_NONBLOCK
> > > > >  
> > > > > +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> > > > > +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
> > > > 
> > > > How would you recommend that userspace determine whether its kernel
> > > > supports this feature, bearing in mind that someone might backport this
> > > > patch into arbitrarily earlier kernel versions?
> > > > 
> > > > What should be userspace's fallback strategy if that support is not
> > > > present?
> > > 
> > > #ifdef EFD_SEMAPHORE, maybe?
> > 
> > That's compile-time.  People who ship binaries will probably want
> > to find a runtime thing for back-compatibility.
> 
> I dunno. How do they actually do when we add new flags, like the O_ ones?
> 

Dunno.  Probably try the syscall and see if it returned -EINVAL.  Does
that work in this case?  If so, it would be sensible to mention this in
the description somewhere as the approved probing method and to
maintain it.

Re: [patch/rfc] eventfd semaphore-like behavior

[Michael Kerrisk]

On Thu, Feb 5, 2009 at 12:55 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Wed, 4 Feb 2009 15:27:45 -0800 (PST)
> Davide Libenzi <davidel@xmailserver.org> wrote:
>
>> On Wed, 4 Feb 2009, Andrew Morton wrote:
>>
>> > On Wed, 4 Feb 2009 15:18:43 -0800 (PST)
>> > Davide Libenzi <davidel@xmailserver.org> wrote:
>> >
>> > > > > Simple test here:
>> > > > >
>> > > > > http://www.xmailserver.org/eventfd-sem.c
>> > > > >
>> > > > >
>> > > > > Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
>> > > > >
>> > > > > +/*
>> > > > > + * CAREFUL: Check include/asm-generic/fcntl.h when defining
>> > > > > + * new flags, since they might collide with O_* ones. We want
>> > > > > + * to re-use O_* flags that couldn't possibly have a meaning
>> > > > > + * from eventfd, in order to leave a free define-space for
>> > > > > + * shared O_* flags.
>> > > > > + */
>> > > > > +#define EFD_SEMAPHORE (1 << 0)
>> > > > >  #define EFD_CLOEXEC O_CLOEXEC
>> > > > >  #define EFD_NONBLOCK O_NONBLOCK
>> > > > >
>> > > > > +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
>> > > > > +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
>> > > >
>> > > > How would you recommend that userspace determine whether its kernel
>> > > > supports this feature, bearing in mind that someone might backport this
>> > > > patch into arbitrarily earlier kernel versions?
>> > > >
>> > > > What should be userspace's fallback strategy if that support is not
>> > > > present?
>> > >
>> > > #ifdef EFD_SEMAPHORE, maybe?
>> >
>> > That's compile-time.  People who ship binaries will probably want
>> > to find a runtime thing for back-compatibility.
>>
>> I dunno. How do they actually do when we add new flags, like the O_ ones?
>>
>
> Dunno.  Probably try the syscall and see if it returned -EINVAL.  Does
> that work in this case?

As youll have seen by now, Ulrich and I noted that it works.

> If so, it would be sensible to mention this in
> the description somewhere as the approved probing method and to
> maintain it.

I'll add something to the man page, as this patch progresses.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html

Re: [patch/rfc] eventfd semaphore-like behavior

[Davide Libenzi]

On Thu, 5 Feb 2009, Michael Kerrisk wrote:

> > Dunno.  Probably try the syscall and see if it returned -EINVAL.  Does
> > that work in this case?
> 
> As youll have seen by now, Ulrich and I noted that it works.
> 
> > If so, it would be sensible to mention this in
> > the description somewhere as the approved probing method and to
> > maintain it.
> 
> I'll add something to the man page, as this patch progresses.

I see we already have stuff like this inside the man pages:

O_CLOEXEC (Since Linux 2.6.23)
          Enable the close-on-exec flag for the new file descriptor.  
          ...

Maybe a similar note for the new flag?



- Davide

Re: [patch/rfc] eventfd semaphore-like behavior

[Michael Kerrisk]

On Thu, Feb 5, 2009 at 2:02 AM, Davide Libenzi <davidel@xmailserver.org> wrote:
> On Thu, 5 Feb 2009, Michael Kerrisk wrote:
>
>> > Dunno.  Probably try the syscall and see if it returned -EINVAL.  Does
>> > that work in this case?
>>
>> As youll have seen by now, Ulrich and I noted that it works.
>>
>> > If so, it would be sensible to mention this in
>> > the description somewhere as the approved probing method and to
>> > maintain it.
>>
>> I'll add something to the man page, as this patch progresses.
>
> I see we already have stuff like this inside the man pages:
>
> O_CLOEXEC (Since Linux 2.6.23)
>          Enable the close-on-exec flag for the new file descriptor.
>          ...
>
> Maybe a similar note for the new flag?

It took a while, but here's the new text in the eventfd.2 (will be in
man-pages-2.36). Could you please ACK, Davide?

       EFD_SEMAPHORE (since Linux 2.6.30)
              Provide semaphore-like semantics  for  reads  from
              the new file descriptor.  See below.
...
              *  If EFD_SEMAPHORE  was  not  specified  and  the
                 eventfd  counter  has  a  nonzero value, then a
                 read(2) returns 8 bytes containing that  value,
                 and the counter's value is reset to zero.

              *  If  EFD_SEMAPHORE was specified and the eventfd
                 counter has a nonzero  value,  then  a  read(2)
                 returns 8 bytes containing the value 1, and the
                 counter's value is decremented by 1.

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/

Re: [patch/rfc] eventfd semaphore-like behavior

[Michael Kerrisk]

On Mon, Aug 30, 2010 at 7:21 AM, Michael Kerrisk <mtk.manpages@gmail.com> wrote:

> It took a while, but here's the new text in the eventfd.2 (will be in
> man-pages-2.36). Could you please ACK, Davide?

Oops: s/2.36/3.26/

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/

Re: [patch/rfc] eventfd semaphore-like behavior

[Davide Libenzi]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1672 bytes --]

On Mon, 30 Aug 2010, Michael Kerrisk wrote:

> On Thu, Feb 5, 2009 at 2:02 AM, Davide Libenzi <davidel@xmailserver.org> wrote:
> > On Thu, 5 Feb 2009, Michael Kerrisk wrote:
> >
> >> > Dunno.  Probably try the syscall and see if it returned -EINVAL.  Does
> >> > that work in this case?
> >>
> >> As youll have seen by now, Ulrich and I noted that it works.
> >>
> >> > If so, it would be sensible to mention this in
> >> > the description somewhere as the approved probing method and to
> >> > maintain it.
> >>
> >> I'll add something to the man page, as this patch progresses.
> >
> > I see we already have stuff like this inside the man pages:
> >
> > O_CLOEXEC (Since Linux 2.6.23)
> >          Enable the close-on-exec flag for the new file descriptor.
> >          ...
> >
> > Maybe a similar note for the new flag?
> 
> It took a while, but here's the new text in the eventfd.2 (will be in
> man-pages-2.36). Could you please ACK, Davide?
> 
>        EFD_SEMAPHORE (since Linux 2.6.30)
>               Provide semaphore-like semantics  for  reads  from
>               the new file descriptor.  See below.
> ...
>               *  If EFD_SEMAPHORE  was  not  specified  and  the
>                  eventfd  counter  has  a  nonzero value, then a
>                  read(2) returns 8 bytes containing that  value,
>                  and the counter's value is reset to zero.
> 
>               *  If  EFD_SEMAPHORE was specified and the eventfd
>                  counter has a nonzero  value,  then  a  read(2)
>                  returns 8 bytes containing the value 1, and the
>                  counter's value is decremented by 1.

Looks fine to me.


- Davide

Re: [patch/rfc] eventfd semaphore-like behavior

[Andrew Morton]

On Thu, 5 Feb 2009 12:59:07 +1300
Michael Kerrisk <mtk.manpages@googlemail.com> wrote:

> >> > > > What should be userspace's fallback strategy if that support is not
> >> > > > present?
> >> > >
> >> > > #ifdef EFD_SEMAPHORE, maybe?
> >> >
> >> > That's compile-time.  People who ship binaries will probably want
> >> > to find a runtime thing for back-compatibility.
> >>
> >> I dunno. How do they actually do when we add new flags, like the O_ ones?
> >>
> >
> > Dunno.  Probably try the syscall and see if it returned -EINVAL.  Does
> > that work in this case?
> 
> As youll have seen by now, Ulrich and I noted that it works.

I think you means "should work" ;)

We're talking about this, yes?

SYSCALL_DEFINE2(eventfd2, unsigned int, count, int, flags)
{
	int fd;
	struct eventfd_ctx *ctx;

	/* Check the EFD_* constants for consistency.  */
	BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
	BUILD_BUG_ON(EFD_NONBLOCK != O_NONBLOCK);

	if (flags & ~(EFD_CLOEXEC | EFD_NONBLOCK))
		return -EINVAL;

That looks like it should work to me.

Re: [patch/rfc] eventfd semaphore-like behavior

[Davide Libenzi]

On Wed, 4 Feb 2009, Andrew Morton wrote:

> On Thu, 5 Feb 2009 12:59:07 +1300
> Michael Kerrisk <mtk.manpages@googlemail.com> wrote:
> 
> > >> > > > What should be userspace's fallback strategy if that support is not
> > >> > > > present?
> > >> > >
> > >> > > #ifdef EFD_SEMAPHORE, maybe?
> > >> >
> > >> > That's compile-time.  People who ship binaries will probably want
> > >> > to find a runtime thing for back-compatibility.
> > >>
> > >> I dunno. How do they actually do when we add new flags, like the O_ ones?
> > >>
> > >
> > > Dunno.  Probably try the syscall and see if it returned -EINVAL.  Does
> > > that work in this case?
> > 
> > As youll have seen by now, Ulrich and I noted that it works.
> 
> I think you means "should work" ;)
> 
> We're talking about this, yes?
> 
> SYSCALL_DEFINE2(eventfd2, unsigned int, count, int, flags)
> {
> 	int fd;
> 	struct eventfd_ctx *ctx;
> 
> 	/* Check the EFD_* constants for consistency.  */
> 	BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
> 	BUILD_BUG_ON(EFD_NONBLOCK != O_NONBLOCK);
> 
> 	if (flags & ~(EFD_CLOEXEC | EFD_NONBLOCK))
> 		return -EINVAL;
> 
> That looks like it should work to me.

I lost you guys. On old kernels you'd get -EINVAL when using the new flag. 
Wasn't it clear? Or is there some side-band traffic in this conversation 
that I missed? :)



- Davide

Re: [patch/rfc] eventfd semaphore-like behavior

[Andrew Morton]

On Wed, 4 Feb 2009 16:25:52 -0800 (PST)
Davide Libenzi <davidel@xmailserver.org> wrote:

> On Wed, 4 Feb 2009, Andrew Morton wrote:
> 
> > On Thu, 5 Feb 2009 12:59:07 +1300
> > Michael Kerrisk <mtk.manpages@googlemail.com> wrote:
> > 
> > > >> > > > What should be userspace's fallback strategy if that support is not
> > > >> > > > present?
> > > >> > >
> > > >> > > #ifdef EFD_SEMAPHORE, maybe?
> > > >> >
> > > >> > That's compile-time.  People who ship binaries will probably want
> > > >> > to find a runtime thing for back-compatibility.
> > > >>
> > > >> I dunno. How do they actually do when we add new flags, like the O_ ones?
> > > >>
> > > >
> > > > Dunno.  Probably try the syscall and see if it returned -EINVAL.  Does
> > > > that work in this case?
> > > 
> > > As youll have seen by now, Ulrich and I noted that it works.
> > 
> > I think you means "should work" ;)
> > 
> > We're talking about this, yes?
> > 
> > SYSCALL_DEFINE2(eventfd2, unsigned int, count, int, flags)
> > {
> > 	int fd;
> > 	struct eventfd_ctx *ctx;
> > 
> > 	/* Check the EFD_* constants for consistency.  */
> > 	BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
> > 	BUILD_BUG_ON(EFD_NONBLOCK != O_NONBLOCK);
> > 
> > 	if (flags & ~(EFD_CLOEXEC | EFD_NONBLOCK))
> > 		return -EINVAL;
> > 
> > That looks like it should work to me.
> 
> I lost you guys. On old kernels you'd get -EINVAL when using the new flag. 
> Wasn't it clear? Or is there some side-band traffic in this conversation 
> that I missed? :)
> 

Well yes, that.  What I was trying to establish here is that we have
thought about (and preferably tested) userspace's back-compatibility
arrangements.

We have now done that.

Re: [patch/rfc] eventfd semaphore-like behavior

[Michael Kerrisk]

On Thu, Feb 5, 2009 at 1:18 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 5 Feb 2009 12:59:07 +1300
> Michael Kerrisk <mtk.manpages@googlemail.com> wrote:
>
>> >> > > > What should be userspace's fallback strategy if that support is not
>> >> > > > present?
>> >> > >
>> >> > > #ifdef EFD_SEMAPHORE, maybe?
>> >> >
>> >> > That's compile-time.  People who ship binaries will probably want
>> >> > to find a runtime thing for back-compatibility.
>> >>
>> >> I dunno. How do they actually do when we add new flags, like the O_ ones?
>> >>
>> >
>> > Dunno.  Probably try the syscall and see if it returned -EINVAL.  Does
>> > that work in this case?
>>
>> As youll have seen by now, Ulrich and I noted that it works.
>
> I think you means "should work" ;)
>
> We're talking about this, yes?
>
> SYSCALL_DEFINE2(eventfd2, unsigned int, count, int, flags)
> {
>        int fd;
>        struct eventfd_ctx *ctx;
>
>        /* Check the EFD_* constants for consistency.  */
>        BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
>        BUILD_BUG_ON(EFD_NONBLOCK != O_NONBLOCK);
>
>        if (flags & ~(EFD_CLOEXEC | EFD_NONBLOCK))
>                return -EINVAL;
>
> That looks like it should work to me.

Yes, that's what we're talking about, plus a similar check that Ulrih
added in the case that glibc's eventfd() falls back to sy_event().

Cheers,

Michael

--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html

Re: [patch/rfc] eventfd semaphore-like behavior

[Ulrich Drepper]

On Wed, Feb 4, 2009 at 3:05 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> How would you recommend that userspace determine whether its kernel
> supports this feature, bearing in mind that someone might backport this
> patch into arbitrarily earlier kernel versions?

   fd = eventfd2 (CNT, EFD_SEMAPHORE);
   if (fd == -1 && errno == EINVAL)
     puts("cannot handled EFD_SEMAPHORE");

Re: [patch/rfc] eventfd semaphore-like behavior

[Michael Kerrisk]

On Thu, Feb 5, 2009 at 12:45 PM, Ulrich Drepper <drepper@gmail.com> wrote:
> On Wed, Feb 4, 2009 at 3:05 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
>> How would you recommend that userspace determine whether its kernel
>> supports this feature, bearing in mind that someone might backport this
>> patch into arbitrarily earlier kernel versions?
>
>   fd = eventfd2 (CNT, EFD_SEMAPHORE);
>   if (fd == -1 && errno == EINVAL)
>     puts("cannot handled EFD_SEMAPHORE");

Crossed mails.  Ulrich put the point more succinctly.


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html

Re: [patch/rfc] eventfd semaphore-like behavior

[Michael Kerrisk]

On Thu, Feb 5, 2009 at 11:58 AM, Davide Libenzi <davidel@xmailserver.org> wrote:
> People started using eventfd in scnarios where before where using pipes.
> Many of them use eventfds in a semaphore-like way, like they were before
> with pipes. The problem with eventfd is that a read() on the fd returns
> and wipes the whole counter, making the use of it as semaphore a little
> bit more cumbersome. You can do a read() followed by a write() of
> COUNTER-1, but IMO it's pretty easy and cheap to make this work w/out
> extra steps. This patch introduces a new eventfd flag that tells eventfd
> to only dequeue 1 from the counter, allowing simple read/write to make it
> behave like a semaphore.
> Simple test here:
>
> http://www.xmailserver.org/eventfd-sem.c
>
>
> Signed-off-by: Davide Libenzi <davidel@xmailserver.org>

Tested-by: Michael Kerrisk <mtk.manpages@gmail.com>

Applied this patch against 2.6.29-rc3, and it works as I would expect.


A question or two.... This change is rather specific to a single use
case, so I wonder

a) Are there use cases that require the ability to read an arbitrary
number of units off the eventfd -- i.e., read N units off the eventfd,
rather than just 1?

b) Is it desirable to be able to toggle the EFD_SEMAPHORE behavior on
and off for an eventfd?

It's difficult to see how these use cases could be accommodated in the
current API, but I just thought it worth raising the ideas.

Cheers,

Michael

> - Davide
>
>
> ---
>  fs/eventfd.c            |   20 +++++++++++---------
>  include/linux/eventfd.h |   12 +++++++++++-
>  2 files changed, 22 insertions(+), 10 deletions(-)
>
> Index: linux-2.6.mod/fs/eventfd.c
> ===================================================================
> --- linux-2.6.mod.orig/fs/eventfd.c     2009-02-03 18:13:33.000000000 -0800
> +++ linux-2.6.mod/fs/eventfd.c  2009-02-04 12:16:39.000000000 -0800
> @@ -28,6 +28,7 @@ struct eventfd_ctx {
>         * issue a wakeup.
>         */
>        __u64 count;
> +       unsigned int flags;
>  };
>
>  /*
> @@ -87,22 +88,20 @@ static ssize_t eventfd_read(struct file
>  {
>        struct eventfd_ctx *ctx = file->private_data;
>        ssize_t res;
> -       __u64 ucnt;
> +       __u64 ucnt = 0;
>        DECLARE_WAITQUEUE(wait, current);
>
>        if (count < sizeof(ucnt))
>                return -EINVAL;
>        spin_lock_irq(&ctx->wqh.lock);
>        res = -EAGAIN;
> -       ucnt = ctx->count;
> -       if (ucnt > 0)
> +       if (ctx->count > 0)
>                res = sizeof(ucnt);
>        else if (!(file->f_flags & O_NONBLOCK)) {
>                __add_wait_queue(&ctx->wqh, &wait);
>                for (res = 0;;) {
>                        set_current_state(TASK_INTERRUPTIBLE);
>                        if (ctx->count > 0) {
> -                               ucnt = ctx->count;
>                                res = sizeof(ucnt);
>                                break;
>                        }
> @@ -117,8 +116,9 @@ static ssize_t eventfd_read(struct file
>                __remove_wait_queue(&ctx->wqh, &wait);
>                __set_current_state(TASK_RUNNING);
>        }
> -       if (res > 0) {
> -               ctx->count = 0;
> +       if (likely(res > 0)) {
> +               ucnt = (ctx->flags & EFD_SEMAPHORE) ? 1 : ctx->count;
> +               ctx->count -= ucnt;
>                if (waitqueue_active(&ctx->wqh))
>                        wake_up_locked_poll(&ctx->wqh, POLLOUT);
>        }
> @@ -166,7 +166,7 @@ static ssize_t eventfd_write(struct file
>                __remove_wait_queue(&ctx->wqh, &wait);
>                __set_current_state(TASK_RUNNING);
>        }
> -       if (res > 0) {
> +       if (likely(res > 0)) {
>                ctx->count += ucnt;
>                if (waitqueue_active(&ctx->wqh))
>                        wake_up_locked_poll(&ctx->wqh, POLLIN);
> @@ -207,7 +207,7 @@ SYSCALL_DEFINE2(eventfd2, unsigned int,
>        BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
>        BUILD_BUG_ON(EFD_NONBLOCK != O_NONBLOCK);
>
> -       if (flags & ~(EFD_CLOEXEC | EFD_NONBLOCK))
> +       if (flags & ~EFD_FLAGS_SET)
>                return -EINVAL;
>
>        ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
> @@ -216,13 +216,14 @@ SYSCALL_DEFINE2(eventfd2, unsigned int,
>
>        init_waitqueue_head(&ctx->wqh);
>        ctx->count = count;
> +       ctx->flags = flags;
>
>        /*
>         * When we call this, the initialization must be complete, since
>         * anon_inode_getfd() will install the fd.
>         */
>        fd = anon_inode_getfd("[eventfd]", &eventfd_fops, ctx,
> -                             flags & (O_CLOEXEC | O_NONBLOCK));
> +                             flags & EFD_SHARED_FCNTL_FLAGS);
>        if (fd < 0)
>                kfree(ctx);
>        return fd;
> @@ -232,3 +233,4 @@ SYSCALL_DEFINE1(eventfd, unsigned int, c
>  {
>        return sys_eventfd2(count, 0);
>  }
> +
> Index: linux-2.6.mod/include/linux/eventfd.h
> ===================================================================
> --- linux-2.6.mod.orig/include/linux/eventfd.h  2009-02-03 18:13:33.000000000 -0800
> +++ linux-2.6.mod/include/linux/eventfd.h       2009-02-04 12:16:32.000000000 -0800
> @@ -13,10 +13,20 @@
>  /* For O_CLOEXEC and O_NONBLOCK */
>  #include <linux/fcntl.h>
>
> -/* Flags for eventfd2.  */
> +/*
> + * CAREFUL: Check include/asm-generic/fcntl.h when defining
> + * new flags, since they might collide with O_* ones. We want
> + * to re-use O_* flags that couldn't possibly have a meaning
> + * from eventfd, in order to leave a free define-space for
> + * shared O_* flags.
> + */
> +#define EFD_SEMAPHORE (1 << 0)
>  #define EFD_CLOEXEC O_CLOEXEC
>  #define EFD_NONBLOCK O_NONBLOCK
>
> +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
> +
>  struct file *eventfd_fget(int fd);
>  int eventfd_signal(struct file *file, int n);
>
>



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html

Re: [patch/rfc] eventfd semaphore-like behavior

[Davide Libenzi]

On Mon, 9 Feb 2009, Michael Kerrisk wrote:

> On Thu, Feb 5, 2009 at 11:58 AM, Davide Libenzi <davidel@xmailserver.org> wrote:
> > People started using eventfd in scnarios where before where using pipes.
> > Many of them use eventfds in a semaphore-like way, like they were before
> > with pipes. The problem with eventfd is that a read() on the fd returns
> > and wipes the whole counter, making the use of it as semaphore a little
> > bit more cumbersome. You can do a read() followed by a write() of
> > COUNTER-1, but IMO it's pretty easy and cheap to make this work w/out
> > extra steps. This patch introduces a new eventfd flag that tells eventfd
> > to only dequeue 1 from the counter, allowing simple read/write to make it
> > behave like a semaphore.
> > Simple test here:
> >
> > http://www.xmailserver.org/eventfd-sem.c
> >
> >
> > Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
> 
> Tested-by: Michael Kerrisk <mtk.manpages@gmail.com>
> 
> Applied this patch against 2.6.29-rc3, and it works as I would expect.
> 
> 
> A question or two.... This change is rather specific to a single use
> case, so I wonder
> 
> a) Are there use cases that require the ability to read an arbitrary
> number of units off the eventfd -- i.e., read N units off the eventfd,
> rather than just 1?

Not that I can think of.


> b) Is it desirable to be able to toggle the EFD_SEMAPHORE behavior on
> and off for an eventfd?

This I'd say no. A synchronization entity in all decently sane sw design 
I've ever seen, remains the same in behaviour and it is never changed 
runtime to behave in different ways.


> It's difficult to see how these use cases could be accommodated in the
> current API, but I just thought it worth raising the ideas.

Since read/write semantics cannot be changed (besides, like the semaphore 
change, for the amount of "data" dequeued), deeper changes to the 
interface will have to go via ioctl(). But I don't see any reason to go 
that way ATM.



- Davide