Messages in this thread | | | Date | Mon, 13 May 2013 09:03:14 -0700 | From | John Stultz <> | Subject | Re: [RFC PATCHv2 2/2] PM: compile-time configuration of device suspend/resume watchdogs. |
| |
On 05/10/2013 11:23 PM, Colin Cross wrote: > On Fri, May 10, 2013 at 2:28 PM, Zoran Markovic > <zoran.markovic@linaro.org> wrote: >> +#ifdef CONFIG_DPM_WD >> +/** >> + * dpm_wd_action - recovery from suspend/resume watchdog timeout >> + * @wd: Watchdog. Must be allocated on the stack. >> + */ >> +#if defined(CONFIG_DPM_WD_ACTION_STACKTRACE) >> +static inline void dpm_wd_action(struct dpm_watchdog *wd) >> +{ >> + show_stack(wd->tsk, NULL); >> +} >> +#elif defined(CONFIG_DPM_WD_ACTION_PANIC) >> +static inline void dpm_wd_action(struct dpm_watchdog *wd) >> +{ >> + panic("%s: unrecoverable failure\n", dev_name(wd->dev)); > The panic here is not very useful, it's going to print the stack of > the task that was running when the timer fired which is likely to be > the idle task if the suspend task is deadlocked. This should call > show_stack and panic. If you take out the log action, then all this > can stay inline with the handler and be: > > dev_emerg(wd->dev, "**** DPM device timeout ****\n"); > show_stack(wd->tsk, NULL); > #ifdef CONFIG_DPM_WD_ACTION_PANIC > panic("%s: unrecoverable failure\n", dev_name(wd->dev)); > #endif
#ifdefs in functions are usually to be avoided. Thus why I suggested he use the config dependent dpm_wd_action() function to handle this.
thanks -john
| |