From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=46254 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OzWVk-0004vR-QN for qemu-devel@nongnu.org; Sat, 25 Sep 2010 11:18:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OzWVf-0004dr-Hh for qemu-devel@nongnu.org; Sat, 25 Sep 2010 11:18:01 -0400 Received: from mail-qy0-f173.google.com ([209.85.216.173]:57103) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OzWVf-0004dg-BT for qemu-devel@nongnu.org; Sat, 25 Sep 2010 11:17:55 -0400 Received: by qyk9 with SMTP id 9so2964716qyk.4 for ; Sat, 25 Sep 2010 08:17:54 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1285412289-3253-1-git-send-email-andreas.faerber@web.de> References: <1285412289-3253-1-git-send-email-andreas.faerber@web.de> From: Blue Swirl Date: Sat, 25 Sep 2010 15:17:33 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: [PATCH v8] 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: Andrea Arcangeli , qemu-devel@nongnu.org, Alexander Graf Thanks, applied. I moved the include line below #include "config-host.h", otherwise CONFIG_xx would have been undefined. On Sat, Sep 25, 2010 at 10:58 AM, 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(). > MinGW implements neither. > > 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 helps with porting though, and it simpli= fies > most call sites. > > v7 -> v8: > * Some versions of MinGW have no sys/mman.h header. Reported by Blue Swir= l. > > v6 -> v7: > * Adopt madvise() rather than posix_madvise() semantics for returning err= ors. > * Use EINVAL in place of ENOTSUP. > > v5 -> v6: > * Replace two leftover instances of POSIX_MADV_NORMAL with QEMU_MADV_INVA= LID. > =C2=A0Spotted by Blue Swirl. > > 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 > Cc: Andrea Arcangeli > --- > =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 12 ++++------= -- > =C2=A0osdep.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 22 +++++= +++++++++++++++++ > =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, 99 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 3bfc5e9..6a21bf2 100755 > --- a/configure > +++ b/configure > @@ -2072,6 +2072,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 > @@ -2238,6 +2263,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" > @@ -2478,6 +2505,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..1cc696f 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -1031,18 +1031,14 @@ 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=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=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=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..2c53015 100644 > --- a/osdep.c > +++ b/osdep.c > @@ -28,6 +28,9 @@ > =C2=A0#include > =C2=A0#include > =C2=A0#include > +#if defined(CONFIG_MADVISE) || defined(CONFIG_POSIX_MADVISE) > +#include > +#endif > > =C2=A0/* Needed early for CONFIG_BSD etc. */ > =C2=A0#include "config-host.h" > @@ -35,6 +38,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 +145,22 @@ 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=A0errno =3D EINVAL; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return -1; > + =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=A0errno =3D EINVAL; > + =C2=A0 =C2=A0return -1; > +#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..6716281 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=A0QEMU_MADV_INVALID > +#define QEMU_MADV_MERGEABLE QEMU_MADV_INVALID > + > +#else /* no-op */ > + > +#define QEMU_MADV_WILLNEED =C2=A0QEMU_MADV_INVALID > +#define QEMU_MADV_DONTNEED =C2=A0QEMU_MADV_INVALID > +#define QEMU_MADV_DONTFORK =C2=A0QEMU_MADV_INVALID > +#define QEMU_MADV_MERGEABLE QEMU_MADV_INVALID > + > +#endif > + > +int qemu_madvise(void *addr, size_t len, int advice); > + > =C2=A0int qemu_create_pidfile(const char *filename); > > =C2=A0#ifdef _WIN32 > diff --git a/vl.c b/vl.c > index 3f45aa9..d352d18 100644 > --- a/vl.c > +++ b/vl.c > @@ -80,9 +80,6 @@ > =C2=A0#include > =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#endif > =C2=A0#endif > -- > 1.7.2.2 > >