[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Introduce qemu_madvise()
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH] Introduce qemu_madvise() |
Date: |
Sat, 11 Sep 2010 23:37:05 +0200 |
On 11.09.2010, at 19:05, Andreas Färber 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.
> The underlying issue is that madvise() is not a POSIX function,
> therefore Solaris' _POSIX_C_SOURCE suppresses the prototype.
>
> Haiku doesn't implement madvise() at all.
> OpenBSD however doesn't implement posix_madvise().
>
> Check for posix_madvise() in configure and supply qemu_madvise() as wrapper.
> Use qemu_madvise() in arch_init.c's ram_load().
>
> http://www.opengroup.org/onlinepubs/9699919799/functions/posix_madvise.html
>
> Remaining madvise() users:
> exec.c: limited to __linux__ and/or MADV_MERGEABLE (no POSIX equivalent)
> kvm-all.c: limited to MADV_DONTFORK (no POSIX equivalent),
> otherwise runtime error if !kvm_has_sync_mmu()
> hw/virtio-balloon.c: limited to __linux__
>
> 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>
> ---
> arch_init.c | 2 +-
> configure | 11 +++++++++++
> osdep.c | 26 ++++++++++++++++++++++++++
> osdep.h | 4 ++++
> vl.c | 3 ---
> 5 files changed, 42 insertions(+), 4 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);
Is this the only occurence in the whole code base? This patch should convert
all users, right?
> }
> #endif
> } else if (flags & RAM_SAVE_FLAG_PAGE) {
> diff --git a/configure b/configure
> index 4061cb7..5e6f3e1 100755
> --- a/configure
> +++ b/configure
> @@ -2069,6 +2069,17 @@ if compile_prog "" "" ; then
> fi
>
> ##########################################
> +# check if we have posix_madvise
> +
> +cat > $TMPC << EOF
> +#include <sys/mman.h>
> +int main(void) { posix_madvise(NULL, 0, POSIX_MADV_DONTNEED); return 0; }
> +EOF
> +if compile_prog "" "" ; then
> + QEMU_CFLAGS="-DCONFIG_POSIX_MADVISE ${QEMU_CFLAGS}"
> +fi
> +
> +##########################################
> # check if trace backend exists
>
> sh "$source_path/tracetool" "--$trace_backend" --check-backend > /dev/null 2>
> /dev/null
> diff --git a/osdep.c b/osdep.c
> index 30426ff..8c09597 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"
> @@ -139,6 +140,31 @@ void qemu_vfree(void *ptr)
>
> #endif
>
> +int qemu_madvise(void *addr, size_t len, int advice)
> +{
> +#ifdef CONFIG_POSIX_MADVISE
> + switch (advice) {
> + case QEMU_MADV_DONTNEED:
> + advice = POSIX_MADV_DONTNEED;
> + break;
> + default:
> + fprintf(stderr, "Advice %d unhandled.\n", advice);
Advise :) It should probably also be 'madvise' here. Plus, I'd recommend %x as
that makes the bits slightly more obvious.
> + abort();
> + }
> + return posix_madvise(addr, len, advice);
> +#else
> + switch (advice) {
> + case QEMU_MADV_DONTNEED:
> + advice = MADV_DONTNEED;
> + break;
> + default:
> + fprintf(stderr, "Advice %d unhandled.\n", advice);
> + abort();
> + }
> + return madvise(addr, len, advice);
So what do you do on haiku where there's no madvise?
> +#endif
> +}
> +
> int qemu_create_pidfile(const char *filename)
> {
> char buffer[128];
> diff --git a/osdep.h b/osdep.h
> index 1cdc7e2..9f0da46 100644
> --- a/osdep.h
> +++ b/osdep.h
> @@ -90,6 +90,10 @@ void *qemu_memalign(size_t alignment, size_t size);
> void *qemu_vmalloc(size_t size);
> void qemu_vfree(void *ptr);
>
> +#define QEMU_MADV_DONTNEED 1
(1 << 0)
There are probably more to follow. I'm also not sure if it wouldn't make sense
to just directly map qemu_madvise and real madvise bits. Something like
#ifdef MADV_DONTNEED
#define QEMU_MADV_DONTNEED (1 << 0)
#else
#define QEMU_MADV_DONTNEED 0
#endif
Unless of course a flag is mandatory - but I don't think any of the madvise
bits are.
> +
> +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);
Does Solaris have posix_madvise? Otherwise it would still require this header
hack, no?
Alex
- [Qemu-devel] [PATCH] Introduce qemu_madvise(), Andreas Färber, 2010/09/11
- Re: [Qemu-devel] [PATCH] Introduce qemu_madvise(),
Alexander Graf <=
- [Qemu-devel] Re: [PATCH] Introduce qemu_madvise(), Blue Swirl, 2010/09/12
- [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