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

[Emil Velikov]

On 31 January 2018 at 13:39, Tapani Pälli <tapani.palli at intel.com> wrote:
>
>
> 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.
>
Right, in that case I'd add an assert, so it flags up ASAP.

-Emil