From c90fccc256ee4cb8d6a516695708bf0c3685c348 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Sun, 30 Jun 2019 15:51:10 +0200 Subject: [PATCH] backend-drm: fix race during system suspend Depending on system loading, weston-launcher could drop the drm master access before the compositor and all the clients receive the notification. In this case, some commit could be sent to the drm driver too late and get refused with error EACCES. This error condition is not properly managed and causes weston to hang. Change the return type of start_repaint_loop() and repaint_flush() from void to int, and return 0 on success or -1 if the repaint has to be cancelled. In the callers of start_repaint_loop() and repaint_flush() handle the return value and cancel the repaint when needed. In backend-drm detect the error EACCES and return -1. Note: to keep the code cleaner, this change inverts the execution order between weston_output_schedule_repaint_reset() and repaint_cancel(). No need to wait for suspend or for any notification; in case the weston reschedules a repaint, it will get EACCES again. At resume, damage-all guarantees a complete repaint. This fix is for atomic modeset only. Legacy modeset suffers from similar problems, but it is not fixed by this change. Since drm_pending_state_apply() never returns error for legacy modeset, this change has no impact on legacy modeset. Signed-off-by: Antonio Borneo Fixes: https://gitlab.freedesktop.org/wayland/weston/issues/117 --- include/libweston/libweston.h | 6 +++--- libweston/backend-drm/drm.c | 25 ++++++++++++++++++------- libweston/backend-fbdev/fbdev.c | 4 +++- libweston/backend-headless/headless.c | 4 +++- libweston/backend-rdp/rdp.c | 4 +++- libweston/backend-wayland/wayland.c | 4 +++- libweston/backend-x11/x11.c | 4 +++- libweston/compositor.c | 19 ++++++++++++------- remoting/remoting-plugin.c | 6 ++++-- 9 files changed, 52 insertions(+), 24 deletions(-) diff --git a/include/libweston/libweston.h b/include/libweston/libweston.h index a50a0bbf..2c1e8645 100644 --- a/include/libweston/libweston.h +++ b/include/libweston/libweston.h @@ -301,7 +301,7 @@ struct weston_output { enum weston_hdcp_protection current_protection; bool allow_protection; - void (*start_repaint_loop)(struct weston_output *output); + int (*start_repaint_loop)(struct weston_output *output); int (*repaint)(struct weston_output *output, pixman_region32_t *damage, void *repaint_data); @@ -1053,8 +1053,8 @@ struct weston_backend { * * @param repaint_data Data returned by repaint_begin */ - void (*repaint_flush)(struct weston_compositor *compositor, - void *repaint_data); + int (*repaint_flush)(struct weston_compositor *compositor, + void *repaint_data); /** Allocate a new output * diff --git a/libweston/backend-drm/drm.c b/libweston/backend-drm/drm.c index 07ae2643..d513010c 100644 --- a/libweston/backend-drm/drm.c +++ b/libweston/backend-drm/drm.c @@ -463,7 +463,7 @@ drm_waitvblank_pipe(struct drm_output *output) return 0; } -static void +static int drm_output_start_repaint_loop(struct weston_output *output_base) { struct drm_output *output = to_drm_output(output_base); @@ -482,7 +482,7 @@ drm_output_start_repaint_loop(struct weston_output *output_base) }; if (output->disable_pending || output->destroy_pending) - return; + return 0; if (!output->scanout_plane->state_cur->fb) { /* We can't page flip if there's no mode set */ @@ -519,7 +519,7 @@ drm_output_start_repaint_loop(struct weston_output *output_base) drm_output_update_msc(output, vbl.reply.sequence); weston_output_finish_frame(output_base, &ts, WP_PRESENTATION_FEEDBACK_INVALID); - return; + return 0; } } @@ -538,15 +538,18 @@ drm_output_start_repaint_loop(struct weston_output *output_base) if (ret != 0) { weston_log("applying repaint-start state failed: %s\n", strerror(errno)); + if (ret == -EACCES) + return -1; goto finish_frame; } - return; + return 0; finish_frame: /* if we cannot page-flip, immediately finish frame */ weston_output_finish_frame(output_base, NULL, WP_PRESENTATION_FEEDBACK_INVALID); + return 0; } /** @@ -585,15 +588,21 @@ drm_repaint_begin(struct weston_compositor *compositor) * the update completes (see drm_output_update_complete), the output * state will be freed. */ -static void +static int drm_repaint_flush(struct weston_compositor *compositor, void *repaint_data) { struct drm_backend *b = to_drm_backend(compositor); struct drm_pending_state *pending_state = repaint_data; + int ret; + + ret = drm_pending_state_apply(pending_state); + if (ret != 0) + weston_log("repaint-flush failed: %s\n", strerror(errno)); - drm_pending_state_apply(pending_state); drm_debug(b, "[repaint] flushed pending_state %p\n", pending_state); b->repaint_data = NULL; + + return (ret == -EACCES) ? -1 : 0; } /** @@ -2928,11 +2937,13 @@ renderer_switch_binding(struct weston_keyboard *keyboard, switch_to_gl_renderer(b); } -static void +static int drm_virtual_output_start_repaint_loop(struct weston_output *output_base) { weston_output_finish_frame(output_base, NULL, WP_PRESENTATION_FEEDBACK_INVALID); + + return 0; } static int diff --git a/libweston/backend-fbdev/fbdev.c b/libweston/backend-fbdev/fbdev.c index bcda7d0e..ae289cc0 100644 --- a/libweston/backend-fbdev/fbdev.c +++ b/libweston/backend-fbdev/fbdev.c @@ -131,13 +131,15 @@ fbdev_output_get_head(struct fbdev_output *output) struct fbdev_head, base.output_link); } -static void +static int fbdev_output_start_repaint_loop(struct weston_output *output) { struct timespec ts; weston_compositor_read_presentation_clock(output->compositor, &ts); weston_output_finish_frame(output, &ts, WP_PRESENTATION_FEEDBACK_INVALID); + + return 0; } static int diff --git a/libweston/backend-headless/headless.c b/libweston/backend-headless/headless.c index 04c33d15..2d019ee5 100644 --- a/libweston/backend-headless/headless.c +++ b/libweston/backend-headless/headless.c @@ -80,13 +80,15 @@ to_headless_backend(struct weston_compositor *base) return container_of(base->backend, struct headless_backend, base); } -static void +static int headless_output_start_repaint_loop(struct weston_output *output) { struct timespec ts; weston_compositor_read_presentation_clock(output->compositor, &ts); weston_output_finish_frame(output, &ts, WP_PRESENTATION_FEEDBACK_INVALID); + + return 0; } static int diff --git a/libweston/backend-rdp/rdp.c b/libweston/backend-rdp/rdp.c index 31af1096..109c20ba 100644 --- a/libweston/backend-rdp/rdp.c +++ b/libweston/backend-rdp/rdp.c @@ -382,13 +382,15 @@ rdp_peer_refresh_region(pixman_region32_t *region, freerdp_peer *peer) rdp_peer_refresh_raw(region, output->shadow_surface, peer); } -static void +static int rdp_output_start_repaint_loop(struct weston_output *output) { struct timespec ts; weston_compositor_read_presentation_clock(output->compositor, &ts); weston_output_finish_frame(output, &ts, WP_PRESENTATION_FEEDBACK_INVALID); + + return 0; } static int diff --git a/libweston/backend-wayland/wayland.c b/libweston/backend-wayland/wayland.c index 729cbb71..dd4c1cca 100644 --- a/libweston/backend-wayland/wayland.c +++ b/libweston/backend-wayland/wayland.c @@ -481,7 +481,7 @@ wayland_output_update_gl_border(struct wayland_output *output) } #endif -static void +static int wayland_output_start_repaint_loop(struct weston_output *output_base) { struct wayland_output *output = to_wayland_output(output_base); @@ -503,6 +503,8 @@ wayland_output_start_repaint_loop(struct weston_output *output_base) wl_callback_add_listener(output->frame_cb, &frame_listener, output); wl_surface_commit(output->parent.surface); wl_display_flush(wb->parent.wl_display); + + return 0; } #ifdef ENABLE_EGL diff --git a/libweston/backend-x11/x11.c b/libweston/backend-x11/x11.c index 854d053f..e6227737 100644 --- a/libweston/backend-x11/x11.c +++ b/libweston/backend-x11/x11.c @@ -399,13 +399,15 @@ x11_input_destroy(struct x11_backend *b) weston_seat_release(&b->core_seat); } -static void +static int x11_output_start_repaint_loop(struct weston_output *output) { struct timespec ts; weston_compositor_read_presentation_clock(output->compositor, &ts); weston_output_finish_frame(output, &ts, WP_PRESENTATION_FEEDBACK_INVALID); + + return 0; } static int diff --git a/libweston/compositor.c b/libweston/compositor.c index 0baf6edb..deb2edd7 100644 --- a/libweston/compositor.c +++ b/libweston/compositor.c @@ -2790,17 +2790,19 @@ output_repaint_timer_handler(void *data) if (ret == 0) { if (compositor->backend->repaint_flush) - compositor->backend->repaint_flush(compositor, - repaint_data); + ret = compositor->backend->repaint_flush(compositor, + repaint_data); } else { + if (compositor->backend->repaint_cancel) + compositor->backend->repaint_cancel(compositor, + repaint_data); + } + + if (ret != 0) { wl_list_for_each(output, &compositor->output_list, link) { if (output->repainted) weston_output_schedule_repaint_reset(output); } - - if (compositor->backend->repaint_cancel) - compositor->backend->repaint_cancel(compositor, - repaint_data); } wl_list_for_each(output, &compositor->output_list, link) @@ -2887,11 +2889,14 @@ static void idle_repaint(void *data) { struct weston_output *output = data; + int ret; assert(output->repaint_status == REPAINT_BEGIN_FROM_IDLE); output->repaint_status = REPAINT_AWAITING_COMPLETION; output->idle_repaint_source = NULL; - output->start_repaint_loop(output); + ret = output->start_repaint_loop(output); + if (ret != 0) + weston_output_schedule_repaint_reset(output); } WL_EXPORT void diff --git a/remoting/remoting-plugin.c b/remoting/remoting-plugin.c index 7c9bbb11..753f206c 100644 --- a/remoting/remoting-plugin.c +++ b/remoting/remoting-plugin.c @@ -93,7 +93,7 @@ struct remoted_output { void (*saved_destroy)(struct weston_output *output); int (*saved_enable)(struct weston_output *output); int (*saved_disable)(struct weston_output *output); - void (*saved_start_repaint_loop)(struct weston_output *output); + int (*saved_start_repaint_loop)(struct weston_output *output); char *host; int port; @@ -647,7 +647,7 @@ remoting_output_destroy(struct weston_output *output) free(remoted_output); } -static void +static int remoting_output_start_repaint_loop(struct weston_output *output) { struct remoted_output *remoted_output = lookup_remoted_output(output); @@ -658,6 +658,8 @@ remoting_output_start_repaint_loop(struct weston_output *output) msec = millihz_to_nsec(remoted_output->output->current_mode->refresh) / 1000000; wl_event_source_timer_update(remoted_output->finish_frame_timer, msec); + + return 0; } static int