| Message ID | 11e5272e78e9c9485b47c41feedd6a30dbf0cb8e.1431967209.git.berto@igalia.com |
|---|---|
| State | New |
| Headers | show |
On 18.05.2015 18:48, Alberto Garcia wrote: > After having emptied the cache, the data in the cache tables is no > longer useful, so we can tell the kernel that we are done with it. In > Linux this frees the resources associated with it. > > The effect of this can be seen in the block-commit operation: it moves > data from the top to the base image (and fills both caches), then it > empties the top image. At this point the data in that cache is no > longer needed so it's just wasting memory. > > Signed-off-by: Alberto Garcia <berto@igalia.com> > --- > block/qcow2-cache.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) Looks good, but by applying the same logic, you could do the same call in qcow2_cache_create(). So what about it? :-) Also note that bdrv_commit() is used only by the HMP commit operation, not by QMP commit. Max
On Tue 26 May 2015 05:39:12 PM CEST, Max Reitz <mreitz@redhat.com> wrote: >> After having emptied the cache, the data in the cache tables is no >> longer useful, so we can tell the kernel that we are done with it. In >> Linux this frees the resources associated with it. > Looks good, but by applying the same logic, you could do the same call > in qcow2_cache_create(). So what about it? :-) But after qcow2_cache_create() the memory is still unused so this doesn't have any affect. > Also note that bdrv_commit() is used only by the HMP commit operation, > not by QMP commit. Thanks, I should probably clarify that in the commit message. Berto
On 26.05.2015 17:51, Alberto Garcia wrote: > On Tue 26 May 2015 05:39:12 PM CEST, Max Reitz <mreitz@redhat.com> wrote: > >>> After having emptied the cache, the data in the cache tables is no >>> longer useful, so we can tell the kernel that we are done with it. In >>> Linux this frees the resources associated with it. >> Looks good, but by applying the same logic, you could do the same call >> in qcow2_cache_create(). So what about it? :-) > But after qcow2_cache_create() the memory is still unused so this > doesn't have any affect. That was too easy. Right. Reviewed-by: Max Reitz <mreitz@redhat.com> >> Also note that bdrv_commit() is used only by the HMP commit operation, >> not by QMP commit. > Thanks, I should probably clarify that in the commit message. > > Berto
diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c index ed92a09..ed14a92 100644 --- a/block/qcow2-cache.c +++ b/block/qcow2-cache.c @@ -22,8 +22,10 @@ * THE SOFTWARE. */ +#include <sys/mman.h> #include "block/block_int.h" #include "qemu-common.h" +#include "qemu/osdep.h" #include "qcow2.h" #include "trace.h" @@ -60,6 +62,22 @@ static inline int qcow2_cache_get_table_idx(BlockDriverState *bs, return idx; } +static void qcow2_cache_table_release(BlockDriverState *bs, Qcow2Cache *c, + int i, int num_tables) +{ +#if QEMU_MADV_DONTNEED != QEMU_MADV_INVALID + BDRVQcowState *s = bs->opaque; + void *t = qcow2_cache_get_table_addr(bs, c, i); + long align = sysconf(_SC_PAGESIZE); + size_t mem_size = (size_t) s->cluster_size * num_tables; + 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); + } +#endif +} + Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables) { BDRVQcowState *s = bs->opaque; @@ -237,6 +255,8 @@ int qcow2_cache_empty(BlockDriverState *bs, Qcow2Cache *c) c->entries[i].lru_counter = 0; } + qcow2_cache_table_release(bs, c, 0, c->size); + c->lru_counter = 0; return 0;
After having emptied the cache, the data in the cache tables is no longer useful, so we can tell the kernel that we are done with it. In Linux this frees the resources associated with it. The effect of this can be seen in the block-commit operation: it moves data from the top to the base image (and fills both caches), then it empties the top image. At this point the data in that cache is no longer needed so it's just wasting memory. Signed-off-by: Alberto Garcia <berto@igalia.com> --- block/qcow2-cache.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)