[Mesa-dev] [PATCH] radv: initialise slab_list

[Timothy Arceri]

On 08/09/17 17:59, Bas Nieuwenhuizen wrote:
> On Fri, Sep 8, 2017 at 9:27 AM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>> It seems by luck master works fine but with an alternate
>> compilation flow this is ends up causing a crash in
>> radv_shader_variant_destroy().
>> ---
>>   src/amd/vulkan/radv_pipeline.c       | 2 ++
>>   src/amd/vulkan/radv_pipeline_cache.c | 1 +
>>   2 files changed, 3 insertions(+)
>>
>> diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
>> index c090b8e2f0..3d9831236f 100644
>> --- a/src/amd/vulkan/radv_pipeline.c
>> +++ b/src/amd/vulkan/radv_pipeline.c
>> @@ -494,20 +494,22 @@ static struct radv_shader_variant *radv_shader_variant_create(struct radv_device
>>                  *code_out = binary.code;
>>                  *code_size_out = binary.code_size;
>>          } else
>>                  free(binary.code);
>>          free(binary.config);
>>          free(binary.rodata);
>>          free(binary.global_symbol_offsets);
>>          free(binary.relocs);
>>          free(binary.disasm_string);
>>          variant->ref_count = 1;
>> +       list_inithead(&variant->slab_list);
> 
> This resets it after the list is added to a slab in
> radv_fill_shader_variant -> radv_alloc_shader_memory ? How does that
> not crash and why does it crash previously?

Yeah it does crash. I originally had this in radv_fill_shader_variant() 
moving it back there fixed things again.

>> +
>>          return variant;
>>   }
>>
>>   static struct radv_shader_variant *
>>   radv_pipeline_create_gs_copy_shader(struct radv_pipeline *pipeline,
>>                                      struct nir_shader *nir,
>>                                      void** code_out,
>>                                      unsigned *code_size_out,
>>                                      bool dump_shader,
>>                                      bool multiview)
>> diff --git a/src/amd/vulkan/radv_pipeline_cache.c b/src/amd/vulkan/radv_pipeline_cache.c
>> index ef1f513f36..39b4e24e70 100644
>> --- a/src/amd/vulkan/radv_pipeline_cache.c
>> +++ b/src/amd/vulkan/radv_pipeline_cache.c
>> @@ -172,20 +172,21 @@ radv_create_shader_variant_from_pipeline_cache(struct radv_device *device,
>>                  if (!variant)
>>                          return NULL;
>>
>>                  variant->code_size = entry->code_size;
>>                  variant->config = entry->config;
>>                  variant->info = entry->variant_info;
>>                  variant->rsrc1 = entry->rsrc1;
>>                  variant->rsrc2 = entry->rsrc2;
>>                  variant->code_size = entry->code_size;
>>                  variant->ref_count = 1;
>> +               list_inithead(&variant->slab_list);
> 
> It should not be needed here. radv_alloc_shader_memory does a list_add
> or list_addtail, which don't need it initialized.

Hmm .. your right, please ignore. I need to update the patch I'm using 
to make use of radv_alloc_shader_memory() in the equivalent function.


>>
>>                  void *ptr = radv_alloc_shader_memory(device, variant);
>>                  memcpy(ptr, entry->code, entry->code_size);
>>
>>                  entry->variant = variant;
>>          }
>>
>>          p_atomic_inc(&entry->variant->ref_count);
>>          return entry->variant;
>>   }
>> --
>> 2.13.5
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev