| Message ID | 1368454796-14989-3-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 13 May 2013 15:19, Paolo Bonzini <pbonzini@redhat.com> wrote: > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -98,6 +98,7 @@ int qemu_daemon(int nochdir, int noclose); > void *qemu_memalign(size_t alignment, size_t size); > void *qemu_anon_ram_alloc(size_t size); > void qemu_vfree(void *ptr); > +void qemu_anon_ram_free(void *ptr, size_t size); No documentation comment? thanks -- PMM
> On 13 May 2013 15:19, Paolo Bonzini <pbonzini@redhat.com> wrote: > > --- a/include/qemu/osdep.h > > +++ b/include/qemu/osdep.h > > @@ -98,6 +98,7 @@ int qemu_daemon(int nochdir, int noclose); > > void *qemu_memalign(size_t alignment, size_t size); > > void *qemu_anon_ram_alloc(size_t size); > > void qemu_vfree(void *ptr); > > +void qemu_anon_ram_free(void *ptr, size_t size); > > No documentation comment? None in the proximity, either, and it's (a) pretty obvious that it matches qemu_anon_ram_alloc (b) not really commonly used, so it won't help a lot of readers. Paolo
This patch would break the build for me, if I didn't configure --disable-werror. Using --enable-trace-backend=stderr in case it makes a difference. Troublemaker flagged inline. Paolo Bonzini <pbonzini@redhat.com> writes: > We switched from qemu_memalign to mmap() but then we don't modify > qemu_vfree() to do a munmap() over free(). Which we cannot do > because qemu_vfree() frees memory allocated by qemu_{mem,block}align. > > Introduce a new function that does the munmap(), luckily the size is > available in the RAMBlock. [...] > diff --git a/trace-events b/trace-events > index acf0f5c..c7dbcf0 100644 > --- a/trace-events > +++ b/trace-events > @@ -34,6 +34,7 @@ g_free(void *ptr) "ptr %p" > qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size %zu ptr %p" > qemu_anon_ram_alloc(size_t size, void *ptr) "size %zu ptr %p" > qemu_vfree(void *ptr) "ptr %p" > +qemu_anon_ram_free(void *ptr, size_t size) "size %zu ptr %p" In file included from /work/armbru/qemu/include/trace.h:4:0, from /work/armbru/qemu/util/osdep.c:49: ./trace/generated-tracers.h: In function 'trace_qemu_anon_ram_free': ./trace/generated-tracers.h:64:9: warning: format '%zu' expects argument of type 'size_t', but argument 3 has type 'void *' [-Wformat] ./trace/generated-tracers.h:64:9: warning: format '%p' expects argument of type 'void *', but argument 4 has type 'size_t' [-Wformat] > > # hw/virtio.c > virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int idx) "vq %p elem %p len %u idx %u" [...]
Il 15/05/2013 16:36, Markus Armbruster ha scritto: > This patch would break the build for me, if I didn't configure > --disable-werror. Using --enable-trace-backend=stderr in case it makes > a difference. Troublemaker flagged inline. Yes, it does---and it makes it a bit less urgent. Will fix tomorrow. Paolo
Am 15.05.2013 16:44, schrieb Paolo Bonzini: > Il 15/05/2013 16:36, Markus Armbruster ha scritto: >> This patch would break the build for me, if I didn't configure >> --disable-werror. Using --enable-trace-backend=stderr in case it makes >> a difference. Troublemaker flagged inline. > Yes, it does---and it makes it a bit less urgent. Will fix tomorrow. > > Paolo I just got this error with the latest QEMU release candidate and have sent a patch to qemu-devel now. Cheers, Stefan
diff --git a/exec.c b/exec.c index b7e7494..7697be7 100644 --- a/exec.c +++ b/exec.c @@ -1156,21 +1156,17 @@ void qemu_ram_free(ram_addr_t addr) munmap(block->host, block->length); close(block->fd); } else { - qemu_vfree(block->host); + qemu_anon_ram_free(block->host, block->length); } #else abort(); #endif } else { -#if defined(TARGET_S390X) && defined(CONFIG_KVM) - munmap(block->host, block->length); -#else if (xen_enabled()) { xen_invalidate_map_cache_entry(block->host); } else { - qemu_vfree(block->host); + qemu_anon_ram_free(block->host, block->length); } -#endif } g_free(block); break; diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index ae71042..d8e8500 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -98,6 +98,7 @@ int qemu_daemon(int nochdir, int noclose); void *qemu_memalign(size_t alignment, size_t size); void *qemu_anon_ram_alloc(size_t size); void qemu_vfree(void *ptr); +void qemu_anon_ram_free(void *ptr, size_t size); #define QEMU_MADV_INVALID -1 diff --git a/trace-events b/trace-events index acf0f5c..c7dbcf0 100644 --- a/trace-events +++ b/trace-events @@ -34,6 +34,7 @@ g_free(void *ptr) "ptr %p" qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size %zu ptr %p" qemu_anon_ram_alloc(size_t size, void *ptr) "size %zu ptr %p" qemu_vfree(void *ptr) "ptr %p" +qemu_anon_ram_free(void *ptr, size_t size) "size %zu ptr %p" # hw/virtio.c virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int idx) "vq %p elem %p len %u idx %u" diff --git a/util/oslib-posix.c b/util/oslib-posix.c index e7e9c01..087edc9 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -135,6 +135,14 @@ void qemu_vfree(void *ptr) free(ptr); } +void qemu_anon_ram_free(void *ptr, size_t size) +{ + trace_qemu_anon_ram_free(ptr, size); + if (ptr) { + munmap(ptr, size); + } +} + void qemu_set_block(int fd) { int f; diff --git a/util/oslib-win32.c b/util/oslib-win32.c index e83c22b..adcf514 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -76,6 +76,14 @@ void qemu_vfree(void *ptr) } } +void qemu_anon_ram_free(void *ptr, size_t size) +{ + trace_qemu_anon_ram_free(ptr, size); + if (ptr) { + VirtualFree(ptr, 0, MEM_RELEASE); + } +} + /* FIXME: add proper locking */ struct tm *gmtime_r(const time_t *timep, struct tm *result) {
We switched from qemu_memalign to mmap() but then we don't modify qemu_vfree() to do a munmap() over free(). Which we cannot do because qemu_vfree() frees memory allocated by qemu_{mem,block}align. Introduce a new function that does the munmap(), luckily the size is available in the RAMBlock. Reported-by: Amos Kong <akong@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- exec.c | 8 ++------ include/qemu/osdep.h | 1 + trace-events | 1 + util/oslib-posix.c | 8 ++++++++ util/oslib-win32.c | 8 ++++++++ 5 files changed, 20 insertions(+), 6 deletions(-)