From 2f2c8d6b371cfc6689affb0b7e463fa2160c9e5b Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Fri, 25 Nov 2016 13:27:43 +0200 Subject: qcow2: Make qcow2_cache_table_release() work only in Linux We are using QEMU_MADV_DONTNEED to discard the memory of individual L2 cache tables. The problem with this is that those semantics are specific to the Linux madvise() system call. Other implementations of madvise() (including the very Linux implementation of posix_madvise()) don't do that, so we cannot use them for the same purpose. This patch makes the code Linux-specific and uses madvise() directly since there's no point in going through qemu_madvise() for this. Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block/qcow2-cache.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c index 6eaefeddc4..ab8ee2d330 100644 --- a/block/qcow2-cache.c +++ b/block/qcow2-cache.c @@ -66,7 +66,8 @@ static inline int qcow2_cache_get_table_idx(BlockDriverState *bs, static void qcow2_cache_table_release(BlockDriverState *bs, Qcow2Cache *c, int i, int num_tables) { -#if QEMU_MADV_DONTNEED != QEMU_MADV_INVALID +/* Using MADV_DONTNEED to discard memory is a Linux-specific feature */ +#ifdef CONFIG_LINUX BDRVQcow2State *s = bs->opaque; void *t = qcow2_cache_get_table_addr(bs, c, i); int align = getpagesize(); @@ -74,7 +75,7 @@ static void qcow2_cache_table_release(BlockDriverState *bs, Qcow2Cache *c, size_t offset = QEMU_ALIGN_UP((uintptr_t) t, align) - (uintptr_t) t; size_t length = QEMU_ALIGN_DOWN(mem_size - offset, align); if (length > 0) { - qemu_madvise((uint8_t *) t + offset, length, QEMU_MADV_DONTNEED); + madvise((uint8_t *) t + offset, length, MADV_DONTNEED); } #endif } -- cgit v1.2.3 From 91203f08f0ca66f1a6aba1d0e5ef62ed98fb3234 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Fri, 25 Nov 2016 13:27:44 +0200 Subject: qcow2: Allow 'cache-clean-interval' in Linux only The cache-clean-interval option of qcow2 only works on Linux. However we allow setting it in other systems regardless of whether it works or not. In those systems this option is not simply a no-op: it actually invalidates perfectly valid cache tables for no good reason without freeing their memory. This patch forbids using that option in non-Linux systems. Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block/qcow2.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index 7cfcd8412c..ed9e0f31d6 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -668,6 +668,14 @@ static int qcow2_update_options_prepare(BlockDriverState *bs, r->cache_clean_interval = qemu_opt_get_number(opts, QCOW2_OPT_CACHE_CLEAN_INTERVAL, s->cache_clean_interval); +#ifndef CONFIG_LINUX + if (r->cache_clean_interval != 0) { + error_setg(errp, QCOW2_OPT_CACHE_CLEAN_INTERVAL + " not supported on this host"); + ret = -EINVAL; + goto fail; + } +#endif if (r->cache_clean_interval > UINT_MAX) { error_setg(errp, "Cache clean interval too big"); ret = -EINVAL; -- cgit v1.2.3 From a8b99dd51649672cb08458a25a762687e8067715 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Fri, 25 Nov 2016 13:27:45 +0200 Subject: qcow2: Remove stale comment We haven't been using CONFIG_MADVISE since 02d0e095031b7fda77de8b Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block/qcow2-cache.c | 1 - 1 file changed, 1 deletion(-) diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c index ab8ee2d330..1d25147392 100644 --- a/block/qcow2-cache.c +++ b/block/qcow2-cache.c @@ -22,7 +22,6 @@ * THE SOFTWARE. */ -/* Needed for CONFIG_MADVISE */ #include "qemu/osdep.h" #include "block/block_int.h" #include "qemu-common.h" -- cgit v1.2.3 From 8f57758311d816c66e88cfcfdc91f986f59345ad Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Fri, 25 Nov 2016 13:27:46 +0200 Subject: docs: Specify that cache-clean-interval is only supported in Linux Make it clear that having Linux is a hard requirement for this feature. Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- docs/qcow2-cache.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt index 5bb06072d3..1fdd6f9ce7 100644 --- a/docs/qcow2-cache.txt +++ b/docs/qcow2-cache.txt @@ -160,5 +160,6 @@ If unset, the default value for this parameter is 0 and it disables this feature. Note that this functionality currently relies on the MADV_DONTNEED -argument for madvise() to actually free the memory, so it is not -useful in systems that don't follow that behavior. +argument for madvise() to actually free the memory. This is a +Linux-specific feature, so cache-clean-interval is not supported in +other systems. -- cgit v1.2.3