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>