Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1416933

Re: [PATCH v3 4/6] watchdog: add watchdog pretimeout framework

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

On Tue, Jun 07, 2016 at 08:38:45PM +0300, Vladimir Zapolskiy wrote:
> The change adds a simple watchdog pretimeout framework infrastructure,
> its purpose is to allow users to select a desired handling of watchdog
> pretimeout events, which may be generated by some watchdog devices.
> 
> A user selects a default watchdog pretimeout governor during
> compilation stage.
> 
> Watchdogs with WDIOF_PRETIMEOUT capability now have two device
> attributes in sysfs: pretimeout to display currently set pretimeout
> value and pretimeout_governor attribute to display the selected
> watchdog pretimeout governor.
> 
> Watchdogs with no WDIOF_PRETIMEOUT capability has no changes in
> sysfs, and such watchdog devices do not require the framework.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> ---
> Changes from v2 to v3:
> * essentially simplified the implementation due to removal of runtime
>   dynamic selection of watchdog pretimeout governors by a user, this
>   feature is supposed to be added later on

Hmm, your call, but I'm not sure this will make the reviewing process
easier...

> * removed support of sleepable watchdog pretimeout governors

This does.

> * moved sysfs device attributes to watchdog_dev.c, this required to
>   add exported watchdog_pretimeout_governor_name() interface

Why this move? Before, all the pretimeout stuff was nicely encapsulated
in its own file which could be compiled out. Now things are mixing. What
was wrong with the approach I took?`

> @@ -244,6 +245,13 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
>  		}
>  	}
>  
> +	ret = watchdog_register_pretimeout(wdd);
> +	if (ret) {
> +		watchdog_dev_unregister(wdd);
> +		ida_simple_remove(&watchdog_ida, wdd->id);
> +		return ret;
> +	}
> +

What is the advantage of adding it here instead of adding it in
watchdog_dev.c? I mean the files to control govenors are tied to the
watchdog_device anyhow, so I'd think it's cleaner to move all that
action to watchdog_dev instead of having this stray one in the core.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v3 0/6] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-06-07 19:50 +0200
  [PATCH v3 4/6] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-06-07 19:50 +0200
    Re: [PATCH v3 4/6] watchdog: add watchdog pretimeout framework Guenter Roeck <linux@roeck-us.net> - 2016-06-07 23:50 +0200
      Re: [PATCH v3 4/6] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-06-08 15:40 +0200
        Re: [PATCH v3 4/6] watchdog: add watchdog pretimeout framework Guenter Roeck <linux@roeck-us.net> - 2016-06-08 16:00 +0200
          Re: [PATCH v3 4/6] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-06-08 17:20 +0200
            kbuild: default n removals? (was: Re: [PATCH v3 4/6] watchdog: add  watchdog pretimeout framework) Joe Perches <joe@perches.com> - 2016-06-08 17:40 +0200
              Re: kbuild: default n removals? (was: Re: [PATCH v3 4/6] watchdog:  add watchdog pretimeout framework) Guenter Roeck <linux@roeck-us.net> - 2016-06-08 20:10 +0200
    Re: [PATCH v3 4/6] watchdog: add watchdog pretimeout framework Wolfram Sang <wsa@the-dreams.de> - 2016-06-08 09:00 +0200
      Re: [PATCH v3 4/6] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-06-08 16:10 +0200
        Re: [PATCH v3 4/6] watchdog: add watchdog pretimeout framework Guenter Roeck <linux@roeck-us.net> - 2016-06-08 20:30 +0200
          Re: [PATCH v3 4/6] watchdog: add watchdog pretimeout framework Wolfram Sang <wsa@the-dreams.de> - 2016-06-09 23:30 +0200
        Re: [PATCH v3 4/6] watchdog: add watchdog pretimeout framework Wolfram Sang <wsa@the-dreams.de> - 2016-06-09 23:20 +0200
  [PATCH v3 5/6] watchdog: pretimeout: add panic pretimeout governor Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-06-07 19:50 +0200
    Re: [PATCH v3 5/6] watchdog: pretimeout: add panic pretimeout  governor Wolfram Sang <wsa@the-dreams.de> - 2016-06-08 09:10 +0200
      Re: [PATCH v3 5/6] watchdog: pretimeout: add panic pretimeout  governor Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-06-08 16:30 +0200
        Re: [PATCH v3 5/6] watchdog: pretimeout: add panic pretimeout  governor Wolfram Sang <wsa@the-dreams.de> - 2016-06-09 23:30 +0200
  [PATCH v3 2/6] watchdog: add WDIOC_SETPRETIMEOUT and WDIOC_GETPRETIMEOUT Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-06-07 19:50 +0200
  [PATCH v3 3/6] watchdog: add pretimeout read-only device attribute to sysfs Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-06-07 19:50 +0200
    Re: [PATCH v3 3/6] watchdog: add pretimeout read-only device  attribute to sysfs Wolfram Sang <wsa@the-dreams.de> - 2016-06-08 09:00 +0200
  Re: [PATCH v3 0/6] watchdog: add watchdog pretimeout framework Wolfram Sang <wsa@the-dreams.de> - 2016-06-08 10:00 +0200
    Re: [PATCH v3 0/6] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-06-08 17:40 +0200
      Re: [PATCH v3 0/6] watchdog: add watchdog pretimeout framework Wolfram Sang <wsa@the-dreams.de> - 2016-06-09 23:40 +0200

csiph-web