* [Qemu-devel] [PATCH] Introduce qemu_madvise() @ 2010-09-11 17:05 Andreas Färber 2010-09-11 21:37 ` Alexander Graf 2010-09-12 7:20 ` [Qemu-devel] " Blue Swirl 0 siblings, 2 replies; 28+ messages in thread From: Andreas Färber @ 2010-09-11 17:05 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Andreas Färber 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. 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 <afaerber@opensolaris.org> Cc: Blue Swirl <blauwirbel@gmail.com> --- 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); } #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); + 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); +#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 + +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 -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH] Introduce qemu_madvise() 2010-09-11 17:05 [Qemu-devel] [PATCH] Introduce qemu_madvise() Andreas Färber @ 2010-09-11 21:37 ` Alexander Graf 2010-09-11 22:39 ` Andreas Färber 2010-09-12 7:20 ` [Qemu-devel] " Blue Swirl 1 sibling, 1 reply; 28+ messages in thread From: Alexander Graf @ 2010-09-11 21:37 UTC (permalink / raw) To: Andreas Färber; +Cc: Blue Swirl, qemu-devel, Andreas Färber On 11.09.2010, at 19:05, Andreas Färber 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. > 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 <afaerber@opensolaris.org> > Cc: Blue Swirl <blauwirbel@gmail.com> > --- > 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH] Introduce qemu_madvise() 2010-09-11 21:37 ` Alexander Graf @ 2010-09-11 22:39 ` Andreas Färber 2010-09-11 22:47 ` Alexander Graf 0 siblings, 1 reply; 28+ messages in thread From: Andreas Färber @ 2010-09-11 22:39 UTC (permalink / raw) To: Alexander Graf; +Cc: Blue Swirl, QEMU Developers Am 11.09.2010 um 23:37 schrieb Alexander Graf: > On 11.09.2010, at 19:05, Andreas Färber wrote: > >> 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__ >> 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? It's the only relevant occurrence, cf. description above. We could in theory create QEMU_MADV_MERGEABLE and QEMU_MADV_DONTFORK and convert the callers, too, but what's the point when only madvise supports it? Using the current #ifdef mechanism we can detect/handle it at compile- time; inside qemu_madvise it would be deferred to runtime. Or do you have a suggestion how to handle that scenario other than returning an error? >> diff --git a/osdep.c b/osdep.c >> index 30426ff..8c09597 100644 >> --- a/osdep.c >> +++ b/osdep.c >> @@ -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 :) I'd advise against that advice. ;) (*hint hint*) > It should probably also be 'madvise' here. Plus, I'd recommend %x as > that makes the bits slightly more obvious. You mean, "qemu_madvise: Advice %x ..."? Or "Advice %x for posix_madvise ..."? >> + 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? It should detect posix_madvise and not run into this code path, just like OpenSolaris. I was hoping for Michael Lotz to resurface though and haven't retried myself yet. Platforms that have neither posix_madvise nor madvise are equally broken before and after. > >> +#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. I don't follow. Something like the following? #ifdef CONFIG_POSIX_MADVISE #define QEMU_MADV_DONTNEED POSIX_MADV_DONTNEED #define qemu_madvise posix_madvise #else #ifdef MADV_DONTNEED #define QEMU_MADV_DONTNEED MADV_DONTNEED #endif #define qemu_madvise madvise #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); > > Does Solaris have posix_madvise? Otherwise it would still require > this header hack, no? I tested on OpenSolaris 2009.06. Haven't tested on Solaris 10 yet, don't have access to older ones. We could wrap it in #ifndef CONFIG_POSIX_MADVISE to be fool-safe. Thanks for the review, Andreas ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH] Introduce qemu_madvise() 2010-09-11 22:39 ` Andreas Färber @ 2010-09-11 22:47 ` Alexander Graf 0 siblings, 0 replies; 28+ messages in thread From: Alexander Graf @ 2010-09-11 22:47 UTC (permalink / raw) To: Andreas Färber; +Cc: Blue Swirl, QEMU Developers On 12.09.2010, at 00:39, Andreas Färber wrote: > Am 11.09.2010 um 23:37 schrieb Alexander Graf: > >> On 11.09.2010, at 19:05, Andreas Färber wrote: >> >>> 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__ > >>> 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? > > It's the only relevant occurrence, cf. description above. > > We could in theory create QEMU_MADV_MERGEABLE and QEMU_MADV_DONTFORK and convert the callers, too, but what's the point when only madvise supports it? > Using the current #ifdef mechanism we can detect/handle it at compile-time; inside qemu_madvise it would be deferred to runtime. Or do you have a suggestion how to handle that scenario other than returning an error? Hrm. I'm not sure. Do we have to fail madvise at all? > >>> diff --git a/osdep.c b/osdep.c >>> index 30426ff..8c09597 100644 >>> --- a/osdep.c >>> +++ b/osdep.c >>> @@ -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 :) > > I'd advise against that advice. ;) (*hint hint*) > >> It should probably also be 'madvise' here. Plus, I'd recommend %x as that makes the bits slightly more obvious. > > You mean, "qemu_madvise: Advice %x ..."? Or "Advice %x for posix_madvise ..."? I'd remove the "Advice" part: "qemu_madvise: Flag %x unknown". > >>> + 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? > > It should detect posix_madvise and not run into this code path, just like OpenSolaris. > I was hoping for Michael Lotz to resurface though and haven't retried myself yet. Oh, so OpenSolaris and Haiku have posix_madvise? Nice. > > Platforms that have neither posix_madvise nor madvise are equally broken before and after. Sure :). > >> >>> +#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. > > I don't follow. Something like the following? > > #ifdef CONFIG_POSIX_MADVISE > #define QEMU_MADV_DONTNEED POSIX_MADV_DONTNEED > #define qemu_madvise posix_madvise > #else > #ifdef MADV_DONTNEED > #define QEMU_MADV_DONTNEED MADV_DONTNEED > #endif > #define qemu_madvise madvise > #endif This could work, though in general preprocessor magic is ... too magic. I was thinking: #ifdef CONFIG_POSIX_MADVISE #define QEMU_MADV_DONTNEED POSIX_MADV_DONTNEED #else #define QEMU_MADV_DONTNEED MADV_DONTNEED #endif and keep the qemu_madvise implementation in C. But then there's no need to convert between QEMU_MADV and real MADV bits. > >>> + >>> +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? > > I tested on OpenSolaris 2009.06. Haven't tested on Solaris 10 yet, don't have access to older ones. > We could wrap it in #ifndef CONFIG_POSIX_MADVISE to be fool-safe. *shrug* I was just afraid this was there for a reason. But maybe the author of those lines simply didn't know about posix_madvise. > Thanks for the review, You're welcome :) Alex ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH] Introduce qemu_madvise() 2010-09-11 17:05 [Qemu-devel] [PATCH] Introduce qemu_madvise() Andreas Färber 2010-09-11 21:37 ` Alexander Graf @ 2010-09-12 7:20 ` Blue Swirl 2010-09-12 8:50 ` Andreas Färber 1 sibling, 1 reply; 28+ messages in thread From: Blue Swirl @ 2010-09-12 7:20 UTC (permalink / raw) To: Andreas Färber; +Cc: qemu-devel, Andreas Färber On Sat, Sep 11, 2010 at 5:05 PM, 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. > 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__ For consistency, I'd convert all users. If an OS doesn't support a flag, we can return something like -ENOTSUP which may be checked by the caller if it cares. > > 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> > --- > 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); > } > #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}" Please take a look around configure:2444 how to add new config flags. I'd also add a similar check for madvise() if posix_madvise() check fails. > +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); > + abort(); This should be an assert, it's an internal error. It's also common to both cases. > + } > + return posix_madvise(addr, len, advice); > +#else #elif defined(CONFIG_MADVISE) > + switch (advice) { > + case QEMU_MADV_DONTNEED: > + advice = MADV_DONTNEED; > + break; case QEMU_MADV_DONTFORK: #ifndef MADV_DONTFORK return -ENOTSUP; #else advice = MADV_DONTFORK; break; #endif Same goes for MADV_MERGEABLE. > + default: > + fprintf(stderr, "Advice %d unhandled.\n", advice); > + abort(); > + } > + return madvise(addr, len, advice); #else return -ENOTSUP; ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH] Introduce qemu_madvise() 2010-09-12 7:20 ` [Qemu-devel] " Blue Swirl @ 2010-09-12 8:50 ` Andreas Färber 2010-09-12 9:02 ` Blue Swirl 2010-09-12 12:55 ` [Qemu-devel] [PATCH v3] " Andreas Färber 0 siblings, 2 replies; 28+ messages in thread From: Andreas Färber @ 2010-09-12 8:50 UTC (permalink / raw) To: Blue Swirl; +Cc: Alexander Graf, QEMU Developers, Andreas Färber Am 12.09.2010 um 09:20 schrieb Blue Swirl: > On Sat, Sep 11, 2010 at 5:05 PM, Andreas Färber <andreas.faerber@web.de > > wrote: >> 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__ > > For consistency, I'd convert all users. If an OS doesn't support a > flag, we can return something like -ENOTSUP which may be checked by > the caller if it cares. Will do. >> 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}" > > Please take a look around configure:2444 how to add new config flags. I did. It's just not obvious that they find their way from the Makefile to a C header, unlike autoconf. > I'd also add a similar check for madvise() if posix_madvise() check > fails. Fine with me. >> 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); >> + abort(); > > This should be an assert, it's an internal error. It's also common to > both cases. > >> + } >> + return posix_madvise(addr, len, advice); >> +#else > > #elif defined(CONFIG_MADVISE) > >> + switch (advice) { >> + case QEMU_MADV_DONTNEED: >> + advice = MADV_DONTNEED; >> + break; > > case QEMU_MADV_DONTFORK: > #ifndef MADV_DONTFORK > return -ENOTSUP; > #else > advice = MADV_DONTFORK; > break; > #endif > > Same goes for MADV_MERGEABLE. So you disagree with or didn't yet read Alex' suggestion of eliminating this mapping code in qemu_madvise() altogether? Doing that in a sensible way would allow code to do: #ifdef QEMU_MADV_MERGEABLE ... as before at compile-time. The only qemu_madvise() error condition would then be the #else below. >> + default: >> + fprintf(stderr, "Advice %d unhandled.\n", advice); >> + abort(); >> + } >> + return madvise(addr, len, advice); > > #else > return -ENOTSUP; Thanks, Andreas ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH] Introduce qemu_madvise() 2010-09-12 8:50 ` Andreas Färber @ 2010-09-12 9:02 ` Blue Swirl 2010-09-12 12:55 ` [Qemu-devel] [PATCH v3] " Andreas Färber 1 sibling, 0 replies; 28+ messages in thread From: Blue Swirl @ 2010-09-12 9:02 UTC (permalink / raw) To: Andreas Färber; +Cc: Alexander Graf, QEMU Developers, Andreas Färber On Sun, Sep 12, 2010 at 8:50 AM, Andreas Färber <andreas.faerber@web.de> wrote: > Am 12.09.2010 um 09:20 schrieb Blue Swirl: > >> On Sat, Sep 11, 2010 at 5:05 PM, Andreas Färber <andreas.faerber@web.de> >> wrote: >>> >>> 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__ >> >> For consistency, I'd convert all users. If an OS doesn't support a >> flag, we can return something like -ENOTSUP which may be checked by >> the caller if it cares. > > Will do. > >>> 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}" >> >> Please take a look around configure:2444 how to add new config flags. > > I did. It's just not obvious that they find their way from the Makefile to a > C header, unlike autoconf. > >> I'd also add a similar check for madvise() if posix_madvise() check >> fails. > > Fine with me. > >>> 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); >>> + abort(); >> >> This should be an assert, it's an internal error. It's also common to >> both cases. >> >>> + } >>> + return posix_madvise(addr, len, advice); >>> +#else >> >> #elif defined(CONFIG_MADVISE) >> >>> + switch (advice) { >>> + case QEMU_MADV_DONTNEED: >>> + advice = MADV_DONTNEED; >>> + break; >> >> case QEMU_MADV_DONTFORK: >> #ifndef MADV_DONTFORK >> return -ENOTSUP; >> #else >> advice = MADV_DONTFORK; >> break; >> #endif >> >> Same goes for MADV_MERGEABLE. > > So you disagree with or didn't yet read Alex' suggestion of eliminating this > mapping code in qemu_madvise() altogether? > Doing that in a sensible way would allow code to do: > > #ifdef QEMU_MADV_MERGEABLE > ... > > as before at compile-time. The only qemu_madvise() error condition would > then be the #else below. That's one way too, if nobody cares about madvise() return values for MADV_MERGEABLE. In any case I'd like to eliminate the #ifdefs in other places than osdep.[ch]. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v3] Introduce qemu_madvise() 2010-09-12 8:50 ` Andreas Färber 2010-09-12 9:02 ` Blue Swirl @ 2010-09-12 12:55 ` Andreas Färber 2010-09-12 17:29 ` [Qemu-devel] " Blue Swirl 1 sibling, 1 reply; 28+ messages in thread From: Andreas Färber @ 2010-09-12 12:55 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Alexander Graf 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_*. No functional change except for arch_init.c:ram_load() now potentially falling back to posix_madvise() or no-op in lack of both. 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. 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> --- arch_init.c | 2 +- configure | 33 +++++++++++++++++++++++++++++++++ exec.c | 8 ++++---- hw/virtio-balloon.c | 4 ++-- kvm-all.c | 6 +++--- osdep.c | 15 +++++++++++++++ osdep.h | 25 +++++++++++++++++++++++++ vl.c | 3 --- 8 files changed, 83 insertions(+), 13 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..b1fe3e9 100644 --- a/exec.c +++ b/exec.c @@ -2841,8 +2841,8 @@ 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); +#ifdef QEMU_MADV_MERGEABLE + qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE); #endif } #else @@ -2858,8 +2858,8 @@ 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); +#ifdef QEMU_MADV_MERGEABLE + qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE); #endif } } 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..9393419 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1031,11 +1031,11 @@ 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); +#ifdef QEMU_MADV_DONTFORK + int ret = qemu_madvise(start, size, QEMU_MADV_DONTFORK); if (ret) { - perror("madvice"); + perror("qemu_madvise"); exit(1); } #else diff --git a/osdep.c b/osdep.c index 30426ff..b5e006f 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,17 @@ void qemu_vfree(void *ptr) #endif +int qemu_madvise(void *addr, size_t len, int advice) +{ +#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..552e453 100644 --- a/osdep.h +++ b/osdep.h @@ -90,6 +90,31 @@ void *qemu_memalign(size_t alignment, size_t size); void *qemu_vmalloc(size_t size); void qemu_vfree(void *ptr); +#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 +#endif +#ifdef MADV_MERGEABLE +#define QEMU_MADV_MERGEABLE MADV_MERGEABLE +#endif + +#elif defined(CONFIG_POSIX_MADVISE) + +#define QEMU_MADV_WILLNEED POSIX_MADV_WILLNEED +#define QEMU_MADV_DONTNEED POSIX_MADV_DONTNEED + +#else /* no-op */ + +#define QEMU_MADV_WILLNEED (1 << 0) +#define QEMU_MADV_DONTNEED (2 << 0) + +#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 -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH v3] Introduce qemu_madvise() 2010-09-12 12:55 ` [Qemu-devel] [PATCH v3] " Andreas Färber @ 2010-09-12 17:29 ` Blue Swirl 2010-09-13 12:02 ` Alexander Graf 2010-09-13 21:26 ` [Qemu-devel] [RFC v4] " Andreas Färber 0 siblings, 2 replies; 28+ messages in thread From: Blue Swirl @ 2010-09-12 17:29 UTC (permalink / raw) To: Andreas Färber; +Cc: qemu-devel, Alexander Graf On Sun, Sep 12, 2010 at 12:55 PM, 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_*. No functional change > except for arch_init.c:ram_load() now potentially falling back to posix_madvise() > or no-op in lack of both. > > 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. > > 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> > --- > arch_init.c | 2 +- > configure | 33 +++++++++++++++++++++++++++++++++ > exec.c | 8 ++++---- > hw/virtio-balloon.c | 4 ++-- > kvm-all.c | 6 +++--- > osdep.c | 15 +++++++++++++++ > osdep.h | 25 +++++++++++++++++++++++++ > vl.c | 3 --- > 8 files changed, 83 insertions(+), 13 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..b1fe3e9 100644 > --- a/exec.c > +++ b/exec.c > @@ -2841,8 +2841,8 @@ 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); > +#ifdef QEMU_MADV_MERGEABLE I'd like to avoid these #ifdefs. How about always #defining QEMU_MADV_MERGEABLE? QEMU_MADV_* values could be synthetic and not rely on MADV_*. > + qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE); > #endif > } > #else > @@ -2858,8 +2858,8 @@ 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); > +#ifdef QEMU_MADV_MERGEABLE > + qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE); > #endif > } > } > 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..9393419 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -1031,11 +1031,11 @@ 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); > +#ifdef QEMU_MADV_DONTFORK > + int ret = qemu_madvise(start, size, QEMU_MADV_DONTFORK); > > if (ret) { > - perror("madvice"); > + perror("qemu_madvise"); > exit(1); > } > #else Here the approach could be that if the return value is -ENOTSUP, we don't print the error message. Then the #ifdefs could be dropped. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH v3] Introduce qemu_madvise() 2010-09-12 17:29 ` [Qemu-devel] " Blue Swirl @ 2010-09-13 12:02 ` Alexander Graf 2010-09-13 21:26 ` [Qemu-devel] [RFC v4] " Andreas Färber 1 sibling, 0 replies; 28+ messages in thread From: Alexander Graf @ 2010-09-13 12:02 UTC (permalink / raw) To: Blue Swirl; +Cc: Andreas Färber, qemu-devel Blue Swirl wrote: > On Sun, Sep 12, 2010 at 12:55 PM, 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_*. No functional change >> except for arch_init.c:ram_load() now potentially falling back to posix_madvise() >> or no-op in lack of both. >> >> 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. >> >> 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> >> --- >> arch_init.c | 2 +- >> configure | 33 +++++++++++++++++++++++++++++++++ >> exec.c | 8 ++++---- >> hw/virtio-balloon.c | 4 ++-- >> kvm-all.c | 6 +++--- >> osdep.c | 15 +++++++++++++++ >> osdep.h | 25 +++++++++++++++++++++++++ >> vl.c | 3 --- >> 8 files changed, 83 insertions(+), 13 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..b1fe3e9 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -2841,8 +2841,8 @@ 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); >> +#ifdef QEMU_MADV_MERGEABLE >> > > I'd like to avoid these #ifdefs. How about always #defining > QEMU_MADV_MERGEABLE? QEMU_MADV_* values could be synthetic and not > rely on MADV_*. > Hrm. Something like #ifdef MADV_MEREABLE #define QEMU_MADV_MERGEABLE MADV_MERGEABLE #else #define QEMU_MADV_MERGEABLE QEMU_MADV_INVALID #endif would work, right? Only need to find an unused bit in madv ... Alex ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [RFC v4] Introduce qemu_madvise() 2010-09-12 17:29 ` [Qemu-devel] " Blue Swirl 2010-09-13 12:02 ` Alexander Graf @ 2010-09-13 21:26 ` Andreas Färber 2010-09-14 16:31 ` [Qemu-devel] " Blue Swirl 1 sibling, 1 reply; 28+ messages in thread From: Andreas Färber @ 2010-09-13 21:26 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Alexander Graf 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 will help with future porting though. 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> --- arch_init.c | 2 +- configure | 33 +++++++++++++++++++++++++++++++++ exec.c | 8 ++------ hw/virtio-balloon.c | 4 ++-- kvm-all.c | 15 +++++++-------- osdep.c | 15 +++++++++++++++ osdep.h | 33 +++++++++++++++++++++++++++++++++ vl.c | 3 --- 8 files changed, 93 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..b5e006f 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,17 @@ void qemu_vfree(void *ptr) #endif +int qemu_madvise(void *addr, size_t len, int advice) +{ +#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..2ba7f71 100644 --- a/osdep.h +++ b/osdep.h @@ -90,6 +90,39 @@ void *qemu_memalign(size_t alignment, size_t size); void *qemu_vmalloc(size_t size); void qemu_vfree(void *ptr); +#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 MADV_NORMAL +#endif +#ifdef MADV_MERGEABLE +#define QEMU_MADV_MERGEABLE MADV_MERGEABLE +#else +#define QEMU_MADV_MERGEABLE MADV_NORMAL +#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 + +#else /* no-op */ + +#define QEMU_MADV_WILLNEED 0 +#define QEMU_MADV_DONTNEED 0 +#define QEMU_MADV_DONTFORK 0 +#define QEMU_MADV_MERGEABLE 0 + +#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 -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [RFC v4] Introduce qemu_madvise() 2010-09-13 21:26 ` [Qemu-devel] [RFC v4] " Andreas Färber @ 2010-09-14 16:31 ` Blue Swirl 2010-09-14 16:34 ` Alexander Graf 0 siblings, 1 reply; 28+ messages in thread From: Blue Swirl @ 2010-09-14 16:31 UTC (permalink / raw) To: Andreas Färber; +Cc: qemu-devel, Alexander Graf On Mon, Sep 13, 2010 at 9:26 PM, 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 will help with future porting though. > > 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. Your original switch with synthetic (1<<0, 1<<1 etc) values for QEMU_MADV_* for all hosts (never reusing MADV_* defines). ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [RFC v4] Introduce qemu_madvise() 2010-09-14 16:31 ` [Qemu-devel] " Blue Swirl @ 2010-09-14 16:34 ` Alexander Graf 2010-09-14 17:10 ` Blue Swirl 0 siblings, 1 reply; 28+ messages in thread From: Alexander Graf @ 2010-09-14 16:34 UTC (permalink / raw) To: Blue Swirl; +Cc: Andreas Färber, qemu-devel Am 14.09.2010 um 18:31 schrieb Blue Swirl <blauwirbel@gmail.com>: > On Mon, Sep 13, 2010 at 9:26 PM, 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 will help with future porting though. >> >> 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. > > Your original switch with synthetic (1<<0, 1<<1 etc) values for > QEMU_MADV_* for all hosts (never reusing MADV_* defines). Ugh. Back to square one? How about #define QEMU_MADV_INVALID -1 Then you can define unknown balues against that. Should never occur otherwise. Alex ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [RFC v4] Introduce qemu_madvise() 2010-09-14 16:34 ` Alexander Graf @ 2010-09-14 17:10 ` Blue Swirl 2010-09-14 20:28 ` [Qemu-devel] [PATCH v5] " Andreas Färber 0 siblings, 1 reply; 28+ messages in thread From: Blue Swirl @ 2010-09-14 17:10 UTC (permalink / raw) To: Alexander Graf; +Cc: Andreas Färber, qemu-devel On Tue, Sep 14, 2010 at 4:34 PM, Alexander Graf <agraf@suse.de> wrote: > > Am 14.09.2010 um 18:31 schrieb Blue Swirl <blauwirbel@gmail.com>: > >> On Mon, Sep 13, 2010 at 9:26 PM, 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 will help with future porting though. >>> >>> 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. >> >> Your original switch with synthetic (1<<0, 1<<1 etc) values for >> QEMU_MADV_* for all hosts (never reusing MADV_* defines). > > Ugh. Back to square one? How about > > #define QEMU_MADV_INVALID -1 > > Then you can define unknown balues against that. Should never occur otherwise. Yeah, then the generic code could check for QEMU_MADV_INVALID. That's simpler than the switch. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v5] Introduce qemu_madvise() 2010-09-14 17:10 ` Blue Swirl @ 2010-09-14 20:28 ` Andreas Färber 2010-09-14 20:36 ` [Qemu-devel] " Blue Swirl 0 siblings, 1 reply; 28+ messages in thread From: Andreas Färber @ 2010-09-14 20:28 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Alexander Graf 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 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 <afaerber@opensolaris.org> Cc: Blue Swirl <blauwirbel@gmail.com> Cc: Alexander Graf <agraf@suse.de> --- 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 + +#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 -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH v5] Introduce qemu_madvise() 2010-09-14 20:28 ` [Qemu-devel] [PATCH v5] " Andreas Färber @ 2010-09-14 20:36 ` Blue Swirl 2010-09-14 20:39 ` Andreas Färber 0 siblings, 1 reply; 28+ messages in thread From: Blue Swirl @ 2010-09-14 20:36 UTC (permalink / raw) To: Andreas Färber; +Cc: qemu-devel, Alexander Graf On Tue, Sep 14, 2010 at 8:28 PM, 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 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 <afaerber@opensolaris.org> > Cc: Blue Swirl <blauwirbel@gmail.com> > Cc: Alexander Graf <agraf@suse.de> > --- > 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. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH v5] Introduce qemu_madvise() 2010-09-14 20:36 ` [Qemu-devel] " Blue Swirl @ 2010-09-14 20:39 ` Andreas Färber 2010-09-15 18:09 ` [Qemu-devel] [PATCH v6] " Andreas Färber 0 siblings, 1 reply; 28+ messages in thread From: Andreas Färber @ 2010-09-14 20:39 UTC (permalink / raw) To: Blue Swirl; +Cc: qemu-devel, Alexander Graf Am 14.09.2010 um 22:36 schrieb Blue Swirl: > On Tue, Sep 14, 2010 at 8:28 PM, Andreas Färber <andreas.faerber@web.de > > wrote: >> 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? Oops, missed these two. Will fix tomorrow. Thanks, Andreas ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v6] Introduce qemu_madvise() 2010-09-14 20:39 ` Andreas Färber @ 2010-09-15 18:09 ` Andreas Färber 2010-09-15 19:00 ` [Qemu-devel] " Blue Swirl 0 siblings, 1 reply; 28+ messages in thread From: Andreas Färber @ 2010-09-15 18:09 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Andrea Arcangeli, Alexander Graf 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 will help with future porting though, and it simplifies most call sites. 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 | 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..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 -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH v6] Introduce qemu_madvise() 2010-09-15 18:09 ` [Qemu-devel] [PATCH v6] " Andreas Färber @ 2010-09-15 19:00 ` Blue Swirl 2010-09-15 19:35 ` Andreas Färber 0 siblings, 1 reply; 28+ messages in thread From: Blue Swirl @ 2010-09-15 19:00 UTC (permalink / raw) To: Andreas Färber; +Cc: Andrea Arcangeli, qemu-devel, Alexander Graf On Wed, Sep 15, 2010 at 6:09 PM, 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 will help with future porting though, and > it simplifies most call sites. > > 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 | 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) { Sorry, I should have noticed this earlier, but madvise() actually returns 0 or -1 with error code in errno. Should we try to match that? ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH v6] Introduce qemu_madvise() 2010-09-15 19:00 ` [Qemu-devel] " Blue Swirl @ 2010-09-15 19:35 ` Andreas Färber 2010-09-15 19:50 ` Blue Swirl 0 siblings, 1 reply; 28+ messages in thread From: Andreas Färber @ 2010-09-15 19:35 UTC (permalink / raw) To: Blue Swirl; +Cc: Andrea Arcangeli, qemu-devel, Alexander Graf Am 15.09.2010 um 21:00 schrieb Blue Swirl: > madvise() actually > returns 0 or -1 with error code in errno. Should we try to match that? posix_madvise() doesn't seem to... "otherwise, an error number shall be returned to indicate the error" It documents EINVAL for invalid advice, and so does madvise. Are we using ENOTSUP knowingly? Maybe return -1 and set errno to the posix_madvise() return code? (but is it errno = -retval or errno = retval then?) Andreas ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH v6] Introduce qemu_madvise() 2010-09-15 19:35 ` Andreas Färber @ 2010-09-15 19:50 ` Blue Swirl 2010-09-15 20:07 ` Andreas Färber 2010-09-19 10:11 ` [Qemu-devel] [PATCH v7] " Andreas Färber 0 siblings, 2 replies; 28+ messages in thread From: Blue Swirl @ 2010-09-15 19:50 UTC (permalink / raw) To: Andreas Färber; +Cc: Andrea Arcangeli, qemu-devel, Alexander Graf On Wed, Sep 15, 2010 at 7:35 PM, Andreas Färber <andreas.faerber@web.de> wrote: > Am 15.09.2010 um 21:00 schrieb Blue Swirl: > >> madvise() actually >> returns 0 or -1 with error code in errno. Should we try to match that? > > posix_madvise() doesn't seem to... "otherwise, an error number shall be > returned to indicate the error" If we match posix_madvise(), then the wrapper function probably should become qemu_posix_madvise() and we should prefer posix_madvise() over madvise(). > It documents EINVAL for invalid advice, and so does madvise. Are we using > ENOTSUP knowingly? Let's use EINVAL. > Maybe return -1 and set errno to the posix_madvise() return code? (but is it > errno = -retval or errno = retval then?) An advantage of posix_madvise() semantics is that no mucking with errno is needed. Though existing code should be checked if they rely on madvise() semantics instead. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH v6] Introduce qemu_madvise() 2010-09-15 19:50 ` Blue Swirl @ 2010-09-15 20:07 ` Andreas Färber 2010-09-19 10:11 ` [Qemu-devel] [PATCH v7] " Andreas Färber 1 sibling, 0 replies; 28+ messages in thread From: Andreas Färber @ 2010-09-15 20:07 UTC (permalink / raw) To: Blue Swirl; +Cc: Andrea Arcangeli, qemu-devel, Alexander Graf Am 15.09.2010 um 21:50 schrieb Blue Swirl: > On Wed, Sep 15, 2010 at 7:35 PM, Andreas Färber <andreas.faerber@web.de > > wrote: >> Maybe return -1 and set errno to the posix_madvise() return code? >> (but is it >> errno = -retval or errno = retval then?) > > An advantage of posix_madvise() semantics is that no mucking with > errno is needed. Though existing code should be checked if they rely > on madvise() semantics instead. perror() after the MADV_DONTFORK failure does, right? Andreas ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v7] Introduce qemu_madvise() 2010-09-15 19:50 ` Blue Swirl 2010-09-15 20:07 ` Andreas Färber @ 2010-09-19 10:11 ` Andreas Färber 2010-09-20 20:33 ` [Qemu-devel] " Blue Swirl 1 sibling, 1 reply; 28+ messages in thread From: Andreas Färber @ 2010-09-19 10:11 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Andrea Arcangeli, Alexander Graf 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> /* 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 -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH v7] Introduce qemu_madvise() 2010-09-19 10:11 ` [Qemu-devel] [PATCH v7] " Andreas Färber @ 2010-09-20 20:33 ` Blue Swirl 2010-09-24 18:08 ` Andreas Färber 0 siblings, 1 reply; 28+ messages in thread From: Blue Swirl @ 2010-09-20 20:33 UTC (permalink / raw) To: Andreas Färber; +Cc: Andrea Arcangeli, qemu-devel, Alexander Graf 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. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v7] Introduce qemu_madvise() 2010-09-20 20:33 ` [Qemu-devel] " Blue Swirl @ 2010-09-24 18:08 ` Andreas Färber 2010-09-25 7:49 ` Blue Swirl 0 siblings, 1 reply; 28+ messages in thread From: Andreas Färber @ 2010-09-24 18:08 UTC (permalink / raw) To: Blue Swirl; +Cc: Andrea Arcangeli, qemu-devel, Alexander Graf 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v7] Introduce qemu_madvise() 2010-09-24 18:08 ` Andreas Färber @ 2010-09-25 7:49 ` Blue Swirl 2010-09-25 10:58 ` [Qemu-devel] [PATCH v8] " Andreas Färber 0 siblings, 1 reply; 28+ messages in thread From: Blue Swirl @ 2010-09-25 7:49 UTC (permalink / raw) To: Andreas Färber; +Cc: Andrea Arcangeli, qemu-devel, Alexander Graf 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). ^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v8] Introduce qemu_madvise() 2010-09-25 7:49 ` Blue Swirl @ 2010-09-25 10:58 ` Andreas Färber 2010-09-25 15:17 ` [Qemu-devel] " Blue Swirl 0 siblings, 1 reply; 28+ messages in thread From: Andreas Färber @ 2010-09-25 10:58 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Andrea Arcangeli, Alexander Graf 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(). MinGW implements neither. 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. v7 -> v8: * Some versions of MinGW have no sys/mman.h header. Reported by Blue Swirl. 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 | 22 ++++++++++++++++++++++ osdep.h | 35 +++++++++++++++++++++++++++++++++++ vl.c | 3 --- 8 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_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..2c53015 100644 --- a/osdep.c +++ b/osdep.c @@ -28,6 +28,9 @@ #include <errno.h> #include <unistd.h> #include <fcntl.h> +#if defined(CONFIG_MADVISE) || defined(CONFIG_POSIX_MADVISE) +#include <sys/mman.h> +#endif /* Needed early for CONFIG_BSD etc. */ #include "config-host.h" @@ -35,6 +38,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 +145,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 -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH v8] Introduce qemu_madvise() 2010-09-25 10:58 ` [Qemu-devel] [PATCH v8] " Andreas Färber @ 2010-09-25 15:17 ` Blue Swirl 0 siblings, 0 replies; 28+ messages in thread From: Blue Swirl @ 2010-09-25 15:17 UTC (permalink / raw) To: Andreas Färber; +Cc: Andrea Arcangeli, qemu-devel, 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ä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(). > MinGW implements neither. > > 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. > > v7 -> v8: > * Some versions of MinGW have no sys/mman.h header. Reported by Blue Swirl. > > 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 | 22 ++++++++++++++++++++++ > osdep.h | 35 +++++++++++++++++++++++++++++++++++ > vl.c | 3 --- > 8 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_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..2c53015 100644 > --- a/osdep.c > +++ b/osdep.c > @@ -28,6 +28,9 @@ > #include <errno.h> > #include <unistd.h> > #include <fcntl.h> > +#if defined(CONFIG_MADVISE) || defined(CONFIG_POSIX_MADVISE) > +#include <sys/mman.h> > +#endif > > /* Needed early for CONFIG_BSD etc. */ > #include "config-host.h" > @@ -35,6 +38,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 +145,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 > -- > 1.7.2.2 > > ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2010-09-25 15:18 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-09-11 17:05 [Qemu-devel] [PATCH] Introduce qemu_madvise() Andreas Färber 2010-09-11 21:37 ` Alexander Graf 2010-09-11 22:39 ` Andreas Färber 2010-09-11 22:47 ` Alexander Graf 2010-09-12 7:20 ` [Qemu-devel] " Blue Swirl 2010-09-12 8:50 ` Andreas Färber 2010-09-12 9:02 ` Blue Swirl 2010-09-12 12:55 ` [Qemu-devel] [PATCH v3] " Andreas Färber 2010-09-12 17:29 ` [Qemu-devel] " Blue Swirl 2010-09-13 12:02 ` Alexander Graf 2010-09-13 21:26 ` [Qemu-devel] [RFC v4] " Andreas Färber 2010-09-14 16:31 ` [Qemu-devel] " Blue Swirl 2010-09-14 16:34 ` Alexander Graf 2010-09-14 17:10 ` Blue Swirl 2010-09-14 20:28 ` [Qemu-devel] [PATCH v5] " Andreas Färber 2010-09-14 20:36 ` [Qemu-devel] " Blue Swirl 2010-09-14 20:39 ` Andreas Färber 2010-09-15 18:09 ` [Qemu-devel] [PATCH v6] " Andreas Färber 2010-09-15 19:00 ` [Qemu-devel] " Blue Swirl 2010-09-15 19:35 ` Andreas Färber 2010-09-15 19:50 ` Blue Swirl 2010-09-15 20:07 ` Andreas Färber 2010-09-19 10:11 ` [Qemu-devel] [PATCH v7] " Andreas Färber 2010-09-20 20:33 ` [Qemu-devel] " Blue Swirl 2010-09-24 18:08 ` Andreas Färber 2010-09-25 7:49 ` Blue Swirl 2010-09-25 10:58 ` [Qemu-devel] [PATCH v8] " Andreas Färber 2010-09-25 15:17 ` [Qemu-devel] " Blue Swirl
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.