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

[Tapani Pälli]

On 31.01.2018 15:18, Emil Velikov wrote:
> On 31 January 2018 at 07:17, Tapani Pälli <tapani.palli at intel.com> wrote:
>> This patch makes disk_cache initialize path and index lazily so
>> that we can utilize disk_cache without a path using callback
>> functionality introduced by next patch.
>>
>> v2: unmap mmap and destroy queue only if index_mmap exists
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/util/disk_cache.c | 127 +++++++++++++++++++++++++++++++-------------------
>>   1 file changed, 78 insertions(+), 49 deletions(-)
>>
> I'd keep the refactor (disk_cache_create -> disk_cache_path_init +
> disk_cache_create) and lazy indexing separate patches.
> As-is tracking all the error paths is a quite fiddly.
> 
>> @@ -999,6 +1015,11 @@ disk_cache_put(struct disk_cache *cache, const cache_key key,
>>      struct disk_cache_put_job *dc_job =
>>         create_put_job(cache, key, data, size, cache_item_metadata);
>>
>> +   /* Initialize path if not initialized yet. */
>> +   if (cache->path_init_failed ||
>> +       (!cache->path && !disk_cache_path_init(cache)))
>> +      return;
>> +
>>      if (dc_job) {
>>         util_queue_fence_init(&dc_job->fence);
>>         util_queue_add_job(&cache->cache_queue, dc_job, &dc_job->fence,
>> @@ -1173,6 +1194,9 @@ disk_cache_put_key(struct disk_cache *cache, const cache_key key)
>>      int i = CPU_TO_LE32(*key_chunk) & CACHE_INDEX_KEY_MASK;
>>      unsigned char *entry;
>>
>> +   if (!cache->path)
>> +      return;
>> +
> Any reason why the blurb in disk_cache_put() is missing here?

Reason why cache is created in disk_cache_has_key because that is called 
before disk_cache_put_key by the compiler.

>  From cache_test.c POV disk_cache_put_key relied on disk_cache_has_key
> being called first, although I'm not sure if that's the most robust
> approach.
> 

Unit test calls disk_cache_put directly that also tries to create the 
cache. I'm OK trying to create cache here also but this should not 
happen either with apps or unit tests.

// Tapani