[PULL 04/47] util/oslib-win32: Use _aligned_malloc for qemu_try_memalign

Richard Henderson posted 47 patches 3 years, 5 months ago
Maintainers: Jiaxun Yang <jiaxun.yang@flygoat.com>, Thomas Huth <thuth@redhat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Alistair Francis <Alistair.Francis@wdc.com>, Cornelia Huck <cohuck@redhat.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Aurelien Jarno <aurelien@aurel32.net>, Palmer Dabbelt <palmer@dabbelt.com>, Laurent Vivier <laurent@vivier.eu>, Richard Henderson <richard.henderson@linaro.org>, Huacai Chen <chenhuacai@kernel.org>, Paolo Bonzini <pbonzini@redhat.com>, Stefan Weil <sw@weilnetz.de>, Andrzej Zaborowski <balrogg@gmail.com>
There is a newer version of this series
[PULL 04/47] util/oslib-win32: Use _aligned_malloc for qemu_try_memalign
Posted by Richard Henderson 3 years, 5 months ago
We do not need or want to be allocating page sized quanta.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
Message-Id: <20201018164836.1149452-1-richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 util/oslib-win32.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 01787df74c..8adc651259 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -39,6 +39,7 @@
 #include "trace.h"
 #include "qemu/sockets.h"
 #include "qemu/cutils.h"
+#include <malloc.h>
 
 /* this must come after including "trace.h" */
 #include <shlobj.h>
@@ -56,10 +57,8 @@ void *qemu_try_memalign(size_t alignment, size_t size)
 {
     void *ptr;
 
-    if (!size) {
-        abort();
-    }
-    ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
+    g_assert(size != 0);
+    ptr = _aligned_malloc(alignment, size);
     trace_qemu_memalign(alignment, size, ptr);
     return ptr;
 }
@@ -93,9 +92,7 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t *align, bool shared)
 void qemu_vfree(void *ptr)
 {
     trace_qemu_vfree(ptr);
-    if (ptr) {
-        VirtualFree(ptr, 0, MEM_RELEASE);
-    }
+    _aligned_free(ptr);
 }
 
 void qemu_anon_ram_free(void *ptr, size_t size)
-- 
2.25.1


Re: [PULL 04/47] util/oslib-win32: Use _aligned_malloc for qemu_try_memalign
Posted by Volker Rümelin 3 years, 5 months ago
> We do not need or want to be allocating page sized quanta.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Stefan Weil <sw@weilnetz.de>
> Message-Id: <20201018164836.1149452-1-richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  util/oslib-win32.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index 01787df74c..8adc651259 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -39,6 +39,7 @@
>  #include "trace.h"
>  #include "qemu/sockets.h"
>  #include "qemu/cutils.h"
> +#include <malloc.h>
>  
>  /* this must come after including "trace.h" */
>  #include <shlobj.h>
> @@ -56,10 +57,8 @@ void *qemu_try_memalign(size_t alignment, size_t size)
>  {
>      void *ptr;
>  
> -    if (!size) {
> -        abort();
> -    }
> -    ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
> +    g_assert(size != 0);
> +    ptr = _aligned_malloc(alignment, size);

Hi Richard,

this doesn't work really well. The _aligned_malloc parameters are swapped. ptr = _aligned_malloc(size, alignment) is correct.

With best regards,
Volker

>      trace_qemu_memalign(alignment, size, ptr);
>      return ptr;
>  }
> @@ -93,9 +92,7 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t *align, bool shared)
>  void qemu_vfree(void *ptr)
>  {
>      trace_qemu_vfree(ptr);
> -    if (ptr) {
> -        VirtualFree(ptr, 0, MEM_RELEASE);
> -    }
> +    _aligned_free(ptr);
>  }
>  
>  void qemu_anon_ram_free(void *ptr, size_t size)


Re: [PULL 04/47] util/oslib-win32: Use _aligned_malloc for qemu_try_memalign
Posted by 罗勇刚 (Yonggang Luo) 3 years, 5 months ago
On Sun, Jan 10, 2021 at 3:19 PM Volker Rümelin <vr_qemu@t-online.de> wrote:
>
> > We do not need or want to be allocating page sized quanta.
> >
> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > Reviewed-by: Stefan Weil <sw@weilnetz.de>
> > Message-Id: <20201018164836.1149452-1-richard.henderson@linaro.org>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >  util/oslib-win32.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> > index 01787df74c..8adc651259 100644
> > --- a/util/oslib-win32.c
> > +++ b/util/oslib-win32.c
> > @@ -39,6 +39,7 @@
> >  #include "trace.h"
> >  #include "qemu/sockets.h"
> >  #include "qemu/cutils.h"
> > +#include <malloc.h>
> >
> >  /* this must come after including "trace.h" */
> >  #include <shlobj.h>
> > @@ -56,10 +57,8 @@ void *qemu_try_memalign(size_t alignment, size_t
size)
> >  {
> >      void *ptr;
> >
> > -    if (!size) {
> > -        abort();
> > -    }
> > -    ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
> > +    g_assert(size != 0);
> > +    ptr = _aligned_malloc(alignment, size);
>
> Hi Richard,
>
> this doesn't work really well. The _aligned_malloc parameters are
swapped. ptr = _aligned_malloc(size, alignment) is correct.
>
> With best regards,
> Volker
>
> >      trace_qemu_memalign(alignment, size, ptr);
> >      return ptr;
> >  }
> > @@ -93,9 +92,7 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t
*align, bool shared)
> >  void qemu_vfree(void *ptr)
> >  {
> >      trace_qemu_vfree(ptr);
> > -    if (ptr) {
> > -        VirtualFree(ptr, 0, MEM_RELEASE);
> > -    }
> > +    _aligned_free(ptr);
> >  }
> >
> >  void qemu_anon_ram_free(void *ptr, size_t size)
>
>

Dos this the cause of this failure?
https://cirrus-ci.com/task/6055645751279616?command=test#L593


MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
G_TEST_SRCDIR=C:/Users/ContainerAdministrator/AppData/Local/Temp/cirrus-ci-build/tests
G_TEST_BUILDDIR=C:/Users/ContainerAdministrator/AppData/Local/Temp/cirrus-ci-build/build/tests
tests/test-qht.exe --tap -k
ERROR test-qht - too few tests run (expected 2, got 0)
make: *** [Makefile.mtest:256: run-test-30] Error 1
--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo


--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo