Hi Vivek,
On 27.05.25 06:20, Kasireddy, Vivek wrote:
Hi Michael,
Subject: Re: [PATCH v4 5/7] ui/console-gl: Add a helper to create a texture
with linear memory layout
Hi all,
I just noticed that Dmitry Osipenko had already pointed out a similar issue
earlier-so my message was somewhat redundant. Apologies for the
duplication.
Yeah, looks like you and Dmitry identified the leak independently, almost at the
same time.
Also, I made a small mistake in the patch I proposed:
The call to glDeleteMemoryObjectsEXT(1, &mem_obj); should be placed
above the #endif, not after it. Sorry about that oversight!
Your patch from Open Source VDI repo seems like a better solution, so, I'll add
it
to this series and send it out for review in v5. Please add description/commit
message
and your signed-off-by line to it.
Thanks! I’ve added it and made a few additional adjustments here:
https://gitlab.uni-freiburg.de/opensourcevdi/qemu/-/commit/859d500761b35cc785fcadd9d554a78712309e88
Feel free to merge it into your patches if you want.
Best regards,
Michael
Thanks,
Vivek
Best regards,
Michael
On 26.05.25 15:06, Michael Scherle wrote:
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 <kra...@redhat.com>
Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
Cc: Dmitry Osipenko <dmitry.osipe...@collabora.com>
Cc: Frediano Ziglio <fredd...@gmail.com>
Cc: Dongwon Kim <dongwon....@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasire...@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
