[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 2/3] qcow2: add option to clean unused cache ent
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time |
Date: |
Wed, 27 May 2015 06:27:35 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 |
On 05/27/2015 03:46 AM, Alberto Garcia wrote:
> This adds a new 'cache-clean-interval' option that cleans all qcow2
> cache entries that haven't been used in a certain interval, given in
> seconds.
>
> This allows setting a large L2 cache size so it can handle scenarios
> with lots of I/O and at the same time use little memory during periods
> of inactivity.
>
> This feature currently relies on MADV_DONTNEED to free that memory, so
> it is not useful in systems that don't follow that behavior.
>
> Signed-off-by: Alberto Garcia <address@hidden>
> Cc: Max Reitz <address@hidden>
> ---
> block/qcow2-cache.c | 35 ++++++++++++++++++++++++++++
> block/qcow2.c | 66
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> block/qcow2.h | 4 ++++
> qapi/block-core.json | 13 +++++++++--
> 4 files changed, 116 insertions(+), 2 deletions(-)
>
> +static void cache_clean_timer_init(BlockDriverState *bs, AioContext *context)
> +{
> + BDRVQcowState *s = bs->opaque;
> + if (s->cache_clean_interval > 0) {
> + s->cache_clean_timer = aio_timer_new(context, QEMU_CLOCK_VIRTUAL,
> + SCALE_MS, cache_clean_timer_cb,
> + bs);
> + timer_mod(s->cache_clean_timer,
> qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
> + (int64_t) s->cache_clean_interval * 1000);
> + }
> +}
> +
This function sets up a timer for non-zero interval, but does nothing if
interval is zero. [1]
> @@ -839,6 +888,16 @@ static int qcow2_open(BlockDriverState *bs, QDict
> *options, int flags,
> goto fail;
> }
>
> + cache_clean_interval =
> + qemu_opt_get_number(opts, QCOW2_OPT_CACHE_CLEAN_INTERVAL, 0);
> + if (cache_clean_interval > UINT_MAX) {
> + error_setg(errp, "Cache clean interval too big");
> + ret = -EINVAL;
> + goto fail;
> + }
If you type the qapi code as 'uint32' rather than 'int', you could skip
the error checking here because the parser would have already clamped
things. But I can live with this as-is.
> + s->cache_clean_interval = cache_clean_interval;
> + cache_clean_timer_init(bs, bdrv_get_aio_context(bs));
[1] But here, you are unconditionally calling init, whether the new
value is 0 or nonzero. Can a block reopen ever cause an existing BDS to
change its interval, in which case I could create a BDS originally with
a timer, then reopen it without a timer, and init() would have to remove
the existing timer? If I'm reading this patch correctly, right now the
interval is a write-once deal (no way to change it after the fact), so
your code is okay; but should a separate patch be added to allow
adjusting the interval, via a reopen operation?
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v3 0/3] Clean unused entries in the qcow2 L2/refcount cache, Alberto Garcia, 2015/05/27
- [Qemu-block] [PATCH 1/3] qcow2: mark the memory as no longer needed after qcow2_cache_empty(), Alberto Garcia, 2015/05/27
- [Qemu-block] [PATCH 3/3] qcow2: reorder fields in Qcow2CachedTable to reduce padding, Alberto Garcia, 2015/05/27
- [Qemu-block] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time, Alberto Garcia, 2015/05/27
- Re: [Qemu-block] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time, Eric Blake, 2015/05/28
- Re: [Qemu-block] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time, Max Reitz, 2015/05/28
- Re: [Qemu-block] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time, Alberto Garcia, 2015/05/28
- Re: [Qemu-block] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time, Max Reitz, 2015/05/28