[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH v5] Introduce qemu_madvise()
From: |
Blue Swirl |
Subject: |
[Qemu-devel] Re: [PATCH v5] Introduce qemu_madvise() |
Date: |
Tue, 14 Sep 2010 20:36:28 +0000 |
On Tue, Sep 14, 2010 at 8:28 PM, Andreas Färber <address@hidden> wrote:
> From: Andreas Färber <address@hidden>
>
> 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 will help with future porting though, and
> it simplifies most call sites.
>
> 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 <address@hidden>
> Cc: Blue Swirl <address@hidden>
> Cc: Alexander Graf <address@hidden>
> ---
> arch_init.c | 2 +-
> configure | 33 +++++++++++++++++++++++++++++++++
> exec.c | 8 ++------
> hw/virtio-balloon.c | 4 ++--
> kvm-all.c | 15 +++++++--------
> osdep.c | 18 ++++++++++++++++++
> osdep.h | 35 +++++++++++++++++++++++++++++++++++
> vl.c | 3 ---
> 8 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_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 4061cb7..86558eb 100755
> --- a/configure
> +++ b/configure
> @@ -2069,6 +2069,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
> @@ -2226,6 +2251,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"
> @@ -2466,6 +2493,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..f7c1d12 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1031,18 +1031,17 @@ 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 == -ENOTSUP) {
> + fprintf(stderr,
> + "Need MADV_DONTFORK in absence of synchronous KVM
> MMU\n");
> + exit(1);
> + }
> if (ret) {
> - perror("madvice");
> + perror("qemu_madvise");
> 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..1724a58 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,20 @@ void qemu_vfree(void *ptr)
>
> #endif
>
> +int qemu_madvise(void *addr, size_t len, int advice)
> +{
> + if (advice == QEMU_MADV_INVALID) {
> + return -ENOTSUP;
> + }
> +#if defined(CONFIG_MADVISE)
> + return madvise(addr, len, advice);
> +#elif defined(CONFIG_POSIX_MADVISE)
> + return posix_madvise(addr, len, advice);
> +#else
> + return -ENOTSUP;
> +#endif
> +}
> +
> int qemu_create_pidfile(const char *filename)
> {
> 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);
> 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 POSIX_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.
- [Qemu-devel] Re: [PATCH] Introduce qemu_madvise(), (continued)
- [Qemu-devel] Re: [PATCH] Introduce qemu_madvise(), Andreas Färber, 2010/09/12
- [Qemu-devel] Re: [PATCH] Introduce qemu_madvise(), Blue Swirl, 2010/09/12
- [Qemu-devel] [PATCH v3] Introduce qemu_madvise(), Andreas Färber, 2010/09/12
- [Qemu-devel] Re: [PATCH v3] Introduce qemu_madvise(), Blue Swirl, 2010/09/12
- [Qemu-devel] Re: [PATCH v3] Introduce qemu_madvise(), Alexander Graf, 2010/09/13
- [Qemu-devel] [RFC v4] Introduce qemu_madvise(), Andreas Färber, 2010/09/13
- [Qemu-devel] Re: [RFC v4] Introduce qemu_madvise(), Blue Swirl, 2010/09/14
- [Qemu-devel] Re: [RFC v4] Introduce qemu_madvise(), Alexander Graf, 2010/09/14
- [Qemu-devel] Re: [RFC v4] Introduce qemu_madvise(), Blue Swirl, 2010/09/14
- [Qemu-devel] [PATCH v5] Introduce qemu_madvise(), Andreas Färber, 2010/09/14
- [Qemu-devel] Re: [PATCH v5] Introduce qemu_madvise(),
Blue Swirl <=
- [Qemu-devel] Re: [PATCH v5] Introduce qemu_madvise(), Andreas Färber, 2010/09/14
- [Qemu-devel] [PATCH v6] Introduce qemu_madvise(), Andreas Färber, 2010/09/15
- [Qemu-devel] Re: [PATCH v6] Introduce qemu_madvise(), Blue Swirl, 2010/09/15
- [Qemu-devel] Re: [PATCH v6] Introduce qemu_madvise(), Andreas Färber, 2010/09/15
- [Qemu-devel] Re: [PATCH v6] Introduce qemu_madvise(), Blue Swirl, 2010/09/15
- [Qemu-devel] Re: [PATCH v6] Introduce qemu_madvise(), Andreas Färber, 2010/09/15
- [Qemu-devel] [PATCH v7] Introduce qemu_madvise(), Andreas Färber, 2010/09/19
- [Qemu-devel] Re: [PATCH v7] Introduce qemu_madvise(), Blue Swirl, 2010/09/20
- Re: [Qemu-devel] Re: [PATCH v7] Introduce qemu_madvise(), Andreas Färber, 2010/09/24
- Re: [Qemu-devel] Re: [PATCH v7] Introduce qemu_madvise(), Blue Swirl, 2010/09/25