[WESTON PATCH 2/2] compositor-drm: Watchdog timer implementation
spitzak at gmail.com
Wed Oct 29 11:45:15 PDT 2014
On 10/29/2014 11:13 AM, Frederic Plourde wrote:
> This watchdog timer (WDT) implementation is software only and includes
> basic features usually found in a WDT. We simply exit Weston gracefully
> with an exit code when the dog barks.
> The watchdog timeout value can be set via weston.ini by adding a
> watchdog-timer-timeout=<SECONDS> entry under a new [compositor-drm]
> section. Setting this timeout to 0 disables the watchdog feature.
> +/* Creates a watchdog timer. Note that timer is not armed by default */
> +static int
> +drm_output_watchdog_timer_create(struct watchdog_timer *wdt, struct wl_display *display,
> + wl_event_loop_timer_func_t timeout_func, void *timer_arg,
> + uint32_t timeout)
I think it would be better to remove all these arguments and just make
this function directly use the values that are being passed to it now.
Right now it is a bit hard to find what function is called on timeout
because I have to locate the call to this function. Also this should
print to the log the error rather than the caller doing so.
> +/* Start/reset/arm the dog's timer. This can also be used to 'pat' the dog */
> +static int
> +drm_output_watchdog_timer_start(struct watchdog_timer *wdt)
> + if (wdt->timer)
> + return wl_event_source_timer_update(wdt->timer, wdt->timeout);
> + return -1;
Instead of the caller doing the logging I would do it here. You could
skip the log message if it failed to create the timer earlier since that
would have printed a message.
IMHO this is a better method for making these functions as long as they
are static and not intended to be reused.
Also wondering if weston_compositor_exit_with_code could be avoided by
fixing things to use atexit and then directly calling exit(). Or at
least fix it up enough that when you want to exit with an error it is ok
to skip the cleanup code that was missed.
More information about the wayland-devel