Closed Bug 1840272 Opened 1 year ago Closed 1 year ago

bug 1833855 breaks non-linux builds because of missing sys/eventfd.h

Categories

(Core :: Widget: Gtk, defect, P3)

Unspecified
OpenBSD
defect

Tracking

()

RESOLVED FIXED
116 Branch
Tracking Status
firefox-esr102 --- unaffected
firefox-esr115 --- unaffected
firefox114 --- unaffected
firefox115 --- unaffected
firefox116 --- fixed

People

(Reporter: gaston, Assigned: stransky)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Chasing several bugs at the same time on m-c, but it seems bug 1833855 broke my builds on OpenBSD:

90:19.58 In file included from /build/buildslave-amd64/mozilla-central-amd64/build/widget/gtk/DMABufSurface.cpp:26:
90:19.58 /build/obj/buildslave/mozilla-central/dist/system_wrappers/sys/eventfd.h:3:15: fatal error: 'sys/eventfd.h' file not found
90:19.58 #include_next <sys/eventfd.h>
90:19.58               ^~~~~~~~~~~~~~~

to my knowledge, eventfd.h is linux-only. Maybe FreeBSD has it..

see http://buildbot.rhaalovely.net/nine/#/builders/3/builds/1739/steps/8/logs/stdio for the full log.. failure has been happening since 2 days so that's why my bet is on bug 1833855.

i'm not sure https://searchfox.org/mozilla-central/source/widget/gtk/DMABufSurface.cpp was built on OpenBSD before bug 1833855.

:stransky, since you are the author of the regressor, bug 1833855, could you take a look?

For more information, please visit BugBot documentation.

Flags: needinfo?(stransky)

my wild guess is that the build failure is due to https://phabricator.services.mozilla.com/D178634 which now builds DMABufSurface.cpp on non-linux. I'll try sprinkling some ifdef linux around the eventfd.h inclusion (seems we cant reuse EVENT__HAVE_SYS_EVENTFD_H from libevent ? maybe a configure test should check for its presence ?) .. shot in the dark.

Edit: won't fly because eventfd is actually used in https://searchfox.org/mozilla-central/source/widget/gtk/DMABufSurface.cpp#159 . sigh.

Summary: bug 1833855 breaks non-linux (BSD) builds → bug 1833855 breaks non-linux (BSD) builds because of missing sys/eventfd.h

Set release status flags based on info from the regressing bug 1833855

afaict eventfd() use was added in bug 1619882, more specially https://hg.mozilla.org/mozilla-central/rev/594ac84515dc - semaphore/shm might be possible but have sandboxing implications on OpenBSD.

dropping the two lines with eventfd (the header inclusion, and the call to eventfd()) allows me to build m-c but i dont know which codepaths exercises this code on X11, so i dunno if it's the right idea. Open to suggestions and patches to test.

(In reply to Landry Breuil (:gaston) from comment #5)

Open to suggestions and patches to test.

Thanks! I'm not sure about the general state of dmabuf on OpenBSD, but generally it's used as fast-path/zero-copy method for WebGL and hardware accelerated video decoding (currently VA-API and V4L2-M2M). If the section DMABUF in about:support reports available, then it should be used for WebGL content. You can double-check with a MOZ_LOG="Dmabuf:5" evn variable.

(In reply to Robert Mader [:rmader] from comment #6)

(In reply to Landry Breuil (:gaston) from comment #5)

Open to suggestions and patches to test.

Thanks! I'm not sure about the general state of dmabuf on OpenBSD, but generally it's used as fast-path/zero-copy method for WebGL and hardware accelerated video decoding (currently VA-API and V4L2-M2M). If the section DMABUF in about:support reports available, then it should be used for WebGL content. You can double-check with a MOZ_LOG="Dmabuf:5" evn variable.

on 115.0b9 it says:

DMABUF	default	unavailable	Wayland support missing	Blocklisted; failure code FEATURE_FAILURE_NO_WAYLAND

to my knowledge, there are no short or long-time plans for wayland on OpenBSD.

to my understanding, to exercise those codepaths, i should try my build from m-c (which removes the eventfd calls) and go to some specific video url ?

(In reply to Landry Breuil (:gaston) from comment #7)

on 115.0b9 it says:

DMABUF	default	unavailable	Wayland support missing	Blocklisted; failure code FEATURE_FAILURE_NO_WAYLAND

DMABUF does not depend on Wayland anymore since bug 1833855 so yes, m-c / nightly would be needed.

to my knowledge, there are no short or long-time plans for wayland on OpenBSD.

Sad to hear.

to my understanding, to exercise those codepaths, i should try my build from m-c (which removes the eventfd calls) and go to some specific video url ?

No, any WebGL page should do. I.e. the good old https://webglsamples.org/aquarium/aquarium.html

For record. I see the same error on Solaris:

44:05.68 In file included from /builds/psumbera/mozilla-central-build/widget/gtk/DMABufSurface.cpp:26,
44:05.68                  from Unified_cpp_widget_gtk0.cpp:47:
44:05.68 /builds/psumbera/mozilla-central-build/obj-x86_64-pc-solaris2.11/dist/system_wrappers/sys/eventfd.h:3:15: fatal error: sys/eventfd.h: No such file or directory
44:05.68     3 | #include_next <sys/eventfd.h>
44:05.68       |               ^~~~~~~~~~~~~~~

So, well, should we add MOZ_DMABUF option and disable that on BDS systems? May that work? Or is dmabuf supported on BDS systems? Is VA-API supported on BDS systems?

Flags: needinfo?(stransky) → needinfo?(petr.sumbera)
Assignee: nobody → stransky

I cannot talk about BSD. Also not exavtly sure what's going on here. libva might work for Solaris but not 100% sure. Adding MOZ_DMABUF option might be the best approach?!

Flags: needinfo?(petr.sumbera)

(In reply to Martin Stránský [:stransky] (ni? me) from comment #10)

So, well, should we add MOZ_DMABUF option and disable that on BDS systems? May that work? Or is dmabuf supported on BDS systems? Is VA-API supported on BDS systems?

i dont know what 'dmabuf' is (a quick googling seems to show that it's a linux-only API ?), and i know we have some ports on OpenBSD that seem to use/support VA-API, whatever that is (but dunno if there's need for kernel low-level support etc, i doubt we have that - for gfx acceleration we have a port of linux's drm 6.1 iirc).

what i know is that if the mozilla code in DMABufSurface.cpp relies on linux-only APIs, then it should only be built on linux.. or behind a configure check making sure that the API is present a configure time ;) or use portable POSIX APIs.

Oh, and it's BSD, not BDS. And solaris is not a BSD either :D

(In reply to Martin Stránský [:stransky] (ni? me) from comment #13)

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=271823 may be related?
More about dmabuf - https://docs.kernel.org/driver-api/dma-buf.html

FreeBSD has made much more progress on the wayland/vaapi/vulkan front than OpenBSD, and after discussing with other devs i have to come to the conclusion that for now there's no support for dmabuf on OpenBSD.

I see two possibilities:

  1. add a new build-time flag
  2. tie dmabuf to Wayland again, i.e. disable it on X11-only builds

Dmabuf support is required for (GPU accelerated) Wayland support, so any platform supporting Wayland will most likely supports dmabuf. It's only X11-only platforms where we can't be sure. Option 2. has the disadvantage that some X11-only users will loose fastpaths, though. So I'd say we should go with option 1.

To be clear here, is is problem with eventfd only? Or does *BSD/Solaris miss all dmabuf related code like libdrm/libgbm?
Because eventfd is not fatal, it's used for dmabuf sync during zero copy VA-API video playback and we don't need it (generally) if surface copy is used.

(In reply to Martin Stránský [:stransky] (ni? me) from comment #16)

To be clear here, is is problem with eventfd only? Or does *BSD/Solaris miss all dmabuf related code like libdrm/libgbm?
Because eventfd is not fatal, it's used for dmabuf sync during zero copy VA-API video playback and we don't need it (generally) if surface copy is used.

i know that we have a (afaict linux-compatible/coming from linux) libdrm version 2.4.115 as of now, and no libgbm. So i wouldnt count on dmabuf support at all on OpenBSD.

(In reply to Martin Stránský [:stransky] (ni? me) from comment #16)

Or does *BSD/Solaris miss all dmabuf related code like libdrm/libgbm?

From what I can see FreeBSD has all of it, eventfd, drm, gbm, Wayland.

testing D181998 via http://buildbot.rhaalovely.net/nine/#/builders/3/builds/1742, afaict configure correctly detects the absence of eventfd function:

/build/obj/buildslave/mozilla-central/config.log:INFO: checking for eventfd... 
/build/obj/buildslave/mozilla-central/config.log:DEBUG: | char eventfd();
/build/obj/buildslave/mozilla-central/config.log:DEBUG: | eventfd();
/build/obj/buildslave/mozilla-central/config.log:DEBUG: | ld.lld: error: undefined symbol: eventfd

i'll check what the resulting build gives.

Cool, Thanks.

Build with provided patch file on Solaris fails with:

25:43.52 In file included from /builds/psumbera/mozilla-central-build/obj-x86_64-pc-solaris2.11/ipc/ipdl/_ipdlheaders/mozilla/dom/PBrowserChild.h:49,
25:43.52                  from /builds/psumbera/mozilla-central-build/obj-x86_64-pc-solaris2.11/dist/include/mozilla/dom/BrowserChild.h:11,
25:43.52                  from /builds/psumbera/mozilla-central-build/uriloader/exthandler/ExternalHelperAppChild.cpp:8,
25:43.52                  from Unified_cpp_uriloader_exthandler0.cpp:11:
25:43.52 /builds/psumbera/mozilla-central-build/obj-x86_64-pc-solaris2.11/dist/include/nsIFrame.h:388:8: note: ‘mozilla::FrameProperties::PropertyType<mozilla::SmallValueHolder<mozilla::FrameBidiData> >’ {aka ‘struct mozilla::FrameBidiData’} declared here
25:43.52   388 | struct FrameBidiData {
25:43.52       |        ^~~~~~~~~~~~~
25:45.15 In file included from Unified_cpp_widget_gtk0.cpp:47:
25:45.15 /builds/psumbera/mozilla-central-build/widget/gtk/DMABufSurface.cpp: In function ‘void SyncDmaBuf(int, uint64_t)’:
25:45.15 /builds/psumbera/mozilla-central-build/widget/gtk/DMABufSurface.cpp:792:50: error: expected primary-expression before ‘struct’
25:45.15   792 | #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)
25:45.15       |                                                  ^~~~~~
25:45.15 /builds/psumbera/mozilla-central-build/widget/gtk/DMABufSurface.cpp:792:50: note: in definition of macro ‘DMA_BUF_IOCTL_SYNC’
25:45.15   792 | #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)
25:45.15       |                                                  ^~~~~~
25:45.20 /builds/psumbera/mozilla-central-build/widget/gtk/DMABufSurface.cpp:792:28: error: ‘_IOW’ was not declared in this scope
25:45.20   792 | #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)
25:45.20       |                            ^~~~
25:45.20 /builds/psumbera/mozilla-central-build/widget/gtk/DMABufSurface.cpp:792:28: note: in definition of macro ‘DMA_BUF_IOCTL_SYNC’
25:45.20   792 | #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)
25:45.20       |                            ^~~~

(In reply to Petr Sumbera from comment #22)

Build with provided patch file on Solaris fails with:

25:43.52 In file included from /builds/psumbera/mozilla-central-build/obj-x86_64-pc-solaris2.11/ipc/ipdl/_ipdlheaders/mozilla/dom/PBrowserChild.h:49,
25:43.52                  from /builds/psumbera/mozilla-central-build/obj-x86_64-pc-solaris2.11/dist/include/mozilla/dom/BrowserChild.h:11,
25:43.52                  from /builds/psumbera/mozilla-central-build/uriloader/exthandler/ExternalHelperAppChild.cpp:8,
25:43.52                  from Unified_cpp_uriloader_exthandler0.cpp:11:
25:43.52 /builds/psumbera/mozilla-central-build/obj-x86_64-pc-solaris2.11/dist/include/nsIFrame.h:388:8: note: ‘mozilla::FrameProperties::PropertyType<mozilla::SmallValueHolder<mozilla::FrameBidiData> >’ {aka ‘struct mozilla::FrameBidiData’} declared here
25:43.52   388 | struct FrameBidiData {
25:43.52       |        ^~~~~~~~~~~~~
25:45.15 In file included from Unified_cpp_widget_gtk0.cpp:47:
25:45.15 /builds/psumbera/mozilla-central-build/widget/gtk/DMABufSurface.cpp: In function ‘void SyncDmaBuf(int, uint64_t)’:
25:45.15 /builds/psumbera/mozilla-central-build/widget/gtk/DMABufSurface.cpp:792:50: error: expected primary-expression before ‘struct’
25:45.15   792 | #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)
25:45.15       |                                                  ^~~~~~
25:45.15 /builds/psumbera/mozilla-central-build/widget/gtk/DMABufSurface.cpp:792:50: note: in definition of macro ‘DMA_BUF_IOCTL_SYNC’
25:45.15   792 | #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)
25:45.15       |                                                  ^~~~~~
25:45.20 /builds/psumbera/mozilla-central-build/widget/gtk/DMABufSurface.cpp:792:28: error: ‘_IOW’ was not declared in this scope
25:45.20   792 | #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)
25:45.20       |                            ^~~~
25:45.20 /builds/psumbera/mozilla-central-build/widget/gtk/DMABufSurface.cpp:792:28: note: in definition of macro ‘DMA_BUF_IOCTL_SYNC’
25:45.20   792 | #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)
25:45.20       |                            ^~~~

for me the _IOW macro is defined in sys/ioccom.h (a low-level kernel header, see https://github.com/openbsd/src/blob/master/sys/sys/ioccom.h#L60) and seems widely used for all ioctl-related functions/code, and afaict my builds don't fail on it like yours. Solaris probably has it in a header which might not be included...

Solaris has _IOW defined also in sys/ioccom.h.

Ok, adding #include <sys/ioccom.h> into widget/gtk/DMABufSurface.cpp resolves the issue. Can you please include it in proposed patch?

i've been able to build & run m-c with D181998, im not sure all processes manage to start as i have lots of crashes on stderr (probably something in the sandboxing config on OpenBSD not properly done to run self-built firefox in a different path, or something failing in gfxtest, will try to fiddle with that) but in about:support i have this for DMABUF:

DMABUF	
default	available		
runtime	failed	Failed to configure	Blocklisted; failure code FEATURE_FAILURE_NO_LIBGBM
HARDWARE_VIDEO_DECODING	
default	available		
runtime	unavailable	Force disabled by gfxInfo	Blocklisted; failure code FEATURE_FAILURE_VIDEO_DECODING_TEST_FAILED
DMABUF_SURFACE_EXPORT	
default	blocked	Blocklisted by gfxInfo	Blocklisted; failure code FEATURE_FAILURE_BROKEN_DRIVER

glxtest manually launched says this:

[18:56] nikki:~/firefox/ $./glxtest                                                                                                                                                                                 
WARNING
cannot access /sys/bus/pci
DRI_DRIVER
iris
VENDOR
Intel
RENDERER
Mesa Intel(R) HD Graphics 5500 (BDW GT2)
VERSION
4.6 (Compatibility Profile) Mesa 22.3.7
TFP
TRUE
DRM_RENDERDEVICE
/dev/dri/renderD128
MESA_VENDOR_ID
0x8086
MESA_DEVICE_ID
0x1616
DDX_DRIVER
modesetting;
TEST_TYPE
EGL

(In reply to Petr Sumbera from comment #25)

Ok, adding #include <sys/ioccom.h> into widget/gtk/DMABufSurface.cpp resolves the issue. Can you please include it in proposed patch?

Do you mind to create a patch based on D181998 for that? I'll submit it to phabricator, just attach it as plain text here.
Thanks.

Flags: needinfo?(petr.sumbera)
Attached patch Bug1840272.patchSplinter Review
Flags: needinfo?(petr.sumbera)
Flags: needinfo?(stransky)

We need to wait to Bug 1839743.

Pushed by stransky@redhat.com:
https://hg.mozilla.org/integration/autoland/rev/15d494dac453
[Linux] Don't break *BSD builds if eventfd is missing r=glandium

<sys/ioccom.h> is missing on Linux. Can you update the patch to include it only if it's available? You may need to update toolkit/moz.configure for it.
Thanks.

Flags: needinfo?(stransky) → needinfo?(petr.sumbera)
Pushed by stransky@redhat.com:
https://hg.mozilla.org/integration/autoland/rev/64e0bb2204d5
[Linux] Don't break *BSD builds if eventfd is missing r=glandium
Flags: needinfo?(petr.sumbera)

(In reply to Petr Sumbera from comment #35)

There is already available HAVE_SYS_IOCCOM_H:
https://searchfox.org/mozilla-central/rev/c0adc2160976e2c118e2e5709d08aac071fddce9/ipc/chromium/src/third_party/libevent/http.c#38
Hopefully this can be used.

No it's not defined.

I don't see in toolkit/moz.configure way to check for existence of header file. Any suggestion please?

Flags: needinfo?(stransky)

It may be

set_define("HAVE_SYS_IOCCOM_H", check_header("sys/ioccom.h"))

It doesn't allow me HAVE_SYS_IOCCOM_H. It says that the key already exists. But when used in DMABufSurface.cpp it doesn't work. I'm trying now without _H.

It's probably because of

https://searchfox.org/mozilla-central/rev/c0adc2160976e2c118e2e5709d08aac071fddce9/config/system-headers.mozbuild#830 ?

The error is:

 0:36.72 checking for eventfd... no
 0:36.78 checking for sys/ioccom.h... yes
 0:36.79 Traceback (most recent call last):
 0:36.79   File "/builds/psumbera/mozilla-central-build/configure.py", line 345, in <module>
 0:36.79     sys.exit(main(sys.argv))
 0:36.79   File "/builds/psumbera/mozilla-central-build/configure.py", line 126, in main
 0:36.79     sandbox.run(os.path.join(os.path.dirname(__file__), "moz.configure"))
 0:36.79   File "/builds/psumbera/mozilla-central-build/python/mozbuild/mozbuild/configure/__init__.py", line 565, in run
 0:36.79     func(*args)
 0:36.79   File "/builds/psumbera/mozilla-central-build/python/mozbuild/mozbuild/configure/__init__.py", line 1120, in _resolve_and_set
 0:36.79     "Cannot add '%s' to configuration: Key already " "exists" % name
 0:36.79 mozbuild.configure.ConfigureError: Cannot add 'HAVE_SYS_IOCCOM_H' to configuration: Key already exists
 Config object not found by mach.

Just try HAVE_SYSIOCCOM_H or so to be different?

Yes, following works:

--- a/toolkit/moz.configure
+++ b/toolkit/moz.configure
@@ -1523,6 +1522,7 @@
     set_define("HAVE_POSIX_FADVISE", check_symbol("posix_fadvise"))
     set_define("HAVE_POSIX_FALLOCATE", check_symbol("posix_fallocate"))
     set_define("HAVE_EVENTFD", check_symbol("eventfd"))
+    set_define("HAVE_SYSIOCCOM_H", check_header("sys/ioccom.h"))

     have_arc4random = check_symbol("arc4random")
     set_define("HAVE_ARC4RANDOM", have_arc4random)
--- a/widget/gtk/DMABufSurface.cpp
+++ b/widget/gtk/DMABufSurface.cpp
@@ -27,6 +27,9 @@
 #  include <sys/eventfd.h>
 #endif
 #include <poll.h>
+#ifdef HAVE_SYSIOCCOM_H
+#  include <sys/ioccom.h>
+#endif
 #include <sys/ioctl.h>

 #include "mozilla/widget/gbm.h"
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 116 Branch
Flags: needinfo?(stransky)

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

(In reply to BugBot [:suhaib / :marco/ :calixte] from comment #45)

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

That's fine I guess.

Flags: needinfo?(stransky)
Status: RESOLVED → REOPENED
Flags: needinfo?(stransky)
Resolution: FIXED → ---
Target Milestone: 116 Branch → ---
Status: REOPENED → NEW
Priority: -- → P3
Summary: bug 1833855 breaks non-linux (BSD) builds because of missing sys/eventfd.h → bug 1833855 breaks non-linux builds because of missing sys/eventfd.h

just to verify this bug, my last-night build on OpenBSD (without any patches) succeeded, per http://buildbot.rhaalovely.net/nine/#/builders/3/builds/1745

Set release status flags based on info from the regressing bug 1833855

Just for the record, it looks like there's active work on bringing Wayland support to OpenBSD: https://bsd.network/users/matthieu/statuses/110679902491627558

Since the BSD fix landed in Firefox 116 and it looks like the Solaris patch will need more work, would it be better to close this bug as fixed in 116 and then open a new bug for patches that will land in Firefox 117?

Flags: needinfo?(stransky)

(In reply to Robert Mader [:rmader] from comment #49)

Just for the record, it looks like there's active work on bringing Wayland support to OpenBSD: https://bsd.network/users/matthieu/statuses/110679902491627558

i'm well aware of this effort, but this is far from complete :) but in some future i have hopes of being able to work on a wayland-enabled firefox on OpenBSD... once all the underlying bits are working and stabilized.

(In reply to Landry Breuil (:gaston) from comment #51)

(In reply to Robert Mader [:rmader] from comment #49)

Just for the record, it looks like there's active work on bringing Wayland support to OpenBSD: https://bsd.network/users/matthieu/statuses/110679902491627558

i'm well aware of this effort, but this is far from complete :) but in some future i have hopes of being able to work on a wayland-enabled firefox on OpenBSD... once all the underlying bits are working and stabilized.

Yes, that would be better.

Flags: needinfo?(stransky)
Status: NEW → RESOLVED
Closed: 1 year ago1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 116 Branch

Filed as Bug 1844285.

See Also: → 1844285

Comment on attachment 9341676 [details]
Bug 1840272 [Solaris] Include sys/ioccom.h if it's available for dmabuf operations r?glandium

Revision D182440 was moved to bug 1844285. Setting attachment 9341676 [details] to obsolete.

Attachment #9341676 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: