* [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.