From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:46015) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ubt9G-0000W2-J5 for qemu-devel@nongnu.org; Mon, 13 May 2013 09:50:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ubt9D-0002AD-Nd for qemu-devel@nongnu.org; Mon, 13 May 2013 09:50:42 -0400 Received: from e23smtp04.au.ibm.com ([202.81.31.146]:53296) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ubt9D-00026F-6J for qemu-devel@nongnu.org; Mon, 13 May 2013 09:50:39 -0400 Received: from /spool/local by e23smtp04.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 13 May 2013 23:37:42 +1000 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [9.190.235.152]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id E84E73578019 for ; Mon, 13 May 2013 23:50:20 +1000 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r4DDaDRQ23396398 for ; Mon, 13 May 2013 23:36:14 +1000 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r4DDoJpb025509 for ; Mon, 13 May 2013 23:50:19 +1000 From: Anthony Liguori In-Reply-To: <1365522223-20153-1-git-send-email-pbonzini@redhat.com> References: <1365522223-20153-1-git-send-email-pbonzini@redhat.com> Date: Mon, 13 May 2013 08:50:09 -0500 Message-ID: <877gj3q99q.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH v3] migration: initialize RAM to zero List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: kwolf@redhat.com, owasserm@redhat.com, pl@kamp.de, stefanha@redhat.com, quintela@redhat.com Paolo Bonzini writes: > Using qemu_memalign only leaves the RAM zero by chance, because libc > will usually use mmap to satisfy our huge requests. But memory will > not be zero when using MALLOC_PERTURB_ with a nonzero value. In the > case of incoming migration, this breaks a recently-introduced > invariant (commit f1c7279, migration: do not sent zero pages in > bulk stage, 2013-03-26). > > To fix this, use mmap ourselves to get a well-aligned, always zero > block for the RAM. Mmap-ed memory is easy to "trim" at the sides. > > This also removes the need to do something special on valgrind > (see commit c2a8238a, Support running QEMU on Valgrind, 2011-10-31), > thus effectively reverts that patch. > > Reviewed-by: Juan Quintela > Signed-off-by: Paolo Bonzini > --- > v2->v3: use MAP_FAILED. You learn something every day. > > util/oslib-posix.c | 35 ++++++++++++++++++----------------- > 1 file changed, 18 insertions(+), 17 deletions(-) > > diff --git a/util/oslib-posix.c b/util/oslib-posix.c > index 4e4b819..bda62c0 100644 > --- a/util/oslib-posix.c > +++ b/util/oslib-posix.c > @@ -40,7 +40,6 @@ extern int daemon(int, int); > Valgrind does not support alignments larger than 1 MiB, > therefore we need special code which handles running on Valgrind. */ > # define QEMU_VMALLOC_ALIGN (512 * 4096) > -# define CONFIG_VALGRIND > #elif defined(__linux__) && defined(__s390x__) > /* Use 1 MiB (segment size) alignment so gmap can be used by KVM. */ > # define QEMU_VMALLOC_ALIGN (256 * 4096) > @@ -52,12 +51,8 @@ extern int daemon(int, int); > #include "sysemu/sysemu.h" > #include "trace.h" > #include "qemu/sockets.h" > +#include > > -#if defined(CONFIG_VALGRIND) > -static int running_on_valgrind = -1; > -#else > -# define running_on_valgrind 0 > -#endif > #ifdef CONFIG_LINUX > #include > #endif > @@ -108,22 +103,28 @@ void *qemu_memalign(size_t alignment, size_t size) > /* alloc shared memory pages */ > void *qemu_vmalloc(size_t size) > { > - void *ptr; > size_t align = QEMU_VMALLOC_ALIGN; > + size_t total = size + align - getpagesize(); > + void *ptr = mmap(0, total, PROT_READ | PROT_WRITE, > + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); > + size_t offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr; > > -#if defined(CONFIG_VALGRIND) > - if (running_on_valgrind < 0) { > - /* First call, test whether we are running on Valgrind. > - This is a substitute for RUNNING_ON_VALGRIND from valgrind.h. */ > - const char *ld = getenv("LD_PRELOAD"); > - running_on_valgrind = (ld != NULL && strstr(ld, "vgpreload")); > + if (ptr == MAP_FAILED) { > + fprintf(stderr, "Failed to allocate %zu B: %s\n", > + size, strerror(errno)); > + abort(); > } > -#endif > > - if (size < align || running_on_valgrind) { > - align = getpagesize(); > + ptr += offset; > + total -= offset; > + > + if (offset > 0) { > + munmap(ptr - offset, offset); > } > - ptr = qemu_memalign(align, size); Hrm, so we switch from qemu_memalign to mmap() but then we don't modify qemu_vfree() to do a munmap() over free(). qemu_vfree() doesn't know the size so calling munmap() is tricky. Regards, Anthony Liguori > + if (total > size) { > + munmap(ptr + size, total - size); > + } > + > trace_qemu_vmalloc(size, ptr); > return ptr; > } > -- > 1.8.1.4