linux-kernel - Re: [PATCH v5 4/8] Watchdog: introdouce "pretimeout" into framework

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20150611163843.GA23656@roeck-us.net>
Date:	Thu, 11 Jun 2015 09:38:43 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Fu Wei <fu.wei@...aro.org>
Cc:	Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>,
	Linaro ACPI Mailman List <linaro-acpi@...ts.linaro.org>,
	linux-watchdog@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
	Wei Fu <tekkamanninja@...il.com>,
	G Gregory <graeme.gregory@...aro.org>,
	Al Stone <al.stone@...aro.org>,
	Hanjun Guo <hanjun.guo@...aro.org>,
	Timur Tabi <timur@...eaurora.org>,
	Ashwin Chaugule <ashwin.chaugule@...aro.org>,
	Arnd Bergmann <arnd@...db.de>,
	Vipul Gandhi <vgandhi@...eaurora.org>,
	Wim Van Sebroeck <wim@...ana.be>,
	Jon Masters <jcm@...hat.com>, Leo Duran <leo.duran@....com>,
	Jon Corbet <corbet@....net>,
	Mark Rutland <mark.rutland@....com>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>, rjw@...ysocki.net
Subject: Re: [PATCH v5 4/8] Watchdog: introdouce "pretimeout" into framework

On Thu, Jun 11, 2015 at 07:22:44PM +0800, Fu Wei wrote:
> Hi Guenter,
> 
[ ... ]
> 
> > value we are trying to set. Effectively the above accepts every pretimeout
> > if wdd->pretimeout is 0. It also accepts every pretimeout if
> > max_pretimeout == 0, even if wdd->timeout is set and t >= wdd->timeout.
> >
> > Try
> >
> >         return (wdd->max_pretimeout && (t < wdd->min_pretimeout ||
> >                                         t > wdd->max_pretimeout)) ||
> >                 (wdd->timeout && t >= wdd->timeout);
> >
> >>   }
> 
> /*
>  * Use the following function to check if a pretimeout value is invalid.
>  * It can be "0", that means we don't use pretimeout.
>  * This function returns 0, when pretimeout is 0.

returns false if pretimeout is 0.

>  */
> static inline bool watchdog_pretimeout_invalid(struct watchdog_device *wdd,
>                            unsigned int t)
> {
>     return t && (wdd->max_pretimeout &&
>         (t < wdd->min_pretimeout || t > wdd->max_pretimeout)) ||
>            (wdd->timeout && t >= wdd->timeout);
> }
> 
Makes sense. Be careful with (), though. This evaluates to

	return t && (...) || (wdd->timeout && t >= wdd->timeout);

but it should probably be
	return t && ((...) || (wdd->timeout && t >= wdd->timeout));

That doesn't make a difference in practice (if t == 0, it is never >= timeout
if timeout is > 0), but it would be a bit cleaner.

> 
> >>
> >>   /* Use the following functions to manipulate watchdog driver specific
> >> data */
> >> @@ -132,11 +153,20 @@ static inline void *watchdog_get_drvdata(struct
> >> watchdog_device *wdd)
> >>   }
> >>
> >>   /* drivers/watchdog/watchdog_core.c */
> >> -extern int watchdog_init_timeout(struct watchdog_device *wdd,
> >> -                                 unsigned int timeout_parm, struct device
> >> *dev);
> >> +int watchdog_init_timeouts(struct watchdog_device *wdd,
> >> +                                 unsigned int pretimeout_parm,
> >> +                                 unsigned int timeout_parm,
> >> +                                 struct device *dev);
> >
> >
> > Please align continuation lines with '('.
> 
> Fixed , thanks
> 
> >
> >
> >>   extern int watchdog_register_device(struct watchdog_device *);
> >>   extern void watchdog_unregister_device(struct watchdog_device *);
> >>
> >> +static inline int watchdog_init_timeout(struct watchdog_device *wdd,
> >> +                                       unsigned int timeout_parm,
> >> +                                       struct device *dev)
> >> +{
> >> +       return watchdog_init_timeouts(wdd, 0, timeout_parm, dev);
> >> +}
> >> +
> >>   #ifdef CONFIG_HARDLOCKUP_DETECTOR
> >>   void watchdog_nmi_disable_all(void);
> >>   void watchdog_nmi_enable_all(void);
> >>
> >
> 
> 
> 
> -- 
> Best regards,
> 
> Fu Wei
> Software Engineer
> Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
> Ph: +86 21 61221326(direct)
> Ph: +86 186 2020 4684 (mobile)
> Room 1512, Regus One Corporate Avenue,Level 15,
> One Corporate Avenue,222 Hubin Road,Huangpu District,
> Shanghai,China 200021
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/