eventfd: Add refcounting
Needs ReviewPublic
Actions

Authored by dumbbell on Jun 14 2025, 5:22 PM.

Details

Reviewers

markj

Group Reviewers

linuxkpi

Summary

An eventfd file descriptor can be used by drivers such as DRM drivers through linuxkpi. A driver can hold a reference to such a file regardless of the fact it is used by userland or not.

This patch introduces a refcount in struct eventfd_ctx, plus the eventfd_get() and eventfd_put() functions to acquire and release references. These functions will be used by DRM drivers for instance.

This structure is internal to sys/kern/sys_eventfd.c and not used anywhere else. Thus it is safe to add a field without breaking anything.

This is a requirement to the following patch to drm-kmod:
https://github.com/freebsd/drm-kmod/pull/358

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

dumbbell created this revision.Jun 14 2025, 5:22 PM

Herald added subscribers: olce, imp. · View Herald TranscriptJun 14 2025, 5:22 PM

dumbbell requested review of this revision.Jun 14 2025, 5:22 PM

dumbbell edited the summary of this revision. (Show Details)Jun 14 2025, 5:50 PM

emaste added a parent revision: D50848: eventfd: Rename `struct eventfd` to `struct eventfd_ctx`.Jun 24 2025, 1:32 PM

emaste added a child revision: D50853: linuxkpi: Add eventfd_*().Jun 24 2025, 1:35 PM

This seems ok. It is kind of weird to have an eventfd that is not attached to a file descriptor, since there's nothing you can do with it. IMO it would be better for the drm drivers+linuxkpi to maintain a reference to the struct file instead, e.g., you could cheat and store a struct file * where drm stores a struct eventfd_ctx *, but that's a bit ugly too, and this is not a very intrusive diff.

sys/kern/sys_eventfd.c
106	It would be better to make it an unsigned int, that's the type used by the refcount(9) interface.

This revision is now accepted and ready to land.Sun, Nov 30, 4:41 PM

Refresh patch after abandonning D50848.

This revision now requires review to proceed.Sat, Dec 13, 9:41 PM

dumbbell mentioned this in D50848: eventfd: Rename `struct eventfd` to `struct eventfd_ctx`.Sat, Dec 13, 9:45 PM

dumbbell removed a parent revision: D50848: eventfd: Rename `struct eventfd` to `struct eventfd_ctx`.

dumbbell added a child revision: D50850: eventfd: Add eventfd_signal().

dumbbell removed a child revision: D50850: eventfd: Add eventfd_signal().Sat, Dec 13, 9:49 PM

christos added a subscriber: christos.Thu, Dec 18, 3:21 PM

christos added inline comments.

sys/kern/sys_eventfd.c
149	Why is this named "put"? Just curious, because this is just freeing resources.

markj added inline comments.Thu, Dec 18, 4:32 PM

sys/kern/sys_eventfd.c
149	"put" is a common way to refer to dropping a reference, I guess because it's a natural complement to "get". Similar to hold/release or acquire/release.

dumbbell marked 2 inline comments as done.Thu, Dec 18, 6:40 PM

dumbbell added inline comments.

sys/kern/sys_eventfd.c
149	Indeed, the functions on Linux are `eventfd_ctx_get()` and `eventfd_ctx_put()`. They frequently use this get/put naming convention. `linuxkpi` exposes `eventfd_ctx_{get,put}()`, thus it's possible to use a different naming for FreeBSD. Would you prefer another name?