[WESTON PATCH 2/2] compositor-drm: Watchdog timer implementation

[Bill Spitzak]

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.