From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=39265 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OvcFQ-0000Jv-0h for qemu-devel@nongnu.org; Tue, 14 Sep 2010 16:37:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OvcFM-00012C-4D for qemu-devel@nongnu.org; Tue, 14 Sep 2010 16:36:59 -0400 Received: from mail-qy0-f180.google.com ([209.85.216.180]:48670) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OvcFL-000127-Tb for qemu-devel@nongnu.org; Tue, 14 Sep 2010 16:36:56 -0400 Received: by qyk31 with SMTP id 31so6709002qyk.4 for ; Tue, 14 Sep 2010 13:36:55 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1284496107-3533-1-git-send-email-andreas.faerber@web.de> References: <1284496107-3533-1-git-send-email-andreas.faerber@web.de> From: Blue Swirl Date: Tue, 14 Sep 2010 20:36:28 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: [PATCH v5] Introduce qemu_madvise() List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Andreas_F=C3=A4rber?= Cc: qemu-devel@nongnu.org, Alexander Graf On Tue, Sep 14, 2010 at 8:28 PM, Andreas F=C3=A4rber wrote: > From: Andreas F=C3=A4rber > > vl.c has a Sun-specific hack to supply a prototype for madvise(), > but the call site has apparently moved to arch_init.c. > > Haiku doesn't implement madvise() in favor of posix_madvise(). > OpenBSD and Solaris 10 don't implement posix_madvise() but madvise(). > > Check for madvise() and posix_madvise() in configure and supply qemu_madv= ise() > as wrapper. Prefer madvise() over posix_madvise() due to flag availabilit= y. > Convert all callers to use qemu_madvise() and QEMU_MADV_*. > > Note that on Solaris the warning is fixed by moving the madvise() prototy= pe, > not by qemu_madvise() itself. It will help with future porting though, an= d > it simplifies most call sites. > > v4 -> v5: > * Introduce QEMU_MADV_INVALID, suggested by Alexander Graf. > =C2=A0Note that this relies on -1 not being a valid advice value. > > v3 -> v4: > * Eliminate #ifdefs at qemu_advise() call sites. Requested by Blue Swirl. > =C2=A0This will currently break the check in kvm-all.c by calling madvise= () with > =C2=A0a supported flag, which will not fail. Ideas/patches welcome. > > v2 -> v3: > * Reuse the *_MADV_* defines for QEMU_MADV_*. Suggested by Alexander Graf= . > * Add configure check for madvise(), too. > =C2=A0Add defines to Makefile, not QEMU_CFLAGS. > =C2=A0Convert all callers, untested. Suggested by Blue Swirl. > * Keep Solaris' madvise() prototype around. Pointed out by Alexander Graf= . > * Display configure check results. > > v1 -> v2: > * Don't rely on posix_madvise() availability, add qemu_madvise(). > =C2=A0Suggested by Blue Swirl. > > Signed-off-by: Andreas F=C3=A4rber > Cc: Blue Swirl > Cc: Alexander Graf > --- > =C2=A0arch_init.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A02 +- > =C2=A0configure =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 33 ++++++++++= +++++++++++++++++++++++ > =C2=A0exec.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2= =A08 ++------ > =C2=A0hw/virtio-balloon.c | =C2=A0 =C2=A04 ++-- > =C2=A0kvm-all.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 15 +++++++---= ----- > =C2=A0osdep.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 18 +++++= +++++++++++++ > =C2=A0osdep.h =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 35 +++++= ++++++++++++++++++++++++++++++ > =C2=A0vl.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2= =A0 =C2=A03 --- > =C2=A08 files changed, 98 insertions(+), 20 deletions(-) > > diff --git a/arch_init.c b/arch_init.c > index e468c0c..a910033 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -396,7 +396,7 @@ int ram_load(QEMUFile *f, void *opaque, int version_i= d) > =C2=A0#ifndef _WIN32 > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (ch =3D=3D 0 && > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (!kvm_enabled() |= | kvm_has_sync_mmu())) { > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0madvise(host, TA= RGET_PAGE_SIZE, MADV_DONTNEED); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_madvise(hos= t, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } > =C2=A0#endif > =C2=A0 =C2=A0 =C2=A0 =C2=A0 } else if (flags & RAM_SAVE_FLAG_PAGE) { > diff --git a/configure b/configure > index 4061cb7..86558eb 100755 > --- a/configure > +++ b/configure > @@ -2069,6 +2069,31 @@ if compile_prog "" "" ; then > =C2=A0fi > > =C2=A0########################################## > +# check if we have madvise > + > +madvise=3Dno > +cat > $TMPC << EOF > +#include > +#include > +int main(void) { return madvise(NULL, 0, MADV_DONTNEED); } > +EOF > +if compile_prog "" "" ; then > + =C2=A0 =C2=A0madvise=3Dyes > +fi > + > +########################################## > +# check if we have posix_madvise > + > +posix_madvise=3Dno > +cat > $TMPC << EOF > +#include > +int main(void) { return posix_madvise(NULL, 0, POSIX_MADV_DONTNEED); } > +EOF > +if compile_prog "" "" ; then > + =C2=A0 =C2=A0posix_madvise=3Dyes > +fi > + > +########################################## > =C2=A0# check if trace backend exists > > =C2=A0sh "$source_path/tracetool" "--$trace_backend" --check-backend > /d= ev/null 2> /dev/null > @@ -2226,6 +2251,8 @@ echo "KVM support =C2=A0 =C2=A0 =C2=A0 $kvm" > =C2=A0echo "fdt support =C2=A0 =C2=A0 =C2=A0 $fdt" > =C2=A0echo "preadv support =C2=A0 =C2=A0$preadv" > =C2=A0echo "fdatasync =C2=A0 =C2=A0 =C2=A0 =C2=A0 $fdatasync" > +echo "madvise =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 $madvise" > +echo "posix_madvise =C2=A0 =C2=A0 $posix_madvise" > =C2=A0echo "uuid support =C2=A0 =C2=A0 =C2=A0$uuid" > =C2=A0echo "vhost-net support $vhost_net" > =C2=A0echo "Trace backend =C2=A0 =C2=A0 $trace_backend" > @@ -2466,6 +2493,12 @@ fi > =C2=A0if test "$fdatasync" =3D "yes" ; then > =C2=A0 echo "CONFIG_FDATASYNC=3Dy" >> $config_host_mak > =C2=A0fi > +if test "$madvise" =3D "yes" ; then > + =C2=A0echo "CONFIG_MADVISE=3Dy" >> $config_host_mak > +fi > +if test "$posix_madvise" =3D "yes" ; then > + =C2=A0echo "CONFIG_POSIX_MADVISE=3Dy" >> $config_host_mak > +fi > > =C2=A0# XXX: suppress that > =C2=A0if [ "$bsd" =3D "yes" ] ; then > diff --git a/exec.c b/exec.c > index 380dab5..9b5464f 100644 > --- a/exec.c > +++ b/exec.c > @@ -2841,9 +2841,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev= , const char *name, > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 new_block->host =3D file_ram_al= loc(new_block, size, mem_path); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!new_block->host) { > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 new_block->host = =3D qemu_vmalloc(size); > -#ifdef MADV_MERGEABLE > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0madvise(new_bloc= k->host, size, MADV_MERGEABLE); > -#endif > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_madvise(new= _block->host, size, QEMU_MADV_MERGEABLE); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } > =C2=A0#else > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fprintf(stderr, "-mem-path opti= on unsupported\n"); > @@ -2858,9 +2856,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev= , const char *name, > =C2=A0#else > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 new_block->host =3D qemu_vmallo= c(size); > =C2=A0#endif > -#ifdef MADV_MERGEABLE > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0madvise(new_block->host, size,= MADV_MERGEABLE); > -#endif > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_madvise(new_block->host, = size, QEMU_MADV_MERGEABLE); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 } > =C2=A0 =C2=A0 } > > diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c > index 9fe3886..1e74674 100644 > --- a/hw/virtio-balloon.c > +++ b/hw/virtio-balloon.c > @@ -51,8 +51,8 @@ static void balloon_page(void *addr, int deflate) > =C2=A0{ > =C2=A0#if defined(__linux__) > =C2=A0 =C2=A0 if (!kvm_enabled() || kvm_has_sync_mmu()) > - =C2=A0 =C2=A0 =C2=A0 =C2=A0madvise(addr, TARGET_PAGE_SIZE, > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0deflate ? MADV_W= ILLNEED : MADV_DONTNEED); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_madvise(addr, TARGET_PAGE_SIZE, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0deflate ? QEMU_M= ADV_WILLNEED : QEMU_MADV_DONTNEED); > =C2=A0#endif > =C2=A0} > > diff --git a/kvm-all.c b/kvm-all.c > index 58b0404..f7c1d12 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -1031,18 +1031,17 @@ int kvm_has_xcrs(void) > =C2=A0void kvm_setup_guest_memory(void *start, size_t size) > =C2=A0{ > =C2=A0 =C2=A0 if (!kvm_has_sync_mmu()) { > -#ifdef MADV_DONTFORK > - =C2=A0 =C2=A0 =C2=A0 =C2=A0int ret =3D madvise(start, size, MADV_DONTFO= RK); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0int ret =3D qemu_madvise(start, size, QEMU_M= ADV_DONTFORK); > > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (ret =3D=3D -ENOTSUP) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fprintf(stderr, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"N= eed MADV_DONTFORK in absence of synchronous KVM MMU\n"); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0exit(1); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0} > =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (ret) { > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0perror("madvice"); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0perror("qemu_madvise"); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 exit(1); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 } > -#else > - =C2=A0 =C2=A0 =C2=A0 =C2=A0fprintf(stderr, > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"Need MADV_DONTF= ORK in absence of synchronous KVM MMU\n"); > - =C2=A0 =C2=A0 =C2=A0 =C2=A0exit(1); > -#endif > =C2=A0 =C2=A0 } > =C2=A0} > > diff --git a/osdep.c b/osdep.c > index 30426ff..1724a58 100644 > --- a/osdep.c > +++ b/osdep.c > @@ -28,6 +28,7 @@ > =C2=A0#include > =C2=A0#include > =C2=A0#include > +#include > > =C2=A0/* Needed early for CONFIG_BSD etc. */ > =C2=A0#include "config-host.h" > @@ -35,6 +36,9 @@ > =C2=A0#ifdef CONFIG_SOLARIS > =C2=A0#include > =C2=A0#include > +/* See MySQL bug #7156 (http://bugs.mysql.com/bug.php?id=3D7156) for > + =C2=A0 discussion about Solaris header problems */ > +extern int madvise(caddr_t, size_t, int); > =C2=A0#endif > > =C2=A0#ifdef CONFIG_EVENTFD > @@ -139,6 +143,20 @@ void qemu_vfree(void *ptr) > > =C2=A0#endif > > +int qemu_madvise(void *addr, size_t len, int advice) > +{ > + =C2=A0 =C2=A0if (advice =3D=3D QEMU_MADV_INVALID) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return -ENOTSUP; > + =C2=A0 =C2=A0} > +#if defined(CONFIG_MADVISE) > + =C2=A0 =C2=A0return madvise(addr, len, advice); > +#elif defined(CONFIG_POSIX_MADVISE) > + =C2=A0 =C2=A0return posix_madvise(addr, len, advice); > +#else > + =C2=A0 =C2=A0return -ENOTSUP; > +#endif > +} > + > =C2=A0int qemu_create_pidfile(const char *filename) > =C2=A0{ > =C2=A0 =C2=A0 char buffer[128]; > diff --git a/osdep.h b/osdep.h > index 1cdc7e2..6fb4ff3 100644 > --- a/osdep.h > +++ b/osdep.h > @@ -90,6 +90,41 @@ void *qemu_memalign(size_t alignment, size_t size); > =C2=A0void *qemu_vmalloc(size_t size); > =C2=A0void qemu_vfree(void *ptr); > > +#define QEMU_MADV_INVALID -1 > + > +#if defined(CONFIG_MADVISE) > + > +#define QEMU_MADV_WILLNEED =C2=A0MADV_WILLNEED > +#define QEMU_MADV_DONTNEED =C2=A0MADV_DONTNEED > +#ifdef MADV_DONTFORK > +#define QEMU_MADV_DONTFORK =C2=A0MADV_DONTFORK > +#else > +#define QEMU_MADV_DONTFORK =C2=A0QEMU_MADV_INVALID > +#endif > +#ifdef MADV_MERGEABLE > +#define QEMU_MADV_MERGEABLE MADV_MERGEABLE > +#else > +#define QEMU_MADV_MERGEABLE QEMU_MADV_INVALID > +#endif > + > +#elif defined(CONFIG_POSIX_MADVISE) > + > +#define QEMU_MADV_WILLNEED =C2=A0POSIX_MADV_WILLNEED > +#define QEMU_MADV_DONTNEED =C2=A0POSIX_MADV_DONTNEED > +#define QEMU_MADV_DONTFORK =C2=A0POSIX_MADV_NORMAL > +#define QEMU_MADV_MERGEABLE POSIX_MADV_NORMAL Shouldn't these two (or at least QEMU_MADV_DONTFORK) be defined to QEMU_MADV_INVALID, so KVM check will fail as it should? Otherwise I'm happy with the patch.