Summary: | [Bisected]Piglit glx/glx-multithread-shader-compile aborted | ||
---|---|---|---|
Product: | Mesa | Reporter: | lu hua <huax.lu> |
Component: | glsl-compiler | Assignee: | Intel 3D Bugs Mailing List <intel-3d-bugs> |
Status: | VERIFIED FIXED | QA Contact: | |
Severity: | major | ||
Priority: | high | CC: | idr |
Version: | unspecified | ||
Hardware: | All | ||
OS: | Linux (All) | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Attachments: |
Patch to add locks to builtin_builder::initialize()
Patch to add a lock for the builtins singleton |
Description
lu hua
2013-09-11 02:20:48 UTC
Created attachment 85595 [details] [review] Patch to add locks to builtin_builder::initialize() This patch fixes the immediate problem, but I'm concerned that we may need locking at a broader scope... Also, I have no idea why the new system would exhibit the issue while the old one didn't. It looks like the old code would have the exact same problem...though, the timing is probebly different. It also happens on pineview. spec_glsl-1.10_execution_built-in-functions_fs-length-float also fails on pineview with same bisect commit. Run: ./bin/shader_runner generated_tests/spec/glsl-1.10/execution/built-in-functions/fs-length-float.shader_test -auto output: Probe color at (1,0) Expected: 0.000000 1.000000 0.000000 1.000000 Observed: 1.000000 0.000000 0.000000 1.000000 Probe color at (2,0) Expected: 0.000000 1.000000 0.000000 1.000000 Observed: 1.000000 0.000000 0.000000 1.000000 This is a compiler issue. It's not specific to any driver or hardware. Comment on attachment 85595 [details] [review] Patch to add locks to builtin_builder::initialize() Review of attachment 85595 [details] [review]: ----------------------------------------------------------------- I think we also want to protect against one thread calling release() while another thread was still in initialize() or find(). So, perhaps we can use a single mutex at file scope to protect all accesses to the builtins singleton? I've uploaded a patch that implements this. Created attachment 93519 [details] [review] Patch to add a lock for the builtins singleton AFAICT, this bug is still open, since I ran into this same bug when running a fairly recent mesa [0] with very recent piglit [1]. [0] cbecd958a7e36736a4447ebe65e5017e5c0ea4a0 [1] 8eb40d7e829500ff4e22d91745cc7c4f0f6e64d0 I'm not sure what the protocol is for submitting mesa patches or getting them accepted. If I need to send this to a list, too, please let me know. To test it, I ran: /usr/local/piglit/bin/glx-multithread-shader-compile -fbo -auto (In reply to comment #6) > Created attachment 93519 [details] [review] [review] > Patch to add a lock for the builtins singleton > > AFAICT, this bug is still open, since I ran into this same bug when running > a fairly recent mesa [0] with very recent piglit [1]. > > [0] cbecd958a7e36736a4447ebe65e5017e5c0ea4a0 > [1] 8eb40d7e829500ff4e22d91745cc7c4f0f6e64d0 > > I'm not sure what the protocol is for submitting mesa patches or getting > them accepted. If I need to send this to a list, too, please let me know. > > To test it, I ran: > /usr/local/piglit/bin/glx-multithread-shader-compile -fbo -auto Test this patch: The 1st run output: Failed to link: error: uniform `gl_BackLightProduct' declared as type `gl_LightProducts[8]' and type `gl_LightProducts[8]' PIGLIT: {'result': 'pass' } The 2nd run output: PIGLIT: {'result': 'pass' } I guess that is what Kenneth Graunke meant by "we may need locking at a broader scope"... In particular, the glsl_type class needs locking in get_record_instance and get_array_instance to ensure that only one flyweight will be created for a particular type, and that the hash tables won't be corrupted. Probably fixed on master by: commit 2d64e4ffbab0426317f329e2bae22566dc80b98a Author: Chia-I Wu <olvaffe@gmail.com> Date: Wed Aug 20 14:40:29 2014 +0800 glsl: protect glsl_type with a mutex glsl_type has several static hash tables and a static ralloc context. They need to be protected by a mutex as they are not thread-safe. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=69200 Signed-off-by: Chia-I Wu <olv@lunarg.com> Reviewed-by: Brian Paul <brianp@vmware.com> Reviewed-by: Ian Romanick <ian.d.romanick@intel.com> It seems to be passing for me pretty consistently, at any rate. Test on latest mesa master branch, it works well. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.