Re: [Mesa-dev] [PATCH v3 0/4] Disable glthread if libX11 isn't thread-safe
On Tue, 09 May 2017 09:31:18 -0700 Eric Anholtwrote: > Gregory Hainaut writes: > > > On 5/8/17, Emil Velikov wrote: > [...] > > > > Hello Emil, > > > > Yes you're right. And potentially it can be back-ported to stable > > branch. Besides it would allow to know which applications aren't X > > thread safe. And potentially app owners can fix the issue too. > > > > By the way, I don't have commit access so feel free to push the series :) > > > > On 5/8/17, Eric Anholt wrote: > [...] > [...] > [...] > [...] > [...] > > > > Hello Eric, > > > > Indeed I saw this behavior on Nouveau. I got a crash on > > GetBuffersWithFormat on DRI2 but it was fine on DRI3. I still kept the > > check on GLX/DRI3 because I don't know if we have a strong guarantee > > that X11 is never used. > > I would be surprised if there was a path that hit X through GL calls as > opposed to the window system, with DRI3. GetBuffersWithFormat was the > problem. Tbh, I'm a noob. If you're sure, we can always return true for GLX/DRI3. Otherwise I will put a comment that it is likely fine. Your call. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 0/4] Disable glthread if libX11 isn't thread-safe
Gregory Hainautwrites: > On 5/8/17, Emil Velikov wrote: >> Having a look at Xlib might be good indeed. >> >> Then again, the solution you've proposed looks perfectly reasonable, >> IMHO. It handles the problem _now_ and should also work when/if we >> address Xlib. >> I'll take another look today/tomorrow, but I think the series is >> perfectly fine to land as-is. >> >> Thanks >> Emil > > Hello Emil, > > Yes you're right. And potentially it can be back-ported to stable > branch. Besides it would allow to know which applications aren't X > thread safe. And potentially app owners can fix the issue too. > > By the way, I don't have commit access so feel free to push the series :) > > On 5/8/17, Eric Anholt wrote: >> gregory hainaut writes: >> >>> On Fri, 5 May 2017 17:45:01 +0200 >>> Axel Davy wrote: >>> Hi, There should be very few X11 calls while rendering (basically only at the beginning or end of a frame). Why not just always run these calls in the main thread (and wait for glthread work to finish) ? That's basically what we do for gallium nine. Yours, Axel >>> >>> Hello Axel, >>> >>> Yes it is another possibility. It would requires to track gl calls that >>> end up in X11. >>> I'm not sure if there is an easy way to list all those gl functions. There >>> are at least the >>> draw calls and maybe the clear operations. Besides I'm afraid that we will >>> need to handle >>> various corner cases of the OpenGL API. It is doable but likely more >>> complicated. >> >> General GL calls (draws, clears) won't call X11 except for DRI2's >> GetBuffers. If you're in DRI3, I believe you won't need to worry about >> that at all. >> >> I think this patch is a good start, though. >> > > Hello Eric, > > Indeed I saw this behavior on Nouveau. I got a crash on > GetBuffersWithFormat on DRI2 but it was fine on DRI3. I still kept the > check on GLX/DRI3 because I don't know if we have a strong guarantee > that X11 is never used. I would be surprised if there was a path that hit X through GL calls as opposed to the window system, with DRI3. GetBuffersWithFormat was the problem. signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 0/4] Disable glthread if libX11 isn't thread-safe
On 5/8/17, Emil Velikovwrote: > Having a look at Xlib might be good indeed. > > Then again, the solution you've proposed looks perfectly reasonable, > IMHO. It handles the problem _now_ and should also work when/if we > address Xlib. > I'll take another look today/tomorrow, but I think the series is > perfectly fine to land as-is. > > Thanks > Emil Hello Emil, Yes you're right. And potentially it can be back-ported to stable branch. Besides it would allow to know which applications aren't X thread safe. And potentially app owners can fix the issue too. By the way, I don't have commit access so feel free to push the series :) On 5/8/17, Eric Anholt wrote: > gregory hainaut writes: > >> On Fri, 5 May 2017 17:45:01 +0200 >> Axel Davy wrote: >> >>> Hi, >>> >>> There should be very few X11 calls while rendering (basically only at >>> the beginning or end of a frame). >>> >>> Why not just always run these calls in the main thread (and wait for >>> glthread work to finish) ? >>> >>> That's basically what we do for gallium nine. >>> >>> Yours, >>> >>> Axel >> >> Hello Axel, >> >> Yes it is another possibility. It would requires to track gl calls that >> end up in X11. >> I'm not sure if there is an easy way to list all those gl functions. There >> are at least the >> draw calls and maybe the clear operations. Besides I'm afraid that we will >> need to handle >> various corner cases of the OpenGL API. It is doable but likely more >> complicated. > > General GL calls (draws, clears) won't call X11 except for DRI2's > GetBuffers. If you're in DRI3, I believe you won't need to worry about > that at all. > > I think this patch is a good start, though. > Hello Eric, Indeed I saw this behavior on Nouveau. I got a crash on GetBuffersWithFormat on DRI2 but it was fine on DRI3. I still kept the check on GLX/DRI3 because I don't know if we have a strong guarantee that X11 is never used. Cheers, Gregory ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 0/4] Disable glthread if libX11 isn't thread-safe
gregory hainautwrites: > On Fri, 5 May 2017 17:45:01 +0200 > Axel Davy wrote: > >> Hi, >> >> There should be very few X11 calls while rendering (basically only at >> the beginning or end of a frame). >> >> Why not just always run these calls in the main thread (and wait for >> glthread work to finish) ? >> >> That's basically what we do for gallium nine. >> >> Yours, >> >> Axel > > Hello Axel, > > Yes it is another possibility. It would requires to track gl calls that end > up in X11. > I'm not sure if there is an easy way to list all those gl functions. There > are at least the > draw calls and maybe the clear operations. Besides I'm afraid that we will > need to handle > various corner cases of the OpenGL API. It is doable but likely more > complicated. General GL calls (draws, clears) won't call X11 except for DRI2's GetBuffers. If you're in DRI3, I believe you won't need to worry about that at all. I think this patch is a good start, though. signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 0/4] Disable glthread if libX11 isn't thread-safe
On 5 May 2017 at 18:28, gregory hainautwrote: > On Fri, 5 May 2017 18:17:22 +0100 > Emil Velikov wrote: > >> On 5 May 2017 at 17:58, gregory hainaut wrote: >> > On Fri, 5 May 2017 17:45:01 +0200 >> > Axel Davy wrote: >> > >> [...] >> > >> > Hello Axel, >> > >> > Yes it is another possibility. It would requires to track gl calls that >> > end up in X11. >> > I'm not sure if there is an easy way to list all those gl functions. There >> > are at least the >> > draw calls and maybe the clear operations. Besides I'm afraid that we will >> > need to handle >> > various corner cases of the OpenGL API. It is doable but likely more >> > complicated. >> > >> > There is also the Nvidia way, i.e. forces the driver to XInitThreads X11. >> > I think it >> > can be implemented with the help of constructor attribute. >> > >> Did you trace the above behaviour? What would happen in the following >> scenario: >> - There is no link against libGL/libEGL >> - User gets the dpy primitive w/o calling XInitThreads >> - Then user dlopens libGL/libEGL, which in itself calls XInitThreads >> At that point it's a bit too late isn't it? >> >> -Emil > > Hello, > > No I didn't trace it. > > You right, it is too late. You can/must use LD_PRELOAD to ensure that > libGL/libEGL is loaded first. > > To be honest, the best solution will be to have a thread safe Xlib. No more > hack everywhere. > Having a look at Xlib might be good indeed. Then again, the solution you've proposed looks perfectly reasonable, IMHO. It handles the problem _now_ and should also work when/if we address Xlib. I'll take another look today/tomorrow, but I think the series is perfectly fine to land as-is. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 0/4] Disable glthread if libX11 isn't thread-safe
Hi, There should be very few X11 calls while rendering (basically only at the beginning or end of a frame). Why not just always run these calls in the main thread (and wait for glthread work to finish) ? That's basically what we do for gallium nine. Yours, Axel On 05/05/2017 17:37, Gregory Hainaut wrote: Hello Mesa developers, Following the discussion from https://lists.freedesktop.org/archives/mesa-dev/2017-April/153137.html A check was added to ensure that X11 display can be locked. It should be enough to ensure thread safety between X11 and glthread. I also did the check on DRI3 as I'm not 100% sure that it is really thread safe. v2: based on Nicolai/Matt reviews Add a check on DRI extension version Use C comments :) v3: based on Emil reviews Split the initial first patch into 3 sub patches dri extension / glx / egl Improve error message Improve code readability Just include libX11 on EGL protected by ifdef Thanks you for all the review comments. Best regards, Gregory Hainaut (4): dri: Extend __DRIbackgroundCallableExtensionRec to include a callback that checks for thread safety glx: implement __DRIbackgroundCallableExtension.isThreadSafe egl: implement __DRIbackgroundCallableExtension.isThreadSafe glthread/gallium: require safe_glthread to start glthread include/GL/internal/dri_interface.h | 11 +++ src/egl/drivers/dri2/egl_dri2.c | 28 +++- src/gallium/state_trackers/dri/dri_context.c | 21 + src/glx/dri2_glx.c | 15 ++- src/glx/dri3_glx.c | 15 ++- 5 files changed, 83 insertions(+), 7 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 0/4] Disable glthread if libX11 isn't thread-safe
On Fri, 5 May 2017 18:17:22 +0100 Emil Velikovwrote: > On 5 May 2017 at 17:58, gregory hainaut wrote: > > On Fri, 5 May 2017 17:45:01 +0200 > > Axel Davy wrote: > > > [...] > > > > Hello Axel, > > > > Yes it is another possibility. It would requires to track gl calls that end > > up in X11. > > I'm not sure if there is an easy way to list all those gl functions. There > > are at least the > > draw calls and maybe the clear operations. Besides I'm afraid that we will > > need to handle > > various corner cases of the OpenGL API. It is doable but likely more > > complicated. > > > > There is also the Nvidia way, i.e. forces the driver to XInitThreads X11. I > > think it > > can be implemented with the help of constructor attribute. > > > Did you trace the above behaviour? What would happen in the following > scenario: > - There is no link against libGL/libEGL > - User gets the dpy primitive w/o calling XInitThreads > - Then user dlopens libGL/libEGL, which in itself calls XInitThreads > At that point it's a bit too late isn't it? > > -Emil Hello, No I didn't trace it. You right, it is too late. You can/must use LD_PRELOAD to ensure that libGL/libEGL is loaded first. To be honest, the best solution will be to have a thread safe Xlib. No more hack everywhere. Cheers, Gregory ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 0/4] Disable glthread if libX11 isn't thread-safe
On 5 May 2017 at 17:58, gregory hainautwrote: > On Fri, 5 May 2017 17:45:01 +0200 > Axel Davy wrote: > >> Hi, >> >> There should be very few X11 calls while rendering (basically only at >> the beginning or end of a frame). >> >> Why not just always run these calls in the main thread (and wait for >> glthread work to finish) ? >> >> That's basically what we do for gallium nine. >> >> Yours, >> >> Axel > > Hello Axel, > > Yes it is another possibility. It would requires to track gl calls that end > up in X11. > I'm not sure if there is an easy way to list all those gl functions. There > are at least the > draw calls and maybe the clear operations. Besides I'm afraid that we will > need to handle > various corner cases of the OpenGL API. It is doable but likely more > complicated. > > There is also the Nvidia way, i.e. forces the driver to XInitThreads X11. I > think it > can be implemented with the help of constructor attribute. > Did you trace the above behaviour? What would happen in the following scenario: - There is no link against libGL/libEGL - User gets the dpy primitive w/o calling XInitThreads - Then user dlopens libGL/libEGL, which in itself calls XInitThreads At that point it's a bit too late isn't it? -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 0/4] Disable glthread if libX11 isn't thread-safe
On Fri, 5 May 2017 17:45:01 +0200 Axel Davywrote: > Hi, > > There should be very few X11 calls while rendering (basically only at > the beginning or end of a frame). > > Why not just always run these calls in the main thread (and wait for > glthread work to finish) ? > > That's basically what we do for gallium nine. > > Yours, > > Axel Hello Axel, Yes it is another possibility. It would requires to track gl calls that end up in X11. I'm not sure if there is an easy way to list all those gl functions. There are at least the draw calls and maybe the clear operations. Besides I'm afraid that we will need to handle various corner cases of the OpenGL API. It is doable but likely more complicated. There is also the Nvidia way, i.e. forces the driver to XInitThreads X11. I think it can be implemented with the help of constructor attribute. Cheers, Gregory > On 05/05/2017 17:37, Gregory Hainaut wrote: > > Hello Mesa developers, > > > > Following the discussion from > > https://lists.freedesktop.org/archives/mesa-dev/2017-April/153137.html > > > > A check was added to ensure that X11 display can be locked. It should be > > enough > > to ensure thread safety between X11 and glthread. > > > > I also did the check on DRI3 as I'm not 100% sure that it is really thread > > safe. > > > > > > > > v2: based on Nicolai/Matt reviews > > Add a check on DRI extension version > > Use C comments :) > > > > v3: based on Emil reviews > > Split the initial first patch into 3 sub patches dri extension / glx / egl > > Improve error message > > Improve code readability > > Just include libX11 on EGL protected by ifdef > > > > Thanks you for all the review comments. > > > > Best regards, > > > > Gregory Hainaut (4): > >dri: Extend __DRIbackgroundCallableExtensionRec to include a callback > > that checks for thread safety > >glx: implement __DRIbackgroundCallableExtension.isThreadSafe > >egl: implement __DRIbackgroundCallableExtension.isThreadSafe > >glthread/gallium: require safe_glthread to start glthread > > > > include/GL/internal/dri_interface.h | 11 +++ > > src/egl/drivers/dri2/egl_dri2.c | 28 > > +++- > > src/gallium/state_trackers/dri/dri_context.c | 21 + > > src/glx/dri2_glx.c | 15 ++- > > src/glx/dri3_glx.c | 15 ++- > > 5 files changed, 83 insertions(+), 7 deletions(-) > > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 0/4] Disable glthread if libX11 isn't thread-safe
Hello Mesa developers, Following the discussion from https://lists.freedesktop.org/archives/mesa-dev/2017-April/153137.html A check was added to ensure that X11 display can be locked. It should be enough to ensure thread safety between X11 and glthread. I also did the check on DRI3 as I'm not 100% sure that it is really thread safe. v2: based on Nicolai/Matt reviews Add a check on DRI extension version Use C comments :) v3: based on Emil reviews Split the initial first patch into 3 sub patches dri extension / glx / egl Improve error message Improve code readability Just include libX11 on EGL protected by ifdef Thanks you for all the review comments. Best regards, Gregory Hainaut (4): dri: Extend __DRIbackgroundCallableExtensionRec to include a callback that checks for thread safety glx: implement __DRIbackgroundCallableExtension.isThreadSafe egl: implement __DRIbackgroundCallableExtension.isThreadSafe glthread/gallium: require safe_glthread to start glthread include/GL/internal/dri_interface.h | 11 +++ src/egl/drivers/dri2/egl_dri2.c | 28 +++- src/gallium/state_trackers/dri/dri_context.c | 21 + src/glx/dri2_glx.c | 15 ++- src/glx/dri3_glx.c | 15 ++- 5 files changed, 83 insertions(+), 7 deletions(-) -- 2.11.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev