[Mesa-dev] [PATCH 3/4] util/disk_cache: have disk_cache_put() optionally free memory

[Timothy Arceri]

On 13/03/17 21:16, Grazvydas Ignotas wrote:
> On Mon, Mar 13, 2017 at 3:01 AM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>> The following patch will move disk_cache_put() into a thread queue
>> so we need to be sure memory is not freed before the thread is
>> completed. Here we move responsibility for releasing the memory
>> onto the disk cache.
>
> I think this is a fragile interface and very easy to mess up, cache
> should just make a copy instead. You already do that at one callsite
> anyway.
>
> You could even do that while creating struct disk_cache_put_job to
> avoid an extra malloc:
>
> struct disk_cache_put_job {
>    ...
>    size_t size;
>    uint8_t mem[0];
> };
> ...
> create_put_job(struct disk_cache *cache, const cache_key key,
>                const void *data, size_t size)
> {
>    struct disk_cache_put_job *dc_job = malloc(sizeof(*dc_job) + size);
>    ...
>    memcpy(dc_job->mem, data, size);
>    ...
> }
>
> That's of course just my opinion, I hope somebody (Marek?) joins in.

I'm not all that happy with the interface by IMO it's better than than 
doing an extra memcpy. These have the potential to be around 200KB+ and 
this can be happening at draw time.

IMO the slightly awkward interface is the better option, it's not like 
it's going to be a common function to implement, besides the common code 
(which is done) each driver should only have to call it once.

We are forced to do the copy for radeonsi, but those copies as far as I 
know should be rather small.

Anyway happy to hear other opinions on this.