[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 <nhaehnle at gmail.com> wrote:
>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>
>> 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(&sel->ready);
>>
>>          mtx_lock(&sel->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(&iter->key, key, sizeof(*key)) == 0) {
>> +                       mtx_unlock(&sel->mutex);
>> +
>>                          if (unlikely(!util_queue_fence_is_signalled(&iter->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(&key->opt, 0, sizeof(key->opt));
>>                                          mtx_unlock(&sel->mutex);
> 
> Double unlock
> 
>>                                          goto again;
>>                                  }
>>
>>                                  util_queue_fence_wait(&iter->ready);
>>                          }
>>
>>                          if (iter->compilation_failed) {
>>                                  mtx_unlock(&sel->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(&sel->mutex);
>>                          return 0;
>>                  }
>>          }
>>
>>          /* Build a new shader. */
>>          shader = CALLOC_STRUCT(si_shader);
>>          if (!shader) {
>>                  mtx_unlock(&sel->mutex);
>>                  return -ENOMEM;
>>          }
> 
> Gražvydas
>