Bug 236607 - [GTK][WPE] Use drm render nodes in GbmDevice and make the fd global to the process
Summary: [GTK][WPE] Use drm render nodes in GbmDevice and make the fd global to the pr...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: ANGLE (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alejandro G. Castro
URL:
Keywords: InRadar
Depends on:
Blocks: DMA-BUF
  Show dependency treegraph
 
Reported: 2022-02-14 13:14 PST by Alejandro G. Castro
Modified: 2022-03-09 04:25 PST (History)
7 users (show)

See Also:


Attachments
Patch (3.36 KB, patch)
2022-02-14 13:22 PST, Alejandro G. Castro
no flags Details | Formatted Diff | Diff
Patch (3.63 KB, patch)
2022-02-15 11:21 PST, Alejandro G. Castro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alejandro G. Castro 2022-02-14 13:14:42 PST
We just need to use render nodes from the library, we do not need the other apis of the primary device file.
Comment 1 Alejandro G. Castro 2022-02-14 13:22:02 PST
Created attachment 451937 [details]
Patch
Comment 2 Zan Dobersek 2022-02-14 23:23:38 PST
Comment on attachment 451937 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=451937&action=review

> Source/WebCore/platform/graphics/gbm/GBMDevice.cpp:61
> +    if (globalFd == -1)
> +        getDeviceFd();

This is now open to cross-thread race conditions.

> Source/WebCore/platform/graphics/gbm/GBMDevice.cpp:75
> -int GBMDevice::getDeviceFd()
> +void GBMDevice::getDeviceFd()

This should be moved out into a standalone static function that internally uses std::call_once() to protect against separate threads racing to open the render node.
Comment 3 Zan Dobersek 2022-02-14 23:32:49 PST
Comment on attachment 451937 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=451937&action=review

> Source/WebCore/platform/graphics/gbm/GBMDevice.cpp:81
>      if (numDevices <=0)

Poor spacing.

> Source/WebCore/platform/graphics/gbm/GBMDevice.cpp:89
> +        globalFd = open(device->nodes[DRM_NODE_RENDER], O_RDWR);

O_CLOEXEC should also be used.
Comment 4 Alejandro G. Castro 2022-02-15 00:31:56 PST
(In reply to Zan Dobersek from comment #2)
> Comment on attachment 451937 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=451937&action=review
> 
> > Source/WebCore/platform/graphics/gbm/GBMDevice.cpp:61
> > +    if (globalFd == -1)
> > +        getDeviceFd();
> 
> This is now open to cross-thread race conditions.
> 

Good point, we need to protect the first access.

> > Source/WebCore/platform/graphics/gbm/GBMDevice.cpp:75
> > -int GBMDevice::getDeviceFd()
> > +void GBMDevice::getDeviceFd()
> 
> This should be moved out into a standalone static function that internally
> uses std::call_once() to protect against separate threads racing to open the
> render node.

Thanks for the comments! I'll update the patch.
Comment 5 Alejandro G. Castro 2022-02-15 11:21:49 PST
Created attachment 452063 [details]
Patch
Comment 6 Zan Dobersek (Reviews) 2022-02-15 21:57:55 PST
Comment on attachment 452063 [details]
Patch

Great, thanks.
Comment 7 EWS 2022-02-16 02:57:13 PST
Committed r289885 (247323@main): <https://commits.webkit.org/247323@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 452063 [details].
Comment 8 Radar WebKit Bug Importer 2022-02-16 02:58:15 PST
<rdar://problem/89016322>