linuxkpi: Add eventfd_*()

[dumbbell]

Summary

Add <linux/eventfd.h> and expose the eventfd_*() API. This is used by DRM drivers for some time, but the code was commented out so far.

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

Sponsored by: The FreeBSD Foundation

[dumbbell]

I will have to bump __FreeBSD_version to be able to use this API in DRM drivers.

[wulf]

sys/compat/linuxkpi/common/include/linux/eventfd.h

40

We do not expose LKPI API directly to userland. So no need in uapi/linux/eventfd.h

[dumbbell]

sys/compat/linuxkpi/common/include/linux/eventfd.h

40

The goal is the same as D50851: it is to reproduce the same includes as Linux.

I frequently see a compilation failure because a source file relies on namespace pollution in Linux (it uses something but doesn’t include the associated header explicitly). As I don’t want to add a FreeBSD-specific include in DRM drivers, I search that mission inclued somewhere in linuxkpi.

This useless header for us is simply to avoid that work in the future.

[bz]

sys/compat/linuxkpi/common/include/linux/eventfd.h

40

Yes, I have a lot of #ifdef for #includes (and missing self-sufficiency of headers) in wireless drivers from the time before we had the dummy/includes and I would love to get rid of them as well. One man one for me is delay,h.

That said there is zero need to #include a dummy header inside linuxkpi/common. so I agree with @wulf here.

[wulf]

sys/compat/linuxkpi/common/include/linux/eventfd.h

40

If you would grep drm-kmod for <uapi/ string, you would find references on only 3 files: uapi/linux/kfd_ioctl.h, uapi/drm/i915_drm.h and uapi/linux/sched/types.h. So they can't produce any significant namespace pollution.
Generally in LKPI uapi/ file contents should be just merged in to corresponding non-uapi file.

[wulf]

@dumbbell FYI. Take a look at https://github.com/lutzbichler/drm-kmod/tree/pr/6.15. It seems that lutzbichler has advanced to drm-kmod v6.15

[dumbbell]

sys/compat/linuxkpi/common/include/linux/eventfd.h

40

Even if linuxkpi doesn't need this extra header, Linux code might expect this to reproduce the same "graph".

It also makes it easier to compare sources from Linux and FreeBSD if we honor the same layout.

40

I prefer to have the exact same layout to make the porting effort less bumpy. This way, we know the provide the same API. We don't know how headers will be included in the future, so running a grep isn't helpful.

Of course, if you all prefer the opposite, I will change it. But I lost a fair amount of time solving these missing includes. I find it sad if we continue to introduce them.

[dumbbell]

In D50853#1160993, @wulf wrote:

@dumbbell FYI. Take a look at https://github.com/lutzbichler/drm-kmod/tree/pr/6.15. It seems that lutzbichler has advanced to drm-kmod v6.15

I stumbled upon this repository a few times, but I don't know this person and they don't seem to submit pull requests so far.

[dumbbell]

Address comments from @wulf and @bz: don’t introduce a <uapi/linux/eventfd.h> header.

[markj]

Looks ok to me aside from style bugs.

sys/compat/linuxkpi/common/src/linux_eventfd.c

45

55

[dumbbell]

Refresh patch after abandonning D50848.

[christos]

sys/compat/linuxkpi/common/include/linux/eventfd.h

58

We're casting eventfd_ctx to eventfd, and their fields do not match. Does this work properly? Haven't tested it yet.

[emaste]

sys/compat/linuxkpi/common/include/linux/eventfd.h

58

It's only used as an opaque pointer in linuxkpi right now so it's OK. This is the header where we'd define the Linux struct so someone needing to implement it in the future will presumably read the comment above.

Our goal is that if there is a conflict in the future we would like the failure to be clear and obvious. I suppose we could instead do #define eventfd_ctx eventfd but I'm not sure if that is more or less likely to lead to a "good" failure in the event of a conflict.

[christos]

sys/compat/linuxkpi/common/include/linux/eventfd.h

58

This function calls the native FreeBSD eventfd_signal_mask(), which does make use of this argument's fields, isn't that a problem when accessing fields in eventfd_signal_mask(), since we're casting eventfd_ctx to an unrelated struct?

[emaste]

sys/compat/linuxkpi/common/include/linux/eventfd.h

58

The pointer we're passing in is a struct eventfd obtained from lkpi_eventfd_ctx_fdget(). It's just called eventfd_ctx to avoid changing Linux code.

[christos]

sys/compat/linuxkpi/common/include/linux/eventfd.h

58

So, conversely, since this is essentially a FreeBSD eventfd, won't Linux code run into problems if it tries to access the structure, thinking it's a properly populated eventfd_ctx?

[emaste]

sys/compat/linuxkpi/common/include/linux/eventfd.h

58

The only definition of eventfd_ctx is struct eventfd_ctx; above so any Linux code that tries to access struct members will not compile.

[dumbbell]

Also, on Linux, struct eventfd_ctx is defined privately in fs/eventfd.c. The struct fields are never accessed by consumers of the eventfd API.

[christos]

Should this be committed now?

[dumbbell]

In D50853#1248923, @christos wrote:

Should this be committed now?

It will go together with the rest of the eventfd-related patches once they are all approved. Otherwise it’s not very useful alone, at least in the context of DRM drivers.

[emaste]

It will go together with the rest of the eventfd-related patches once they are all approved. Otherwise it’s not very useful alone, at least in the context of DRM drivers.

D50849 and D50850 were approved before updates; I guess you're waiting on @markj to accept again.

[markj]

sys/compat/linuxkpi/common/src/linux_eventfd.c

45

Still applies.

55

Still applies.

[dumbbell]

sys/compat/linuxkpi/common/src/linux_eventfd.c

45

Indeed, I missed your comments… Fixed now.

[dumbbell]

Fix style, as reported by @markj.