On 14-10-29 02:45 PM, Bill Spitzak wrote:
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.
Mhh... you mean something like :
static int
drm_output_watchdog_timer_create(struct drm_output *output)
{
struct wl_event_loop *loop = NULL;
struct weston_compositor *ec = output_base->compositor;
loop = wl_display_get_event_loop(ec->wl_display);
if (loop) {
output->wdt->timer = wl_event_loop_add_timer(loop,
drm_output_watchdog_bark,
ec->watchdog_timer_timeout);
blah..blah...
?
If so, then I think I'd prefer to completely remove
'drm_output_watchdog_timer_create" and inline it directly from
'create_output_for_connector', since it's not a very big function anyway
and we're not sure as of today that we're going to reuse this for
anything else.
In that case, I'd also remove 'drm_output_watchdog_timer_destroy' and
inline it in 'drm_output_destroy'
what do you think ?
I'm usually not a big fan of static functions with hard-coded values,
but if you tell me it's better for clarity... I might be tempted, dunno.
+/* 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.
fair.
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.
I'd like to hear Pekka on that one ;-)
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
