EventFd::defuse doesn't work

[dreiss]

Documentation for eventfd: https://man7.org/linux/man-pages/man2/eventfd.2.html
Linux source code: https://github.com/torvalds/linux/blob/83814698cf48ce3aadc5

Both show that "write" adds the written value to the internal count. It will not reset the count or cause the next read call to block.

I think the only way to implement "defuse" would be as an alias for "read", which probably doesn't make sense and wouldn't even work for semaphore eventfds.

[SteveLauC]

Thanks for the report! Sorry for the late response!

It indeed won't reset the kernel counter, and since it add 0 to the counter, this method looks like a no-op.

I think the only way to implement "defuse" would be as an alias for "read", which probably doesn't make sense and wouldn't even work for semaphore eventfds.

Yeah, I agree.

Seems that we should simply remove this API? cc @JonathanWoollett-Light

[Gnurou]

Just chiming in to confirm that I was also a bit confused by the behavior (and existence) of defuse, and after testing it to make sure it indeed looks like a no-op that could induce users into error.

I guess that arm does not make much sense in that context either, in particular for eventfds using semaphore semantics. One would rather implement arm/defuse behavior using a new type wrapping EventFd and making sure EFD_SEMAPHORE is not used.

[SteveLauC]

I guess that arm does not make much sense in that context either, in particular for eventfds using semaphore semantics.

Would you like to elaborate on it a bit? Though I do agree if defuse() will be removed, then arm() should be removed as well, the basic read()/write() APIs would be already sufficient.

[Gnurou]

The first reason is symmetry - if defuse is removed, it makes less (if at all) sense for arm to exist IMHO.

The second thing I was thinking about is that if one creates an EventFd with EFD_SEMAPHORE, then read will only decrement the counter by 1, meaning that you need to read as many times as you have armed. I suspect this leaves room for confusion. write/read, on the other hand, is more explicit on what happens to the counter and a direct application of the eventfd documentation, meaning nobody can blame nix for confusing them with new APIs. 😛

[JonathanWoollett-Light]

@SteveLauC I think removing it is reasonable, I don't remember any reason to argue against this.

[SteveLauC]

Get it, I will remove these 2 interfaces then, thanks.