From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57679) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vq692-0001Ma-UP for qemu-devel@nongnu.org; Mon, 09 Dec 2013 14:05:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vq68v-00061Z-2B for qemu-devel@nongnu.org; Mon, 09 Dec 2013 14:05:28 -0500 Received: from v220110690675601.yourvserver.net ([37.221.199.173]:56311) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vq68u-00061N-PS for qemu-devel@nongnu.org; Mon, 09 Dec 2013 14:05:20 -0500 Message-ID: <52A6146B.6070008@weilnetz.de> Date: Mon, 09 Dec 2013 20:05:15 +0100 From: Stefan Weil MIME-Version: 1.0 References: <1386345276-9803-1-git-send-email-pbonzini@redhat.com> <1386345276-9803-2-git-send-email-pbonzini@redhat.com> In-Reply-To: <1386345276-9803-2-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PULL 2/3] qemu: mempath: prefault pages manually (v4) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: Marcelo Tosatti Hi, this patch was recently committed to git master. It now causes problems with gcc-4.7 -Werror=3Dclobbered. See more comment= s below. Am 06.12.2013 16:54, schrieb Paolo Bonzini: > From: Marcelo Tosatti > > v4: s/fail/failed/ (Peter Maydell) > > Signed-off-by: Paolo Bonzini > Signed-off-by: Marcelo Tosatti > --- > exec.c | 59 +++++++++++++++++++++++++++++++++++++++++++----= ------- > qemu-options.hx | 2 - > vl.c | 4 --- > 3 files changed, 47 insertions(+), 18 deletions(-) > > diff --git a/exec.c b/exec.c > index 95c4356..f4b9ef2 100644 > --- a/exec.c > +++ b/exec.c > @@ -904,6 +904,13 @@ static long gethugepagesize(const char *path) > return fs.f_bsize; > } > =20 > +static sigjmp_buf sigjump; > + > +static void sigbus_handler(int signal) > +{ > + siglongjmp(sigjump, 1); > +} > + > static void *file_ram_alloc(RAMBlock *block, > ram_addr_t memory, > const char *path) > @@ -913,9 +920,6 @@ static void *file_ram_alloc(RAMBlock *block, > char *c; > void *area; > int fd; > -#ifdef MAP_POPULATE > - int flags; > -#endif > unsigned long hpagesize; > =20 > hpagesize =3D gethugepagesize(path); > @@ -963,21 +967,52 @@ static void *file_ram_alloc(RAMBlock *block, > if (ftruncate(fd, memory)) > perror("ftruncate"); > =20 > -#ifdef MAP_POPULATE > - /* NB: MAP_POPULATE won't exhaustively alloc all phys pages in the= case > - * MAP_PRIVATE is requested. For mem_prealloc we mmap as MAP_SHAR= ED > - * to sidestep this quirk. > - */ > - flags =3D mem_prealloc ? MAP_POPULATE | MAP_SHARED : MAP_PRIVATE; > - area =3D mmap(0, memory, PROT_READ | PROT_WRITE, flags, fd, 0); > -#else > area =3D mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, = 0); > -#endif > if (area =3D=3D MAP_FAILED) { > perror("file_ram_alloc: can't mmap RAM pages"); > close(fd); > return (NULL); > } > + > + if (mem_prealloc) { > + int ret, i; > + struct sigaction act, oldact; > + sigset_t set, oldset; > + > + memset(&act, 0, sizeof(act)); > + act.sa_handler =3D &sigbus_handler; > + act.sa_flags =3D 0; > + > + ret =3D sigaction(SIGBUS, &act, &oldact); > + if (ret) { > + perror("file_ram_alloc: failed to install signal handler")= ; > + exit(1); > + } > + > + /* unblock SIGBUS */ > + sigemptyset(&set); > + sigaddset(&set, SIGBUS); > + pthread_sigmask(SIG_UNBLOCK, &set, &oldset); > + The sigsetjmp instruction causes the compiler to assume that 'area' might be clobbered (sigsetjmp is declared to return twice, and gcc does not now anything about its semantics). > + if (sigsetjmp(sigjump, 1)) { > + fprintf(stderr, "file_ram_alloc: failed to preallocate pag= es\n"); > + exit(1); > + } > + > + /* MAP_POPULATE silently ignores failures */ Yes, but why this comment in this context? Here we don't use MAP_POPULATE and can get SIGBUS (which is not silent). Or am I wrong? > + for (i =3D 0; i < (memory/hpagesize)-1; i++) { > + memset(area + (hpagesize*i), 0, 1); > + } > + > + ret =3D sigaction(SIGBUS, &oldact, NULL); > + if (ret) { > + perror("file_ram_alloc: failed to reinstall signal handler= "); > + exit(1); > + } > + > + pthread_sigmask(SIG_SETMASK, &oldset, NULL); > + } > + > block->fd =3D fd; > return area; > } This is the warning shown by newer gcc versions: qemu/exec.c:921:11: error: variable =E2=80=98area=E2=80=99 might be clobbered by =E2=80=98longjmp=E2= =80=99 or =E2=80=98vfork=E2=80=99 [-Werror=3Dclobbered] I want to get support for -Wextra, and -Wclobbered is part of -Wextra. Of course we could disable -Wclobbered and still use the other advantages of -Wextra, but this might hide real problems with clobbered variables in the future. Declaring the local variable 'area' to be volatile also fixes the warning= . Any opinion how this problem should be solved? Thanks, Stefan