Re: [Qemu-devel] [Qemu-block] [PATCH] qcow2: do lazy allocation of the L2 cache

[Alberto Garcia]

On Fri 08 May 2015 11:47:00 AM CEST, Kevin Wolf wrote:

>> There's also the problem that with a single chunk of memory for all
>> cache tables it's not so easy to free individual entries.
>
> That's one of the reasons why I suggested to fix the problem by using
> mmap() for the individual entries rather than allocating one big
> chunk.

Yeah but how portable mmap() is? Does it work in Windows?

>> One possible solution would be madvise() with MADV_DONTNEED, and
>> that's rather simple to use, but I'm unsure about its portability.
>
> Probably not portable at all, but the worst that can happen is that a
> cache doesn't shrink, i.e. behaves like today. Seems good enough.

We actually have qemu_madvise() and QEMU_MADV_DONTNEED, so if the system
does not support the operation then it's just a no-op. However that
means that if we expose some sort of API to configure the cache
expiration time, it would only work on some platforms.

>> > The other option would be to let the management tool change the cache
>> > size options at runtime (and as it happens, my current bdrv_reopen()
>> > overhaul allows just that - except that a QMP interface isn't planned
>> > yet), but I'm not sure if it has enough information to make good
>> > decisions. If we know what the criteria are, they could possibly be
>> > exposed, though.
>> 
>> But what would you do there? Recreate the whole cache?
>
> Recreating the cache is the easiest solution (and it is what my reopen
> patches do) and the overhead is probably neglegible as long as you
> don't resize the cache all the time.
>
> If we really need it, resizing the cache while maintaining its content
> would be possible, too.

I agree that the overhead of recreating the cache is probably negligible
so there's no need to make it more complex.

But as you say I don't think it's easy for the management tool to make
good decisions. If we have to expose the relevant information so it can
be done I think we would end up with an unnecessarily complex solution.

A simple way to set the expiration time of cache entries seems like a
much easier solution, both from the implementation and the user point of
views.

Berto