[Mesa-dev] [PATCH 3/4] radeonsi: don't lock shader cache mutex during compilation

[Marek Olšák]

On Sat, Jul 2, 2016 at 10:51 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 29.06.2016 18:32, Marek Olšák wrote:
>>
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> to allow multiple shaders to be compiled simultaneously.
>> ---
>>   src/gallium/drivers/radeonsi/si_state_shaders.c | 21
>> ++++++++++++++++-----
>>   1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c
>> b/src/gallium/drivers/radeonsi/si_state_shaders.c
>> index 0fc3db9..e433055 100644
>> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
>> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
>> @@ -179,8 +179,14 @@ static bool si_shader_cache_insert_shader(struct
>> si_screen *sscreen,
>>                                           void *tgsi_binary,
>>                                           struct si_shader *shader)
>>   {
>> -       void *hw_binary = si_get_shader_binary(shader);
>> +       void *hw_binary;
>> +       struct hash_entry *entry;
>>
>> +       entry = _mesa_hash_table_search(sscreen->shader_cache,
>> tgsi_binary);
>> +       if (entry)
>> +               return false; /* already added */
>> +
>> +       hw_binary = si_get_shader_binary(shader);
>>         if (!hw_binary)
>>                 return false;
>>
>> @@ -1091,7 +1097,10 @@ void si_init_shader_selector_async(void *job, int
>> thread_index)
>>                 if (tgsi_binary &&
>>                     si_shader_cache_load_shader(sscreen, tgsi_binary,
>> shader)) {
>>                         FREE(tgsi_binary);
>> +                       pipe_mutex_unlock(sscreen->shader_cache_mutex);
>>                 } else {
>> +                       pipe_mutex_unlock(sscreen->shader_cache_mutex);
>> +
>>                         /* Compile the shader if it hasn't been loaded
>> from the cache. */
>>                         if (si_compile_tgsi_shader(sscreen, tm, shader,
>> false,
>>                                                    debug) != 0) {
>
>
> It's hard to tell in the context of the diffs, but isn't there a left-over
> pipe_mutex_unlock in the error path here?

Yes, you're right. Thanks. I'll remove that unlock call before pushing.

Marek