[Mesa-dev] [PATCH v2 5/7] disk cache: initialize cache path and index only when used

[Tapani Pälli]

On 02/09/2018 06:03 AM, Timothy Arceri wrote:
> On 09/02/18 14:45, Timothy Arceri wrote:
>> On 08/02/18 16:36, Tapani Pälli wrote:
>>> Hi;
>>>
>>> On 02/08/2018 06:00 AM, Timothy Arceri wrote:
>>>> Hi Tapani,
>>>>
>>>> This patch causes deadlock when running piglit on my radeonsi with 
>>>> my ryzen (16 threads all running shader_runner). Have you tested it 
>>>> on similar Intel hardware?
>>>
>>> It passes Intel CI, not sure if that counts though. Can you send some 
>>> more debug information?
>>
>> Looking closer at this change I think it needs to be reverted. It's 
>> not thread safe at all, applications are allowed to compile multiple 
>> programs attached to the same context in different threads which means 
>> we can end up with multiple threads inside disk_cache_path_init() at 
>> the same time causing all sorts of problems. glthread, radeonsi 
>> threaded compile, etc have the potential to hit this race condition 
>> even if the app doesn't do threaded compiles.
>>
>> I'm still not exactly sure why we must avoid creating the path in 
>> order to use callbacks. Can you explain the issue you are trying to 
>> solve in greater detail?

Reason is that we do not want to create such path on Android where cache 
is managed outside the driver (by callbacks provided by Android) and 
very likely we don't have permissions to create such path in a sane 
location. So we should be able to create disk_cache instance even if 
path creation fails.

> I've sent a series to revert things for now.
> 
> https://patchwork.freedesktop.org/series/37972/
> 

I just sent a fix proposal that moves path creation back to the 
constructor. With this nothing should change for desktop Linux. Please 
consider this instead of reverting patches.

Thanks;

// Tapani