| Message ID | 1284891104-3652-1-git-send-email-andreas.faerber@web.de |
|---|---|
| State | New |
| Headers | show |
On Sun, Sep 19, 2010 at 10:11 AM, Andreas Färber <andreas.faerber@web.de> wrote: > From: Andreas Färber <afaerber@opensolaris.org> > > 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_madvise() > as wrapper. Prefer madvise() over posix_madvise() due to flag availability. > Convert all callers to use qemu_madvise() and QEMU_MADV_*. > > Note that on Solaris the warning is fixed by moving the madvise() prototype, > not by qemu_madvise() itself. It helps with porting though, and it simplifies > most call sites. > > v6 -> v7: > * Adopt madvise() rather than posix_madvise() semantics for returning errors. > * Use EINVAL in place of ENOTSUP. > > v5 -> v6: > * Replace two leftover instances of POSIX_MADV_NORMAL with QEMU_MADV_INVALID. > Spotted by Blue Swirl. > > v4 -> v5: > * Introduce QEMU_MADV_INVALID, suggested by Alexander Graf. > Note that this relies on -1 not being a valid advice value. > > v3 -> v4: > * Eliminate #ifdefs at qemu_advise() call sites. Requested by Blue Swirl. > This will currently break the check in kvm-all.c by calling madvise() with > a 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. > Add defines to Makefile, not QEMU_CFLAGS. > Convert 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(). > Suggested by Blue Swirl. > > Signed-off-by: Andreas Färber <afaerber@opensolaris.org> > Cc: Blue Swirl <blauwirbel@gmail.com> > Cc: Alexander Graf <agraf@suse.de> > Cc: Andrea Arcangeli <aarcange@redhat.com> > --- > arch_init.c | 2 +- > configure | 33 +++++++++++++++++++++++++++++++++ > exec.c | 8 ++------ > hw/virtio-balloon.c | 4 ++-- > kvm-all.c | 12 ++++-------- > osdep.c | 20 ++++++++++++++++++++ > osdep.h | 35 +++++++++++++++++++++++++++++++++++ > vl.c | 3 --- > 8 files changed, 97 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_id) > #ifndef _WIN32 > if (ch == 0 && > (!kvm_enabled() || kvm_has_sync_mmu())) { > - madvise(host, TARGET_PAGE_SIZE, MADV_DONTNEED); > + qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED); > } > #endif > } 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 > fi > > ########################################## > +# check if we have madvise > + > +madvise=no > +cat > $TMPC << EOF > +#include <sys/types.h> > +#include <sys/mman.h> > +int main(void) { return madvise(NULL, 0, MADV_DONTNEED); } > +EOF > +if compile_prog "" "" ; then > + madvise=yes > +fi > + > +########################################## > +# check if we have posix_madvise > + > +posix_madvise=no > +cat > $TMPC << EOF > +#include <sys/mman.h> > +int main(void) { return posix_madvise(NULL, 0, POSIX_MADV_DONTNEED); } > +EOF > +if compile_prog "" "" ; then > + posix_madvise=yes > +fi > + > +########################################## > # check if trace backend exists > > sh "$source_path/tracetool" "--$trace_backend" --check-backend > /dev/null 2> /dev/null > @@ -2238,6 +2263,8 @@ echo "KVM support $kvm" > echo "fdt support $fdt" > echo "preadv support $preadv" > echo "fdatasync $fdatasync" > +echo "madvise $madvise" > +echo "posix_madvise $posix_madvise" > echo "uuid support $uuid" > echo "vhost-net support $vhost_net" > echo "Trace backend $trace_backend" > @@ -2478,6 +2505,12 @@ fi > if test "$fdatasync" = "yes" ; then > echo "CONFIG_FDATASYNC=y" >> $config_host_mak > fi > +if test "$madvise" = "yes" ; then > + echo "CONFIG_MADVISE=y" >> $config_host_mak > +fi > +if test "$posix_madvise" = "yes" ; then > + echo "CONFIG_POSIX_MADVISE=y" >> $config_host_mak > +fi > > # XXX: suppress that > if [ "$bsd" = "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, > new_block->host = file_ram_alloc(new_block, size, mem_path); > if (!new_block->host) { > new_block->host = qemu_vmalloc(size); > -#ifdef MADV_MERGEABLE > - madvise(new_block->host, size, MADV_MERGEABLE); > -#endif > + qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE); > } > #else > fprintf(stderr, "-mem-path option unsupported\n"); > @@ -2858,9 +2856,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name, > #else > new_block->host = qemu_vmalloc(size); > #endif > -#ifdef MADV_MERGEABLE > - madvise(new_block->host, size, MADV_MERGEABLE); > -#endif > + qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE); > } > } > > 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) > { > #if defined(__linux__) > if (!kvm_enabled() || kvm_has_sync_mmu()) > - madvise(addr, TARGET_PAGE_SIZE, > - deflate ? MADV_WILLNEED : MADV_DONTNEED); > + qemu_madvise(addr, TARGET_PAGE_SIZE, > + deflate ? QEMU_MADV_WILLNEED : QEMU_MADV_DONTNEED); > #endif > } > > 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) > void kvm_setup_guest_memory(void *start, size_t size) > { > if (!kvm_has_sync_mmu()) { > -#ifdef MADV_DONTFORK > - int ret = madvise(start, size, MADV_DONTFORK); > + int ret = qemu_madvise(start, size, QEMU_MADV_DONTFORK); > > if (ret) { > - perror("madvice"); > + perror("qemu_madvise"); > + fprintf(stderr, > + "Need MADV_DONTFORK in absence of synchronous KVM MMU\n"); > exit(1); > } > -#else > - fprintf(stderr, > - "Need MADV_DONTFORK in absence of synchronous KVM MMU\n"); > - exit(1); > -#endif > } > } > > diff --git a/osdep.c b/osdep.c > index 30426ff..56c6944 100644 > --- a/osdep.c > +++ b/osdep.c > @@ -28,6 +28,7 @@ > #include <errno.h> > #include <unistd.h> > #include <fcntl.h> > +#include <sys/mman.h> With the patch applied, I get a warning here with mingw from Debian stable: CC osdep.o /src/qemu/osdep.c:31:22: error: sys/mman.h: No such file or directory For some reason, it doesn't happen with newer mingw from Debian testing.
Am 20.09.2010 um 22:33 schrieb Blue Swirl: > On Sun, Sep 19, 2010 at 10:11 AM, Andreas Färber <andreas.faerber@web.de > > wrote: >> From: Andreas Färber <afaerber@opensolaris.org> >> >> 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_madvise() >> as wrapper. Prefer madvise() over posix_madvise() due to flag >> availability. >> Convert all callers to use qemu_madvise() and QEMU_MADV_*. >> >> Note that on Solaris the warning is fixed by moving the madvise() >> prototype, >> not by qemu_madvise() itself. It helps with porting though, and it >> simplifies >> most call sites. >> >> v6 -> v7: >> * Adopt madvise() rather than posix_madvise() semantics for >> returning errors. >> * Use EINVAL in place of ENOTSUP. >> >> v5 -> v6: >> * Replace two leftover instances of POSIX_MADV_NORMAL with >> QEMU_MADV_INVALID. >> Spotted by Blue Swirl. >> >> v4 -> v5: >> * Introduce QEMU_MADV_INVALID, suggested by Alexander Graf. >> Note that this relies on -1 not being a valid advice value. >> >> v3 -> v4: >> * Eliminate #ifdefs at qemu_advise() call sites. Requested by Blue >> Swirl. >> This will currently break the check in kvm-all.c by calling >> madvise() with >> a 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. >> Add defines to Makefile, not QEMU_CFLAGS. >> Convert 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(). >> Suggested by Blue Swirl. >> >> Signed-off-by: Andreas Färber <afaerber@opensolaris.org> >> Cc: Blue Swirl <blauwirbel@gmail.com> >> Cc: Alexander Graf <agraf@suse.de> >> Cc: Andrea Arcangeli <aarcange@redhat.com> >> --- >> arch_init.c | 2 +- >> configure | 33 +++++++++++++++++++++++++++++++++ >> exec.c | 8 ++------ >> hw/virtio-balloon.c | 4 ++-- >> kvm-all.c | 12 ++++-------- >> osdep.c | 20 ++++++++++++++++++++ >> osdep.h | 35 +++++++++++++++++++++++++++++++++++ >> vl.c | 3 --- >> 8 files changed, 97 insertions(+), 20 deletions(-) >> diff --git a/osdep.c b/osdep.c >> index 30426ff..56c6944 100644 >> --- a/osdep.c >> +++ b/osdep.c >> @@ -28,6 +28,7 @@ >> #include <errno.h> >> #include <unistd.h> >> #include <fcntl.h> >> +#include <sys/mman.h> > > With the patch applied, I get a warning here with mingw from Debian > stable: > CC osdep.o > /src/qemu/osdep.c:31:22: error: sys/mman.h: No such file or directory > > For some reason, it doesn't happen with newer mingw from Debian > testing. Any suggestions what to do about that? Would it work without, i.e. could we enclose it in #ifndef _WIN32? Andreas
On Fri, Sep 24, 2010 at 6:08 PM, Andreas Färber <andreas.faerber@web.de> wrote: > Am 20.09.2010 um 22:33 schrieb Blue Swirl: > >> On Sun, Sep 19, 2010 at 10:11 AM, Andreas Färber <andreas.faerber@web.de> >> wrote: >>> >>> From: Andreas Färber <afaerber@opensolaris.org> >>> >>> 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_madvise() >>> as wrapper. Prefer madvise() over posix_madvise() due to flag >>> availability. >>> Convert all callers to use qemu_madvise() and QEMU_MADV_*. >>> >>> Note that on Solaris the warning is fixed by moving the madvise() >>> prototype, >>> not by qemu_madvise() itself. It helps with porting though, and it >>> simplifies >>> most call sites. >>> >>> v6 -> v7: >>> * Adopt madvise() rather than posix_madvise() semantics for returning >>> errors. >>> * Use EINVAL in place of ENOTSUP. >>> >>> v5 -> v6: >>> * Replace two leftover instances of POSIX_MADV_NORMAL with >>> QEMU_MADV_INVALID. >>> Spotted by Blue Swirl. >>> >>> v4 -> v5: >>> * Introduce QEMU_MADV_INVALID, suggested by Alexander Graf. >>> Note that this relies on -1 not being a valid advice value. >>> >>> v3 -> v4: >>> * Eliminate #ifdefs at qemu_advise() call sites. Requested by Blue Swirl. >>> This will currently break the check in kvm-all.c by calling madvise() >>> with >>> a 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. >>> Add defines to Makefile, not QEMU_CFLAGS. >>> Convert 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(). >>> Suggested by Blue Swirl. >>> >>> Signed-off-by: Andreas Färber <afaerber@opensolaris.org> >>> Cc: Blue Swirl <blauwirbel@gmail.com> >>> Cc: Alexander Graf <agraf@suse.de> >>> Cc: Andrea Arcangeli <aarcange@redhat.com> >>> --- >>> arch_init.c | 2 +- >>> configure | 33 +++++++++++++++++++++++++++++++++ >>> exec.c | 8 ++------ >>> hw/virtio-balloon.c | 4 ++-- >>> kvm-all.c | 12 ++++-------- >>> osdep.c | 20 ++++++++++++++++++++ >>> osdep.h | 35 +++++++++++++++++++++++++++++++++++ >>> vl.c | 3 --- >>> 8 files changed, 97 insertions(+), 20 deletions(-) > >>> diff --git a/osdep.c b/osdep.c >>> index 30426ff..56c6944 100644 >>> --- a/osdep.c >>> +++ b/osdep.c >>> @@ -28,6 +28,7 @@ >>> #include <errno.h> >>> #include <unistd.h> >>> #include <fcntl.h> >>> +#include <sys/mman.h> >> >> With the patch applied, I get a warning here with mingw from Debian >> stable: >> CC osdep.o >> /src/qemu/osdep.c:31:22: error: sys/mman.h: No such file or directory >> >> For some reason, it doesn't happen with newer mingw from Debian testing. > > Any suggestions what to do about that? Would it work without, i.e. could we > enclose it in #ifndef _WIN32? There's no madvise in mingw, so either that or #if defined(CONFIG_MADVISE) || defined(CONFIG_POSIX_MADVISE).
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_id) #ifndef _WIN32 if (ch == 0 && (!kvm_enabled() || kvm_has_sync_mmu())) { - madvise(host, TARGET_PAGE_SIZE, MADV_DONTNEED); + qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED); } #endif } 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 fi ########################################## +# check if we have madvise + +madvise=no +cat > $TMPC << EOF +#include <sys/types.h> +#include <sys/mman.h> +int main(void) { return madvise(NULL, 0, MADV_DONTNEED); } +EOF +if compile_prog "" "" ; then + madvise=yes +fi + +########################################## +# check if we have posix_madvise + +posix_madvise=no +cat > $TMPC << EOF +#include <sys/mman.h> +int main(void) { return posix_madvise(NULL, 0, POSIX_MADV_DONTNEED); } +EOF +if compile_prog "" "" ; then + posix_madvise=yes +fi + +########################################## # check if trace backend exists sh "$source_path/tracetool" "--$trace_backend" --check-backend > /dev/null 2> /dev/null @@ -2238,6 +2263,8 @@ echo "KVM support $kvm" echo "fdt support $fdt" echo "preadv support $preadv" echo "fdatasync $fdatasync" +echo "madvise $madvise" +echo "posix_madvise $posix_madvise" echo "uuid support $uuid" echo "vhost-net support $vhost_net" echo "Trace backend $trace_backend" @@ -2478,6 +2505,12 @@ fi if test "$fdatasync" = "yes" ; then echo "CONFIG_FDATASYNC=y" >> $config_host_mak fi +if test "$madvise" = "yes" ; then + echo "CONFIG_MADVISE=y" >> $config_host_mak +fi +if test "$posix_madvise" = "yes" ; then + echo "CONFIG_POSIX_MADVISE=y" >> $config_host_mak +fi # XXX: suppress that if [ "$bsd" = "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, new_block->host = file_ram_alloc(new_block, size, mem_path); if (!new_block->host) { new_block->host = qemu_vmalloc(size); -#ifdef MADV_MERGEABLE - madvise(new_block->host, size, MADV_MERGEABLE); -#endif + qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE); } #else fprintf(stderr, "-mem-path option unsupported\n"); @@ -2858,9 +2856,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name, #else new_block->host = qemu_vmalloc(size); #endif -#ifdef MADV_MERGEABLE - madvise(new_block->host, size, MADV_MERGEABLE); -#endif + qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE); } } 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) { #if defined(__linux__) if (!kvm_enabled() || kvm_has_sync_mmu()) - madvise(addr, TARGET_PAGE_SIZE, - deflate ? MADV_WILLNEED : MADV_DONTNEED); + qemu_madvise(addr, TARGET_PAGE_SIZE, + deflate ? QEMU_MADV_WILLNEED : QEMU_MADV_DONTNEED); #endif } 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) void kvm_setup_guest_memory(void *start, size_t size) { if (!kvm_has_sync_mmu()) { -#ifdef MADV_DONTFORK - int ret = madvise(start, size, MADV_DONTFORK); + int ret = qemu_madvise(start, size, QEMU_MADV_DONTFORK); if (ret) { - perror("madvice"); + perror("qemu_madvise"); + fprintf(stderr, + "Need MADV_DONTFORK in absence of synchronous KVM MMU\n"); exit(1); } -#else - fprintf(stderr, - "Need MADV_DONTFORK in absence of synchronous KVM MMU\n"); - exit(1); -#endif } } diff --git a/osdep.c b/osdep.c index 30426ff..56c6944 100644 --- a/osdep.c +++ b/osdep.c @@ -28,6 +28,7 @@ #include <errno.h> #include <unistd.h> #include <fcntl.h> +#include <sys/mman.h> /* Needed early for CONFIG_BSD etc. */ #include "config-host.h" @@ -35,6 +36,9 @@ #ifdef CONFIG_SOLARIS #include <sys/types.h> #include <sys/statvfs.h> +/* See MySQL bug #7156 (http://bugs.mysql.com/bug.php?id=7156) for + discussion about Solaris header problems */ +extern int madvise(caddr_t, size_t, int); #endif #ifdef CONFIG_EVENTFD @@ -139,6 +143,22 @@ void qemu_vfree(void *ptr) #endif +int qemu_madvise(void *addr, size_t len, int advice) +{ + if (advice == QEMU_MADV_INVALID) { + errno = EINVAL; + return -1; + } +#if defined(CONFIG_MADVISE) + return madvise(addr, len, advice); +#elif defined(CONFIG_POSIX_MADVISE) + return posix_madvise(addr, len, advice); +#else + errno = EINVAL; + return -1; +#endif +} + int qemu_create_pidfile(const char *filename) { 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); void *qemu_vmalloc(size_t size); void qemu_vfree(void *ptr); +#define QEMU_MADV_INVALID -1 + +#if defined(CONFIG_MADVISE) + +#define QEMU_MADV_WILLNEED MADV_WILLNEED +#define QEMU_MADV_DONTNEED MADV_DONTNEED +#ifdef MADV_DONTFORK +#define QEMU_MADV_DONTFORK MADV_DONTFORK +#else +#define QEMU_MADV_DONTFORK QEMU_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 POSIX_MADV_WILLNEED +#define QEMU_MADV_DONTNEED POSIX_MADV_DONTNEED +#define QEMU_MADV_DONTFORK QEMU_MADV_INVALID +#define QEMU_MADV_MERGEABLE QEMU_MADV_INVALID + +#else /* no-op */ + +#define QEMU_MADV_WILLNEED QEMU_MADV_INVALID +#define QEMU_MADV_DONTNEED QEMU_MADV_INVALID +#define QEMU_MADV_DONTFORK QEMU_MADV_INVALID +#define QEMU_MADV_MERGEABLE QEMU_MADV_INVALID + +#endif + +int qemu_madvise(void *addr, size_t len, int advice); + int qemu_create_pidfile(const char *filename); #ifdef _WIN32 diff --git a/vl.c b/vl.c index 3f45aa9..d352d18 100644 --- a/vl.c +++ b/vl.c @@ -80,9 +80,6 @@ #include <net/if.h> #include <syslog.h> #include <stropts.h> -/* See MySQL bug #7156 (http://bugs.mysql.com/bug.php?id=7156) for - discussion about Solaris header problems */ -extern int madvise(caddr_t, size_t, int); #endif #endif #endif