[Mesa-dev] [PATCH] glsl: fix disk cache eviction issue

[Tapani Pälli]

On 01/09/2017 05:42 PM, Emil Velikov wrote:
> On 9 January 2017 at 15:36, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 9 December 2016 at 05:58, Tapani Pälli <tapani.palli at intel.com> wrote:
>>> When 'evict_random_item' attempts to remove cache content to make more
>>> space, it may try to remove from new cache directory it just created
>>> which only has '.tmp' content at this phase and not proper cache files,
>>> this will cause eviction to fail and 'test_put_and_get' subtest
>>> assumptions about cache size and item count won't hold.
>>>
>>> Patch fixes this issue by adding 'exception' argument for random file
>>> matching so that algorithm won't pick the new directory.
>>>
>> Hi Tapani,
>>
>> I've mentioned this over at #android-ia, but adding it here for posterity.
>>
>>
>> The issue itself is actually quite different - in my experiments with
>> importing SHA1 implementation to mesa, I've noticed that there bug
>> lies with a buggy (and/or undefined behaviour) SHA1 implementation
>> given the input data.
>>
>> Namely:
>> As one observes the output of _mesa_sha1_compute they will notice that
>> it differs across implementations. This in itself combined with the
>> (afaict) correct assumption in test_put_and_get() that the beginning
>> for the SHA1blob_key_byte_zero
>>
> [Pressed enter to quickly]
>
> ...the SHA1 matches blob_key_byte_zero (as mentioned in the comment of
> said function) makes things work ... sometimes.

OK looks like this is going to be pretty tricky to debug. Independent of 
this test issue however I still believe my patch is required/correct. 
There is nothing currently in cache eviction code that would prevent it 
from picking the directory created for new entry. If it picks it, it 
does not delete anything away (since no cache files inside) and 
therefore does not respect the maximum size limit set for the cache.

> So in a Tl;Dr; we want to drop the ~5 different codepaths, in favour
> of a single one, used by everyone - devs, testers, and users.
>

Right, I agree this would be very nice. I will still check what 
supporting boringssl would mean, at least from quick glance it has quite 
different API from others (which is also unstable ..) which did not 
encourage me very much last time.

// Tapani