| Message ID | 20200916004638.2444147-2-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | qom: Allow object to be aligned | expand |
Am 16.09.20 um 02:46 schrieb Richard Henderson: > We do not need or want to be allocating page sized quanta. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > Cc: Stefan Weil <sw@weilnetz.de> > --- > util/oslib-win32.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index c654dafd93..8d838bf342 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -56,10 +56,8 @@ void *qemu_try_memalign(size_t alignment, size_t size) > { > void *ptr; > > - if (!size) { > - abort(); > - } > - ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE); > + assert(size != 0); > + ptr = _aligned_malloc(alignment, size); > trace_qemu_memalign(alignment, size, ptr); > return ptr; > } > @@ -93,9 +91,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) According to the documentation, malloc.h should be included for _aligned_malloc. See https://docs.microsoft.com/de-de/cpp/c-runtime-library/reference/aligned-malloc?view=vs-2019 I'd also use g_assert instead of assert to make sure that it is not removed by NDEBUG. Thanks, Stefan
On Tue, Sep 15, 2020 at 05:46:33PM -0700, Richard Henderson wrote: > We do not need or want to be allocating page sized quanta. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Would it be safe to keep patches 2-6 applied in case this one gets dropped or reverted for any reason? I consider patches 2-6 bug fixes (that I'd like to merge soon), and this one as an optimization (that could be merged later). > --- > Cc: Stefan Weil <sw@weilnetz.de> > --- > util/oslib-win32.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index c654dafd93..8d838bf342 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -56,10 +56,8 @@ void *qemu_try_memalign(size_t alignment, size_t size) > { > void *ptr; > > - if (!size) { > - abort(); > - } > - ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE); > + assert(size != 0); > + ptr = _aligned_malloc(alignment, size); > trace_qemu_memalign(alignment, size, ptr); > return ptr; > } > @@ -93,9 +91,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 > >
On 9/17/20 11:43 AM, Eduardo Habkost wrote: > On Tue, Sep 15, 2020 at 05:46:33PM -0700, Richard Henderson wrote: >> We do not need or want to be allocating page sized quanta. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > > Would it be safe to keep patches 2-6 applied in case this one > gets dropped or reverted for any reason? Yes. Only objects using QEMU_ALIGNED() should have an __alignof__ larger than qemu_max_align_t. Therefore, I believe that the CPUArchState objects to be the only ones that will go through qemu_memalign. So we won't end up with all QOM objects being page allocated on Windows. r~
diff --git a/util/oslib-win32.c b/util/oslib-win32.c index c654dafd93..8d838bf342 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -56,10 +56,8 @@ void *qemu_try_memalign(size_t alignment, size_t size) { void *ptr; - if (!size) { - abort(); - } - ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE); + assert(size != 0); + ptr = _aligned_malloc(alignment, size); trace_qemu_memalign(alignment, size, ptr); return ptr; } @@ -93,9 +91,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)
We do not need or want to be allocating page sized quanta. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- Cc: Stefan Weil <sw@weilnetz.de> --- util/oslib-win32.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)