Re: [Mesa-dev] [PATCH 0/6] st/mesa: add locking around lazy texture/sampler updates
Am 07.10.2017 um 01:01 schrieb Nicolai Hähnle: > On 07.10.2017 00:36, Roland Scheidegger wrote: >> I can't help but think that there'd be none of those problems if those >> views were actually used as specced by gallium initially: all sampler >> views (and surfaces) are per context, and they simply cannot be used in >> another context. > > That is actually the case today, with the exception that when a texture > object is destroyed, all of sampler views are destroyed as well, from > within whatever happens to be the current context. (This can be solved > with some kind of "orphaned" list, but I'd rather not do that in st/mesa.) > > The problem is that when we need a sampler view for a texture, we have > (a) the context pointer and (b) the texture object pointer, and we need > to map those to a sampler view. This happens via a mutable per-texture > object structure, access to which must be protected somehow. Ah I see I didn't look close enough. This is all just about protecting st_texture_object, which is global (as per gl rules of texture objects). So yes locking I guess locking is indeed unavoidable. Roland > Cheers, > Nicolai > > >> >> Roland >> >> Am 06.10.2017 um 22:38 schrieb Nicolai Hähnle: >>> Hi all, >>> >>> This series is a first result of debugging some random crashes in dEQP's >>> multi-threaded EGL tests. >>> >>> The OpenGL spec is pretty clear that application must not modify a >>> texture >>> in one context while simultaneously using it for texturing (or even >>> modifying) it in another context. Texturing simultaneously in multiple >>> contexts is fine, though. >>> >>> This can lead to races when st_finalize_texture is called simultaneously >>> for two contexts, or if two contexts simultaneously attempt to access >>> the >>> sampler view array of a texture. >>> >>> This series should fix most (all?) issues where our internal data >>> structures could become corrupted. I am aware of one remaining corner >>> case that could lead to rendering corruption. It appears rather >>> non-trivial >>> to fix, and it is documented in a comment of patch #5. >>> >>> Please review! >>> Thanks, >>> Nicolai >>> -- >>> src/gallium/auxiliary/util/u_inlines.h | 16 ++- >>> src/gallium/include/pipe/p_context.h | 10 ++ >>> src/mesa/state_tracker/st_atom_sampler.c | 12 +- >>> src/mesa/state_tracker/st_atom_texture.c | 12 -- >>> src/mesa/state_tracker/st_cb_texture.c | 59 +++-- >>> src/mesa/state_tracker/st_manager.c | 3 + >>> src/mesa/state_tracker/st_sampler_view.c | 137 + >>> src/mesa/state_tracker/st_sampler_view.h | 7 ++ >>> src/mesa/state_tracker/st_texture.h | 23 +++- >>> 9 files changed, 205 insertions(+), 74 deletions(-) >>> >>> ___ >>> mesa-dev mailing list >>> mesa-dev@lists.freedesktop.org >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0=aJQCyBglFPt5-boeH0TmdwZPCq7mlewl6Obk3Gs6ckY=uJEnKpDKTfDfspSkPSUNxyVCXwiiizKYJ4xWFybULew= >>> >>> >> > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/6] st/mesa: add locking around lazy texture/sampler updates
On 07.10.2017 00:36, Roland Scheidegger wrote: I can't help but think that there'd be none of those problems if those views were actually used as specced by gallium initially: all sampler views (and surfaces) are per context, and they simply cannot be used in another context. That is actually the case today, with the exception that when a texture object is destroyed, all of sampler views are destroyed as well, from within whatever happens to be the current context. (This can be solved with some kind of "orphaned" list, but I'd rather not do that in st/mesa.) The problem is that when we need a sampler view for a texture, we have (a) the context pointer and (b) the texture object pointer, and we need to map those to a sampler view. This happens via a mutable per-texture object structure, access to which must be protected somehow. Cheers, Nicolai Roland Am 06.10.2017 um 22:38 schrieb Nicolai Hähnle: Hi all, This series is a first result of debugging some random crashes in dEQP's multi-threaded EGL tests. The OpenGL spec is pretty clear that application must not modify a texture in one context while simultaneously using it for texturing (or even modifying) it in another context. Texturing simultaneously in multiple contexts is fine, though. This can lead to races when st_finalize_texture is called simultaneously for two contexts, or if two contexts simultaneously attempt to access the sampler view array of a texture. This series should fix most (all?) issues where our internal data structures could become corrupted. I am aware of one remaining corner case that could lead to rendering corruption. It appears rather non-trivial to fix, and it is documented in a comment of patch #5. Please review! Thanks, Nicolai -- src/gallium/auxiliary/util/u_inlines.h | 16 ++- src/gallium/include/pipe/p_context.h | 10 ++ src/mesa/state_tracker/st_atom_sampler.c | 12 +- src/mesa/state_tracker/st_atom_texture.c | 12 -- src/mesa/state_tracker/st_cb_texture.c | 59 +++-- src/mesa/state_tracker/st_manager.c | 3 + src/mesa/state_tracker/st_sampler_view.c | 137 + src/mesa/state_tracker/st_sampler_view.h | 7 ++ src/mesa/state_tracker/st_texture.h | 23 +++- 9 files changed, 205 insertions(+), 74 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0=aJQCyBglFPt5-boeH0TmdwZPCq7mlewl6Obk3Gs6ckY=uJEnKpDKTfDfspSkPSUNxyVCXwiiizKYJ4xWFybULew= -- Lerne, wie die Welt wirklich ist, Aber vergiss niemals, wie sie sein sollte. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/6] st/mesa: add locking around lazy texture/sampler updates
I can't help but think that there'd be none of those problems if those views were actually used as specced by gallium initially: all sampler views (and surfaces) are per context, and they simply cannot be used in another context. Roland Am 06.10.2017 um 22:38 schrieb Nicolai Hähnle: > Hi all, > > This series is a first result of debugging some random crashes in dEQP's > multi-threaded EGL tests. > > The OpenGL spec is pretty clear that application must not modify a texture > in one context while simultaneously using it for texturing (or even > modifying) it in another context. Texturing simultaneously in multiple > contexts is fine, though. > > This can lead to races when st_finalize_texture is called simultaneously > for two contexts, or if two contexts simultaneously attempt to access the > sampler view array of a texture. > > This series should fix most (all?) issues where our internal data > structures could become corrupted. I am aware of one remaining corner > case that could lead to rendering corruption. It appears rather non-trivial > to fix, and it is documented in a comment of patch #5. > > Please review! > Thanks, > Nicolai > -- > src/gallium/auxiliary/util/u_inlines.h | 16 ++- > src/gallium/include/pipe/p_context.h | 10 ++ > src/mesa/state_tracker/st_atom_sampler.c | 12 +- > src/mesa/state_tracker/st_atom_texture.c | 12 -- > src/mesa/state_tracker/st_cb_texture.c | 59 +++-- > src/mesa/state_tracker/st_manager.c | 3 + > src/mesa/state_tracker/st_sampler_view.c | 137 + > src/mesa/state_tracker/st_sampler_view.h | 7 ++ > src/mesa/state_tracker/st_texture.h | 23 +++- > 9 files changed, 205 insertions(+), 74 deletions(-) > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0=aJQCyBglFPt5-boeH0TmdwZPCq7mlewl6Obk3Gs6ckY=uJEnKpDKTfDfspSkPSUNxyVCXwiiizKYJ4xWFybULew= > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/6] st/mesa: add locking around lazy texture/sampler updates
Hi all, This series is a first result of debugging some random crashes in dEQP's multi-threaded EGL tests. The OpenGL spec is pretty clear that application must not modify a texture in one context while simultaneously using it for texturing (or even modifying) it in another context. Texturing simultaneously in multiple contexts is fine, though. This can lead to races when st_finalize_texture is called simultaneously for two contexts, or if two contexts simultaneously attempt to access the sampler view array of a texture. This series should fix most (all?) issues where our internal data structures could become corrupted. I am aware of one remaining corner case that could lead to rendering corruption. It appears rather non-trivial to fix, and it is documented in a comment of patch #5. Please review! Thanks, Nicolai -- src/gallium/auxiliary/util/u_inlines.h | 16 ++- src/gallium/include/pipe/p_context.h | 10 ++ src/mesa/state_tracker/st_atom_sampler.c | 12 +- src/mesa/state_tracker/st_atom_texture.c | 12 -- src/mesa/state_tracker/st_cb_texture.c | 59 +++-- src/mesa/state_tracker/st_manager.c | 3 + src/mesa/state_tracker/st_sampler_view.c | 137 + src/mesa/state_tracker/st_sampler_view.h | 7 ++ src/mesa/state_tracker/st_texture.h | 23 +++- 9 files changed, 205 insertions(+), 74 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
