Patchwork Prefer posix_madvise() over madvise()

login
register
mail settings
Submitter Andreas Färber
Date Sept. 11, 2010, 11:51 a.m.
Message ID <1284205917-8124-1-git-send-email-andreas.faerber@web.de>
Download mbox | patch
Permalink /patch/64510/
State New
Headers show

Comments

Andreas Färber - Sept. 11, 2010, 11:51 a.m.
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.

Where functionality equivalent to that of madvise() is provided,
use the POSIX function posix_madvise().
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__

Signed-off-by: Andreas Färber <afaerber@opensolaris.org>
Cc: Blue Swirl <blauwirbel@gmail.com>
---
 arch_init.c |    2 +-
 vl.c        |    3 ---
 2 files changed, 1 insertions(+), 4 deletions(-)
Blue Swirl - Sept. 11, 2010, 12:11 p.m.
On Sat, Sep 11, 2010 at 11:51 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.
> 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 doesn't implement posix_madvise(). How about adding probing to
configure and wrapping the stuff found as qemu_madvise()?

>
> Where functionality equivalent to that of madvise() is provided,
> use the POSIX function posix_madvise().
> 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__
>
> Signed-off-by: Andreas Färber <afaerber@opensolaris.org>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> ---
>  arch_init.c |    2 +-
>  vl.c        |    3 ---
>  2 files changed, 1 insertions(+), 4 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index e468c0c..fa39557 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);
> +                posix_madvise(host, TARGET_PAGE_SIZE, POSIX_MADV_DONTNEED);
>             }
>  #endif
>         } else if (flags & RAM_SAVE_FLAG_PAGE) {
> 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
>
>
Andreas Färber - Sept. 11, 2010, 5:13 p.m.
Am 11.09.2010 um 14:11 schrieb Blue Swirl:

> On Sat, Sep 11, 2010 at 11:51 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.
>> 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 doesn't implement posix_madvise(). How about adding probing to
> configure and wrapping the stuff found as qemu_madvise()?

Done in "Introduce qemu_madvise()". Should've been "[PATCH v2]".

Andreas

Patch

diff --git a/arch_init.c b/arch_init.c
index e468c0c..fa39557 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);
+                posix_madvise(host, TARGET_PAGE_SIZE, POSIX_MADV_DONTNEED);
             }
 #endif
         } else if (flags & RAM_SAVE_FLAG_PAGE) {
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