From: Marc-André Lureau <marcandre.lureau@redhat.com>
Filtering pending messages when a new scanout is given shouldn't discard
pending cursor changes, for example.
Since filtering happens in a different thread, use atomic set/get.
Fixes: fa88b85dea ("ui/dbus: filter out pending messages when scanout")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
ui/dbus-listener.c | 44 ++++++++++++++++++++++++++++++++------------
1 file changed, 32 insertions(+), 12 deletions(-)
diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
index 434bd608f2..a70cad3a90 100644
--- a/ui/dbus-listener.c
+++ b/ui/dbus-listener.c
@@ -26,6 +26,7 @@
#include "qapi/error.h"
#include "sysemu/sysemu.h"
#include "dbus.h"
+#include "glib.h"
#ifdef G_OS_UNIX
#include <gio/gunixfdlist.h>
#endif
@@ -85,7 +86,7 @@ struct _DBusDisplayListener {
#endif
guint dbus_filter;
- guint32 out_serial_to_discard;
+ guint32 display_serial_to_discard;
};
G_DEFINE_TYPE(DBusDisplayListener, dbus_display_listener, G_TYPE_OBJECT)
@@ -93,10 +94,12 @@ G_DEFINE_TYPE(DBusDisplayListener, dbus_display_listener, G_TYPE_OBJECT)
static void dbus_gfx_update(DisplayChangeListener *dcl,
int x, int y, int w, int h);
-static void ddl_discard_pending_messages(DBusDisplayListener *ddl)
+static void ddl_discard_display_messages(DBusDisplayListener *ddl)
{
- ddl->out_serial_to_discard = g_dbus_connection_get_last_serial(
+ guint32 serial = g_dbus_connection_get_last_serial(
g_dbus_proxy_get_connection(G_DBUS_PROXY(ddl->proxy)));
+
+ g_atomic_int_set(&ddl->display_serial_to_discard, serial);
}
#ifdef CONFIG_OPENGL
@@ -290,7 +293,7 @@ static void dbus_scanout_dmabuf(DisplayChangeListener *dcl,
return;
}
- ddl_discard_pending_messages(ddl);
+ ddl_discard_display_messages(ddl);
width = qemu_dmabuf_get_width(dmabuf);
height = qemu_dmabuf_get_height(dmabuf);
@@ -338,7 +341,7 @@ static bool dbus_scanout_map(DBusDisplayListener *ddl)
return false;
}
- ddl_discard_pending_messages(ddl);
+ ddl_discard_display_messages(ddl);
if (!qemu_dbus_display1_listener_win32_map_call_scanout_map_sync(
ddl->map_proxy,
@@ -401,7 +404,7 @@ dbus_scanout_share_d3d_texture(
return false;
}
- ddl_discard_pending_messages(ddl);
+ ddl_discard_display_messages(ddl);
qemu_dbus_display1_listener_win32_d3d11_call_scanout_texture2d(
ddl->d3d11_proxy,
@@ -659,7 +662,7 @@ static void ddl_scanout(DBusDisplayListener *ddl)
surface_stride(ddl->ds) * surface_height(ddl->ds), TRUE,
(GDestroyNotify)pixman_image_unref, pixman_image_ref(ddl->ds->image));
- ddl_discard_pending_messages(ddl);
+ ddl_discard_display_messages(ddl);
qemu_dbus_display1_listener_call_scanout(
ddl->proxy, surface_width(ddl->ds), surface_height(ddl->ds),
@@ -992,17 +995,34 @@ dbus_filter(GDBusConnection *connection,
gpointer user_data)
{
DBusDisplayListener *ddl = DBUS_DISPLAY_LISTENER(user_data);
- guint32 serial;
+ const gchar *member = NULL;
+ guint32 serial, discard_serial;
if (incoming) {
return message;
}
serial = g_dbus_message_get_serial(message);
- if (serial <= ddl->out_serial_to_discard) {
- trace_dbus_filter(serial, ddl->out_serial_to_discard);
- g_object_unref(message);
- return NULL;
+
+ discard_serial = g_atomic_int_get(&ddl->display_serial_to_discard);
+ if (serial <= discard_serial) {
+ member = g_dbus_message_get_member(message);
+ if (g_strv_contains((const gchar *[]) {
+ "Scanout",
+ "Update",
+#ifdef CONFIG_GBM
+ "ScanoutDMABUF",
+ "UpdateDMABUF",
+#endif
+ "ScanoutMap",
+ "UpdateMap",
+ "Disable",
+ NULL,
+ }, member)) {
+ trace_dbus_filter(serial, discard_serial);
+ g_object_unref(message);
+ return NULL;
+ }
}
return message;
--
2.45.2.827.g557ae147e6
On 2024/10/03 20:22, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Filtering pending messages when a new scanout is given shouldn't discard
> pending cursor changes, for example.
>
> Since filtering happens in a different thread, use atomic set/get.
>
> Fixes: fa88b85dea ("ui/dbus: filter out pending messages when scanout")
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> ui/dbus-listener.c | 44 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
> index 434bd608f2..a70cad3a90 100644
> --- a/ui/dbus-listener.c
> +++ b/ui/dbus-listener.c
> @@ -26,6 +26,7 @@
> #include "qapi/error.h"
> #include "sysemu/sysemu.h"
> #include "dbus.h"
> +#include "glib.h"
> #ifdef G_OS_UNIX
> #include <gio/gunixfdlist.h>
> #endif
> @@ -85,7 +86,7 @@ struct _DBusDisplayListener {
> #endif
>
> guint dbus_filter;
> - guint32 out_serial_to_discard;
> + guint32 display_serial_to_discard;
> };
>
> G_DEFINE_TYPE(DBusDisplayListener, dbus_display_listener, G_TYPE_OBJECT)
> @@ -93,10 +94,12 @@ G_DEFINE_TYPE(DBusDisplayListener, dbus_display_listener, G_TYPE_OBJECT)
> static void dbus_gfx_update(DisplayChangeListener *dcl,
> int x, int y, int w, int h);
>
> -static void ddl_discard_pending_messages(DBusDisplayListener *ddl)
> +static void ddl_discard_display_messages(DBusDisplayListener *ddl)
> {
> - ddl->out_serial_to_discard = g_dbus_connection_get_last_serial(
> + guint32 serial = g_dbus_connection_get_last_serial(
> g_dbus_proxy_get_connection(G_DBUS_PROXY(ddl->proxy)));
> +
> + g_atomic_int_set(&ddl->display_serial_to_discard, serial);
> }
>
> #ifdef CONFIG_OPENGL
> @@ -290,7 +293,7 @@ static void dbus_scanout_dmabuf(DisplayChangeListener *dcl,
> return;
> }
>
> - ddl_discard_pending_messages(ddl);
> + ddl_discard_display_messages(ddl);
>
> width = qemu_dmabuf_get_width(dmabuf);
> height = qemu_dmabuf_get_height(dmabuf);
> @@ -338,7 +341,7 @@ static bool dbus_scanout_map(DBusDisplayListener *ddl)
> return false;
> }
>
> - ddl_discard_pending_messages(ddl);
> + ddl_discard_display_messages(ddl);
>
> if (!qemu_dbus_display1_listener_win32_map_call_scanout_map_sync(
> ddl->map_proxy,
> @@ -401,7 +404,7 @@ dbus_scanout_share_d3d_texture(
> return false;
> }
>
> - ddl_discard_pending_messages(ddl);
> + ddl_discard_display_messages(ddl);
>
> qemu_dbus_display1_listener_win32_d3d11_call_scanout_texture2d(
> ddl->d3d11_proxy,
> @@ -659,7 +662,7 @@ static void ddl_scanout(DBusDisplayListener *ddl)
> surface_stride(ddl->ds) * surface_height(ddl->ds), TRUE,
> (GDestroyNotify)pixman_image_unref, pixman_image_ref(ddl->ds->image));
>
> - ddl_discard_pending_messages(ddl);
> + ddl_discard_display_messages(ddl);
>
> qemu_dbus_display1_listener_call_scanout(
> ddl->proxy, surface_width(ddl->ds), surface_height(ddl->ds),
> @@ -992,17 +995,34 @@ dbus_filter(GDBusConnection *connection,
> gpointer user_data)
> {
> DBusDisplayListener *ddl = DBUS_DISPLAY_LISTENER(user_data);
> - guint32 serial;
> + const gchar *member = NULL;
I suggest removing the initialization will NULL as it may suppress
uninitialized variable warning.
> + guint32 serial, discard_serial;
>
> if (incoming) {
> return message;
> }
>
> serial = g_dbus_message_get_serial(message);
> - if (serial <= ddl->out_serial_to_discard) {
> - trace_dbus_filter(serial, ddl->out_serial_to_discard);
> - g_object_unref(message);
> - return NULL;
> +
> + discard_serial = g_atomic_int_get(&ddl->display_serial_to_discard);
> + if (serial <= discard_serial) {
> + member = g_dbus_message_get_member(message);
> + if (g_strv_contains((const gchar *[]) {
> + "Scanout",
> + "Update",
> +#ifdef CONFIG_GBM
> + "ScanoutDMABUF",
> + "UpdateDMABUF",
> +#endif
> + "ScanoutMap",
> + "UpdateMap",
> + "Disable",
> + NULL,
> + }, member)) {
I prefer to have a static variable for the array. It makes the object
code simpler and also avoids to have a multi-line condition in the if
statement.
> + trace_dbus_filter(serial, discard_serial);
> + g_object_unref(message);
> + return NULL;
> + }
> }
>
> return message;
Hi Akihiko
On Sat, Oct 5, 2024 at 12:44 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>
> >
> > Filtering pending messages when a new scanout is given shouldn't discard
> > pending cursor changes, for example.
> >
> > Since filtering happens in a different thread, use atomic set/get.
> >
> > Fixes: fa88b85dea ("ui/dbus: filter out pending messages when scanout")
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > ui/dbus-listener.c | 44 ++++++++++++++++++++++++++++++++------------
> > 1 file changed, 32 insertions(+), 12 deletions(-)
> >
> > diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
> > index 434bd608f2..a70cad3a90 100644
> > --- a/ui/dbus-listener.c
> > +++ b/ui/dbus-listener.c
> > @@ -26,6 +26,7 @@
> > #include "qapi/error.h"
> > #include "sysemu/sysemu.h"
> > #include "dbus.h"
> > +#include "glib.h"
> > #ifdef G_OS_UNIX
> > #include <gio/gunixfdlist.h>
> > #endif
> > @@ -85,7 +86,7 @@ struct _DBusDisplayListener {
> > #endif
> >
> > guint dbus_filter;
> > - guint32 out_serial_to_discard;
> > + guint32 display_serial_to_discard;
> > };
> >
> > G_DEFINE_TYPE(DBusDisplayListener, dbus_display_listener, G_TYPE_OBJECT)
> > @@ -93,10 +94,12 @@ G_DEFINE_TYPE(DBusDisplayListener, dbus_display_listener, G_TYPE_OBJECT)
> > static void dbus_gfx_update(DisplayChangeListener *dcl,
> > int x, int y, int w, int h);
> >
> > -static void ddl_discard_pending_messages(DBusDisplayListener *ddl)
> > +static void ddl_discard_display_messages(DBusDisplayListener *ddl)
> > {
> > - ddl->out_serial_to_discard = g_dbus_connection_get_last_serial(
> > + guint32 serial = g_dbus_connection_get_last_serial(
> > g_dbus_proxy_get_connection(G_DBUS_PROXY(ddl->proxy)));
> > +
> > + g_atomic_int_set(&ddl->display_serial_to_discard, serial);
> > }
> >
> > #ifdef CONFIG_OPENGL
> > @@ -290,7 +293,7 @@ static void dbus_scanout_dmabuf(DisplayChangeListener *dcl,
> > return;
> > }
> >
> > - ddl_discard_pending_messages(ddl);
> > + ddl_discard_display_messages(ddl);
> >
> > width = qemu_dmabuf_get_width(dmabuf);
> > height = qemu_dmabuf_get_height(dmabuf);
> > @@ -338,7 +341,7 @@ static bool dbus_scanout_map(DBusDisplayListener *ddl)
> > return false;
> > }
> >
> > - ddl_discard_pending_messages(ddl);
> > + ddl_discard_display_messages(ddl);
> >
> > if (!qemu_dbus_display1_listener_win32_map_call_scanout_map_sync(
> > ddl->map_proxy,
> > @@ -401,7 +404,7 @@ dbus_scanout_share_d3d_texture(
> > return false;
> > }
> >
> > - ddl_discard_pending_messages(ddl);
> > + ddl_discard_display_messages(ddl);
> >
> > qemu_dbus_display1_listener_win32_d3d11_call_scanout_texture2d(
> > ddl->d3d11_proxy,
> > @@ -659,7 +662,7 @@ static void ddl_scanout(DBusDisplayListener *ddl)
> > surface_stride(ddl->ds) * surface_height(ddl->ds), TRUE,
> > (GDestroyNotify)pixman_image_unref, pixman_image_ref(ddl->ds->image));
> >
> > - ddl_discard_pending_messages(ddl);
> > + ddl_discard_display_messages(ddl);
> >
> > qemu_dbus_display1_listener_call_scanout(
> > ddl->proxy, surface_width(ddl->ds), surface_height(ddl->ds),
> > @@ -992,17 +995,34 @@ dbus_filter(GDBusConnection *connection,
> > gpointer user_data)
> > {
> > DBusDisplayListener *ddl = DBUS_DISPLAY_LISTENER(user_data);
> > - guint32 serial;
> > + const gchar *member = NULL;
>
> I suggest removing the initialization will NULL as it may suppress
> uninitialized variable warning.
ok
>
> > + guint32 serial, discard_serial;
> >
> > if (incoming) {
> > return message;
> > }
> >
> > serial = g_dbus_message_get_serial(message);
> > - if (serial <= ddl->out_serial_to_discard) {
> > - trace_dbus_filter(serial, ddl->out_serial_to_discard);
> > - g_object_unref(message);
> > - return NULL;
> > +
> > + discard_serial = g_atomic_int_get(&ddl->display_serial_to_discard);
> > + if (serial <= discard_serial) {
> > + member = g_dbus_message_get_member(message);
> > + if (g_strv_contains((const gchar *[]) {
> > + "Scanout",
> > + "Update",
> > +#ifdef CONFIG_GBM
> > + "ScanoutDMABUF",
> > + "UpdateDMABUF",
> > +#endif
> > + "ScanoutMap",
> > + "UpdateMap",
> > + "Disable",
> > + NULL,
> > + }, member)) {
>
> I prefer to have a static variable for the array. It makes the object
> code simpler and also avoids to have a multi-line condition in the if
> statement.
>
done, thanks
> > + trace_dbus_filter(serial, discard_serial);
> > + g_object_unref(message);
> > + return NULL;
> > + }
> > }
> >
> > return message;
>
© 2016 - 2024 Red Hat, Inc.