Re: [Mesa-dev] [PATCH 7/7] radeonsi: reduce the scope of sel->mutex in si_shader_select_with_key

[Nicolai Hähnle]

On 23.10.2017 13:53, Grazvydas Ignotas wrote:

On Sun, Oct 22, 2017 at 9:33 PM, Nicolai Hähnle  wrote:

From: Nicolai Hähnle 

We only need the lock to guard changes in the variant linked list. The
actual compilation can happen outside the lock, since we use the ready
fence as a guard.
---
  src/gallium/drivers/radeonsi/si_state_shaders.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c 
b/src/gallium/drivers/radeonsi/si_state_shaders.c
index 8c589928b8c..d0bef09748f 100644
--- a/src/gallium/drivers/radeonsi/si_state_shaders.c
+++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
@@ -1618,41 +1618,42 @@ current_not_ready:
 if (thread_index < 0)
 util_queue_fence_wait(>ready);

 mtx_lock(>mutex);

 /* Find the shader variant. */
 for (iter = sel->first_variant; iter; iter = iter->next_variant) {
 /* Don't check the "current" shader. We checked it above. */
 if (current != iter &&
 memcmp(>key, key, sizeof(*key)) == 0) {
+   mtx_unlock(>mutex);
+
 if 
(unlikely(!util_queue_fence_is_signalled(>ready))) {
 /* If it's an optimized shader and its 
compilation has
  * been started but isn't done, use the 
unoptimized
  * shader so as not to cause a stall due to 
compilation.
  */
 if (iter->is_optimized) {
 memset(>opt, 0, sizeof(key->opt));
 mtx_unlock(>mutex);


Double unlock


 goto again;
 }

 util_queue_fence_wait(>ready);
 }

 if (iter->compilation_failed) {
 mtx_unlock(>mutex);


here too.


Right on both counts. No idea what happened there :) I'll fix it.

Cheers,
Nicolai




 return -1; /* skip the draw call */
 }

 state->current = iter;
-   mtx_unlock(>mutex);
 return 0;
 }
 }

 /* Build a new shader. */
 shader = CALLOC_STRUCT(si_shader);
 if (!shader) {
 mtx_unlock(>mutex);
 return -ENOMEM;
 }


Gražvydas



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 7/7] radeonsi: reduce the scope of sel->mutex in si_shader_select_with_key

[Grazvydas Ignotas]

On Sun, Oct 22, 2017 at 9:33 PM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> We only need the lock to guard changes in the variant linked list. The
> actual compilation can happen outside the lock, since we use the ready
> fence as a guard.
> ---
>  src/gallium/drivers/radeonsi/si_state_shaders.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c 
> b/src/gallium/drivers/radeonsi/si_state_shaders.c
> index 8c589928b8c..d0bef09748f 100644
> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
> @@ -1618,41 +1618,42 @@ current_not_ready:
> if (thread_index < 0)
> util_queue_fence_wait(>ready);
>
> mtx_lock(>mutex);
>
> /* Find the shader variant. */
> for (iter = sel->first_variant; iter; iter = iter->next_variant) {
> /* Don't check the "current" shader. We checked it above. */
> if (current != iter &&
> memcmp(>key, key, sizeof(*key)) == 0) {
> +   mtx_unlock(>mutex);
> +
> if 
> (unlikely(!util_queue_fence_is_signalled(>ready))) {
> /* If it's an optimized shader and its 
> compilation has
>  * been started but isn't done, use the 
> unoptimized
>  * shader so as not to cause a stall due to 
> compilation.
>  */
> if (iter->is_optimized) {
> memset(>opt, 0, 
> sizeof(key->opt));
> mtx_unlock(>mutex);

Double unlock

> goto again;
> }
>
> util_queue_fence_wait(>ready);
> }
>
> if (iter->compilation_failed) {
> mtx_unlock(>mutex);

here too.

> return -1; /* skip the draw call */
> }
>
> state->current = iter;
> -   mtx_unlock(>mutex);
> return 0;
> }
> }
>
> /* Build a new shader. */
> shader = CALLOC_STRUCT(si_shader);
> if (!shader) {
> mtx_unlock(>mutex);
> return -ENOMEM;
> }

Gražvydas
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 7/7] radeonsi: reduce the scope of sel->mutex in si_shader_select_with_key

[Nicolai Hähnle]

From: Nicolai Hähnle 

We only need the lock to guard changes in the variant linked list. The
actual compilation can happen outside the lock, since we use the ready
fence as a guard.
---
 src/gallium/drivers/radeonsi/si_state_shaders.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c 
b/src/gallium/drivers/radeonsi/si_state_shaders.c
index 8c589928b8c..d0bef09748f 100644
--- a/src/gallium/drivers/radeonsi/si_state_shaders.c
+++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
@@ -1618,41 +1618,42 @@ current_not_ready:
if (thread_index < 0)
util_queue_fence_wait(>ready);
 
mtx_lock(>mutex);
 
/* Find the shader variant. */
for (iter = sel->first_variant; iter; iter = iter->next_variant) {
/* Don't check the "current" shader. We checked it above. */
if (current != iter &&
memcmp(>key, key, sizeof(*key)) == 0) {
+   mtx_unlock(>mutex);
+
if 
(unlikely(!util_queue_fence_is_signalled(>ready))) {
/* If it's an optimized shader and its 
compilation has
 * been started but isn't done, use the 
unoptimized
 * shader so as not to cause a stall due to 
compilation.
 */
if (iter->is_optimized) {
memset(>opt, 0, sizeof(key->opt));
mtx_unlock(>mutex);
goto again;
}
 
util_queue_fence_wait(>ready);
}
 
if (iter->compilation_failed) {
mtx_unlock(>mutex);
return -1; /* skip the draw call */
}
 
state->current = iter;
-   mtx_unlock(>mutex);
return 0;
}
}
 
/* Build a new shader. */
shader = CALLOC_STRUCT(si_shader);
if (!shader) {
mtx_unlock(>mutex);
return -ENOMEM;
}
@@ -1765,29 +1766,30 @@ current_not_ready:
util_queue_fence_reset(>ready);
 
if (!sel->last_variant) {
sel->first_variant = shader;
sel->last_variant = shader;
} else {
sel->last_variant->next_variant = shader;
sel->last_variant = shader;
}
 
+   mtx_unlock(>mutex);
+
assert(!shader->is_optimized);
si_build_shader_variant(shader, thread_index, false);
 
util_queue_fence_signal(>ready);
 
if (!shader->compilation_failed)
state->current = shader;
 
-   mtx_unlock(>mutex);
return shader->compilation_failed ? -1 : 0;
 }
 
 static int si_shader_select(struct pipe_context *ctx,
struct si_shader_ctx_state *state,
struct si_compiler_ctx_state *compiler_state)
 {
struct si_context *sctx = (struct si_context *)ctx;
struct si_shader_key key;
 
-- 
2.11.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev