1. Patchew
  2. QEMU
  3. [PATCH 00/16] UI-related fixes & shareable 2d memory with -display dbus
  4. View patch

[PATCH 09/16] ui/pixman: generalize shared_image_destroy

marcandre.lureau@redhat.com posted 16 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH 09/16] ui/pixman: generalize shared_image_destroy
Posted by marcandre.lureau@redhat.com 1 month, 3 weeks ago 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Learn to free memfd-allocated shared memory.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/qemu-pixman.h |  2 +-
 hw/display/virtio-gpu.c  |  4 ++--
 ui/console.c             |  2 +-
 ui/qemu-pixman.c         | 20 ++++++++++++--------
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
index e3dd72b9e3..a97f56d09a 100644
--- a/include/ui/qemu-pixman.h
+++ b/include/ui/qemu-pixman.h
@@ -97,7 +97,7 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
 
 void qemu_pixman_image_unref(pixman_image_t *image);
 
-void qemu_pixman_win32_image_destroy(pixman_image_t *image, void *data);
+void qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(pixman_image_t, qemu_pixman_image_unref)
 
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 017a0f170c..77f6e76f23 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -298,7 +298,7 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
             bits, c2d.height ? res->hostmem / c2d.height : 0);
 #ifdef WIN32
         if (res->image) {
-            pixman_image_set_destroy_function(res->image, qemu_pixman_win32_image_destroy, res->handle);
+            pixman_image_set_destroy_function(res->image, qemu_pixman_shared_image_destroy, res->handle);
         }
 #endif
     }
@@ -1317,7 +1317,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
             return -EINVAL;
         }
 #ifdef WIN32
-        pixman_image_set_destroy_function(res->image, qemu_pixman_win32_image_destroy, res->handle);
+        pixman_image_set_destroy_function(res->image, qemu_pixman_shared_image_destroy, res->handle);
 #endif
 
         res->addrs = g_new(uint64_t, res->iov_cnt);
diff --git a/ui/console.c b/ui/console.c
index 8f416ff0b9..fdd76c2be4 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -487,7 +487,7 @@ DisplaySurface *qemu_create_displaysurface(int width, int height)
 #ifdef WIN32
     qemu_displaysurface_win32_set_handle(surface, handle, 0);
     pixman_image_set_destroy_function(surface->image,
-                                      qemu_pixman_win32_image_destroy, handle);
+                                      qemu_pixman_shared_image_destroy, handle);
 #endif
     return surface;
 }
diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
index 3870e1a215..43050ab7c5 100644
--- a/ui/qemu-pixman.c
+++ b/ui/qemu-pixman.c
@@ -6,6 +6,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "ui/console.h"
+#include "qemu/memfd.h"
 #include "standard-headers/drm/drm_fourcc.h"
 #include "trace.h"
 
@@ -269,16 +270,19 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
 }
 #endif /* CONFIG_PIXMAN */
 
-#ifdef WIN32
 void
-qemu_pixman_win32_image_destroy(pixman_image_t *image, void *data)
+qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data)
 {
+    void *ptr = pixman_image_get_data(image);
+
+#ifndef WIN32
+    int shmfd = GPOINTER_TO_INT(data);
+    size_t size = pixman_image_get_height(image) * pixman_image_get_stride(image);
+
+    qemu_memfd_free(ptr, size, shmfd);
+#else
     HANDLE handle = data;
 
-    qemu_win32_map_free(
-        pixman_image_get_data(image),
-        handle,
-        &error_warn
-    );
-}
+    qemu_win32_map_free(ptr, handle, &error_warn);
 #endif
+}
-- 
2.45.2.827.g557ae147e6
Re: [PATCH 09/16] ui/pixman: generalize shared_image_destroy
Posted by Akihiko Odaki 1 month, 2 weeks ago 
On 2024/10/03 20:22, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Learn to free memfd-allocated shared memory.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   include/ui/qemu-pixman.h |  2 +-
>   hw/display/virtio-gpu.c  |  4 ++--
>   ui/console.c             |  2 +-
>   ui/qemu-pixman.c         | 20 ++++++++++++--------
>   4 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
> index e3dd72b9e3..a97f56d09a 100644
> --- a/include/ui/qemu-pixman.h
> +++ b/include/ui/qemu-pixman.h
> @@ -97,7 +97,7 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
>   
>   void qemu_pixman_image_unref(pixman_image_t *image);
>   
> -void qemu_pixman_win32_image_destroy(pixman_image_t *image, void *data);
> +void qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data);
>   
>   G_DEFINE_AUTOPTR_CLEANUP_FUNC(pixman_image_t, qemu_pixman_image_unref)
>   
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 017a0f170c..77f6e76f23 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -298,7 +298,7 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
>               bits, c2d.height ? res->hostmem / c2d.height : 0);
>   #ifdef WIN32
>           if (res->image) {
> -            pixman_image_set_destroy_function(res->image, qemu_pixman_win32_image_destroy, res->handle);
> +            pixman_image_set_destroy_function(res->image, qemu_pixman_shared_image_destroy, res->handle);
>           }
>   #endif
>       }
> @@ -1317,7 +1317,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
>               return -EINVAL;
>           }
>   #ifdef WIN32
> -        pixman_image_set_destroy_function(res->image, qemu_pixman_win32_image_destroy, res->handle);
> +        pixman_image_set_destroy_function(res->image, qemu_pixman_shared_image_destroy, res->handle);
>   #endif
>   
>           res->addrs = g_new(uint64_t, res->iov_cnt);
> diff --git a/ui/console.c b/ui/console.c
> index 8f416ff0b9..fdd76c2be4 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -487,7 +487,7 @@ DisplaySurface *qemu_create_displaysurface(int width, int height)
>   #ifdef WIN32
>       qemu_displaysurface_win32_set_handle(surface, handle, 0);
>       pixman_image_set_destroy_function(surface->image,
> -                                      qemu_pixman_win32_image_destroy, handle);
> +                                      qemu_pixman_shared_image_destroy, handle);
>   #endif
>       return surface;
>   }
> diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
> index 3870e1a215..43050ab7c5 100644
> --- a/ui/qemu-pixman.c
> +++ b/ui/qemu-pixman.c
> @@ -6,6 +6,7 @@
>   #include "qemu/osdep.h"
>   #include "qapi/error.h"
>   #include "ui/console.h"
> +#include "qemu/memfd.h"
>   #include "standard-headers/drm/drm_fourcc.h"
>   #include "trace.h"
>   
> @@ -269,16 +270,19 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
>   }
>   #endif /* CONFIG_PIXMAN */
>   
> -#ifdef WIN32
>   void
> -qemu_pixman_win32_image_destroy(pixman_image_t *image, void *data)
> +qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data)
>   {
> +    void *ptr = pixman_image_get_data(image);
> +
> +#ifndef WIN32

I think it is better to have #ifdef instead of #ifndef. It is a common 
pattern to have #ifdef and #elif defined() for the platform-specific 
code and follow them with #else that implements the generic code. This 
allows supporting multiple platform conditions.

> +    int shmfd = GPOINTER_TO_INT(data);
> +    size_t size = pixman_image_get_height(image) * pixman_image_get_stride(image);
> +
> +    qemu_memfd_free(ptr, size, shmfd);
> +#else
>       HANDLE handle = data;
>   
> -    qemu_win32_map_free(
> -        pixman_image_get_data(image),
> -        handle,
> -        &error_warn
> -    );
> -}
> +    qemu_win32_map_free(ptr, handle, &error_warn);
>   #endif
> +}
Re: [PATCH 09/16] ui/pixman: generalize shared_image_destroy
Posted by Marc-André Lureau 1 month, 2 weeks ago 
Hi

On Sat, Oct 5, 2024 at 12:50 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> On 2024/10/03 20:22, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Learn to free memfd-allocated shared memory.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   include/ui/qemu-pixman.h |  2 +-
> >   hw/display/virtio-gpu.c  |  4 ++--
> >   ui/console.c             |  2 +-
> >   ui/qemu-pixman.c         | 20 ++++++++++++--------
> >   4 files changed, 16 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
> > index e3dd72b9e3..a97f56d09a 100644
> > --- a/include/ui/qemu-pixman.h
> > +++ b/include/ui/qemu-pixman.h
> > @@ -97,7 +97,7 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
> >
> >   void qemu_pixman_image_unref(pixman_image_t *image);
> >
> > -void qemu_pixman_win32_image_destroy(pixman_image_t *image, void *data);
> > +void qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data);
> >
> >   G_DEFINE_AUTOPTR_CLEANUP_FUNC(pixman_image_t, qemu_pixman_image_unref)
> >
> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > index 017a0f170c..77f6e76f23 100644
> > --- a/hw/display/virtio-gpu.c
> > +++ b/hw/display/virtio-gpu.c
> > @@ -298,7 +298,7 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
> >               bits, c2d.height ? res->hostmem / c2d.height : 0);
> >   #ifdef WIN32
> >           if (res->image) {
> > -            pixman_image_set_destroy_function(res->image, qemu_pixman_win32_image_destroy, res->handle);
> > +            pixman_image_set_destroy_function(res->image, qemu_pixman_shared_image_destroy, res->handle);
> >           }
> >   #endif
> >       }
> > @@ -1317,7 +1317,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
> >               return -EINVAL;
> >           }
> >   #ifdef WIN32
> > -        pixman_image_set_destroy_function(res->image, qemu_pixman_win32_image_destroy, res->handle);
> > +        pixman_image_set_destroy_function(res->image, qemu_pixman_shared_image_destroy, res->handle);
> >   #endif
> >
> >           res->addrs = g_new(uint64_t, res->iov_cnt);
> > diff --git a/ui/console.c b/ui/console.c
> > index 8f416ff0b9..fdd76c2be4 100644
> > --- a/ui/console.c
> > +++ b/ui/console.c
> > @@ -487,7 +487,7 @@ DisplaySurface *qemu_create_displaysurface(int width, int height)
> >   #ifdef WIN32
> >       qemu_displaysurface_win32_set_handle(surface, handle, 0);
> >       pixman_image_set_destroy_function(surface->image,
> > -                                      qemu_pixman_win32_image_destroy, handle);
> > +                                      qemu_pixman_shared_image_destroy, handle);
> >   #endif
> >       return surface;
> >   }
> > diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
> > index 3870e1a215..43050ab7c5 100644
> > --- a/ui/qemu-pixman.c
> > +++ b/ui/qemu-pixman.c
> > @@ -6,6 +6,7 @@
> >   #include "qemu/osdep.h"
> >   #include "qapi/error.h"
> >   #include "ui/console.h"
> > +#include "qemu/memfd.h"
> >   #include "standard-headers/drm/drm_fourcc.h"
> >   #include "trace.h"
> >
> > @@ -269,16 +270,19 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
> >   }
> >   #endif /* CONFIG_PIXMAN */
> >
> > -#ifdef WIN32
> >   void
> > -qemu_pixman_win32_image_destroy(pixman_image_t *image, void *data)
> > +qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data)
> >   {
> > +    void *ptr = pixman_image_get_data(image);
> > +
> > +#ifndef WIN32
>
> I think it is better to have #ifdef instead of #ifndef. It is a common
> pattern to have #ifdef and #elif defined() for the platform-specific
> code and follow them with #else that implements the generic code. This
> allows supporting multiple platform conditions.

indeed! thanks

>
> > +    int shmfd = GPOINTER_TO_INT(data);
> > +    size_t size = pixman_image_get_height(image) * pixman_image_get_stride(image);
> > +
> > +    qemu_memfd_free(ptr, size, shmfd);
> > +#else
> >       HANDLE handle = data;
> >
> > -    qemu_win32_map_free(
> > -        pixman_image_get_data(image),
> > -        handle,
> > -        &error_warn
> > -    );
> > -}
> > +    qemu_win32_map_free(ptr, handle, &error_warn);
> >   #endif
> > +}
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.