[PATCH v4 5/7] ui/console-gl: Add a helper to create a texture with linear memory layout

[Michael Scherle <michael.scherle@rz.uni-freiburg.de>]

Great to see this patch making progress.

I've tested it extensively, and unfortunately, I’ve noticed a memory 
leak in surface_gl_create_texture_from_fd(). The memory leak is hard to 
see since the memory is owned by the gpu driver.
On Intel hardware, it's possible to observe the leak using:

cat /sys/module/i915/refcnt
or
xpu-smi ps

In on of my use case—which involves frequent scanout disable/enable 
cycles—the leak is quite apparent. However, in more typical scenarios, 
it might be difficult to catch.

The issue stems from the mem_obj not being deleted after use. I’ve put 
together a minimal modification to address it:



On 15.05.25 04:45, Vivek Kasireddy wrote:
> There are cases where we do not want the memory layout of a texture to
> be tiled as the component processing the texture would not know how to
> de-tile either via software or hardware. Therefore, ensuring that the
> memory backing the texture has a linear layout is absolutely necessary
> in these situations.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Cc: Frediano Ziglio <freddy77@gmail.com>
> Cc: Dongwon Kim <dongwon.kim@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   include/ui/console.h |  2 ++
>   ui/console-gl.c      | 32 ++++++++++++++++++++++++++++++++
>   2 files changed, 34 insertions(+)
> 
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 46b3128185..5cfa6ae215 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -422,6 +422,8 @@ bool console_gl_check_format(DisplayChangeListener *dcl,
>                                pixman_format_code_t format);
>   void surface_gl_create_texture(QemuGLShader *gls,
>                                  DisplaySurface *surface);
> +bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
> +                                       int fd, GLuint *texture);
>   void surface_gl_update_texture(QemuGLShader *gls,
>                                  DisplaySurface *surface,
>                                  int x, int y, int w, int h);
> diff --git a/ui/console-gl.c b/ui/console-gl.c
> index 103b954017..97f7989651 100644
> --- a/ui/console-gl.c
> +++ b/ui/console-gl.c
> @@ -25,6 +25,7 @@
>    * THE SOFTWARE.
>    */
>   #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
>   #include "ui/console.h"
>   #include "ui/shader.h"
>   
> @@ -96,6 +97,37 @@ void surface_gl_create_texture(QemuGLShader *gls,
>       glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
>   }
>   
> +bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
> +                                       int fd, GLuint *texture)
> +{
> +    unsigned long size = surface_stride(surface) * surface_height(surface);
> +    GLenum err = glGetError();
> +    GLuint mem_obj;

+    GLuint mem_obj = 0;

> +
> +    if (!epoxy_has_gl_extension("GL_EXT_memory_object") ||
> +        !epoxy_has_gl_extension("GL_EXT_memory_object_fd")) {
> +        return false;
> +    }
> +
> +#ifdef GL_EXT_memory_object_fd
> +    glCreateMemoryObjectsEXT(1, &mem_obj);
> +    glImportMemoryFdEXT(mem_obj, size, GL_HANDLE_TYPE_OPAQUE_FD_EXT, fd);
> +
> +    err = glGetError();
> +    if (err != GL_NO_ERROR) {

+          if (mem_obj) {
+              glDeleteMemoryObjectsEXT(1, &mem_obj);
+          }

> +        error_report("spice: cannot import memory object from fd");
> +        return false;
> +    }
> +
> +    glGenTextures(1, texture);
> +    glBindTexture(GL_TEXTURE_2D, *texture);
> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_TILING_EXT, GL_LINEAR_TILING_EXT);
> +    glTexStorageMem2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8, surface_width(surface),
> +                         surface_height(surface), mem_obj, 0);
> +#endif

+    glDeleteMemoryObjectsEXT(1, &mem_obj);

> +    return *texture > 0 && glGetError() == GL_NO_ERROR;
> +}
> +
>   void surface_gl_update_texture(QemuGLShader *gls,
>                                  DisplaySurface *surface,
>                                  int x, int y, int w, int h)



That said, my OpenGL knowledge is somewhat limited, and the 
documentation wasn’t entirely clear to me on whether deleting the memory 
object while the texture is still being used, is always safe. Based on a 
quick look at the iris and llvmpipe implementations, it appears to be 
acceptable.

If that's not the case, an alternative fix could follow this approach 
instead: 
https://gitlab.uni-freiburg.de/opensourcevdi/qemu/-/commit/4ca806871c141089be16af25c1820d3e04f3e27d

Greetings Michael