Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752075AbbERGdv (ORCPT ); Mon, 18 May 2015 02:33:51 -0400 Received: from mga11.intel.com ([192.55.52.93]:31813 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750782AbbERGdl (ORCPT ); Mon, 18 May 2015 02:33:41 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,451,1427785200"; d="scan'208";a="730966407" Message-ID: <555987C0.6000500@linux.intel.com> Date: Mon, 18 May 2015 14:33:36 +0800 From: "Fu, Zhonghui" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: "Rafael J. Wysocki" CC: len.brown@intel.com, pavel@ucw.cz, Greg Kroah-Hartman , linux-pm@vger.kernel.org, "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] PM / sleep: cancel the synchronous restriction of pm-trace References: <554A297C.2040305@linux.intel.com> In-Reply-To: <554A297C.2040305@linux.intel.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8091 Lines: 287 Hi guys, Could you please come up with your comments about this patch? Thanks, Zhonghui On 2015/5/6 22:47, Fu, Zhonghui wrote: > Some system-hang occur only when multiple device PM methods > are running asynchronously. So should cancel the synchronization > of pm-trace, and make it suitable for asynchronous PM environment. > > Signed-off-by: Zhonghui Fu > --- > drivers/base/power/main.c | 53 +++++++++++++++++++++----------------------- > include/linux/pm-trace.h | 24 ++++++++++++-------- > kernel/power/main.c | 2 + > 3 files changed, 41 insertions(+), 38 deletions(-) > > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c > index 3d874ec..40daf48 100644 > --- a/drivers/base/power/main.c > +++ b/drivers/base/power/main.c > @@ -476,9 +476,6 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn > char *info = NULL; > int error = 0; > > - TRACE_DEVICE(dev); > - TRACE_RESUME(0); > - > if (dev->power.syscore || dev->power.direct_complete) > goto Out; > > @@ -506,19 +503,21 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn > callback = pm_noirq_op(dev->driver->pm, state); > } > > + TRACE_DEVICE_START(dev); > + TRACE_RESUME(0); > + > error = dpm_run_callback(callback, dev, state, info); > dev->power.is_noirq_suspended = false; > > + TRACE_DEVICE_END(); > Out: > complete_all(&dev->power.completion); > - TRACE_RESUME(error); > return error; > } > > static bool is_async(struct device *dev) > { > - return dev->power.async_suspend && pm_async_enabled > - && !pm_trace_is_enabled(); > + return dev->power.async_suspend && pm_async_enabled; > } > > static void async_resume_noirq(void *data, async_cookie_t cookie) > @@ -605,9 +604,6 @@ static int device_resume_early(struct device *dev, pm_message_t state, bool asyn > char *info = NULL; > int error = 0; > > - TRACE_DEVICE(dev); > - TRACE_RESUME(0); > - > if (dev->power.syscore || dev->power.direct_complete) > goto Out; > > @@ -635,12 +631,14 @@ static int device_resume_early(struct device *dev, pm_message_t state, bool asyn > callback = pm_late_early_op(dev->driver->pm, state); > } > > + TRACE_DEVICE_START(dev); > + TRACE_RESUME(0); > + > error = dpm_run_callback(callback, dev, state, info); > dev->power.is_late_suspended = false; > > + TRACE_DEVICE_END(); > Out: > - TRACE_RESUME(error); > - > pm_runtime_enable(dev); > complete_all(&dev->power.completion); > return error; > @@ -734,9 +732,6 @@ static int device_resume(struct device *dev, pm_message_t state, bool async) > int error = 0; > DECLARE_DPM_WATCHDOG_ON_STACK(wd); > > - TRACE_DEVICE(dev); > - TRACE_RESUME(0); > - > if (dev->power.syscore) > goto Complete; > > @@ -801,9 +796,13 @@ static int device_resume(struct device *dev, pm_message_t state, bool async) > } > > End: > + TRACE_DEVICE_START(dev); > + TRACE_RESUME(0); > + > error = dpm_run_callback(callback, dev, state, info); > dev->power.is_suspended = false; > > + TRACE_DEVICE_END(); > Unlock: > device_unlock(dev); > dpm_watchdog_clear(&wd); > @@ -811,8 +810,6 @@ static int device_resume(struct device *dev, pm_message_t state, bool async) > Complete: > complete_all(&dev->power.completion); > > - TRACE_RESUME(error); > - > return error; > } > > @@ -1017,9 +1014,6 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a > char *info = NULL; > int error = 0; > > - TRACE_DEVICE(dev); > - TRACE_SUSPEND(0); > - > if (async_error) > goto Complete; > > @@ -1052,15 +1046,18 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a > callback = pm_noirq_op(dev->driver->pm, state); > } > > + TRACE_DEVICE_START(dev); > + TRACE_SUSPEND(0); > + > error = dpm_run_callback(callback, dev, state, info); > if (!error) > dev->power.is_noirq_suspended = true; > else > async_error = error; > > + TRACE_DEVICE_END(); > Complete: > complete_all(&dev->power.completion); > - TRACE_SUSPEND(error); > return error; > } > > @@ -1161,9 +1158,6 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as > char *info = NULL; > int error = 0; > > - TRACE_DEVICE(dev); > - TRACE_SUSPEND(0); > - > __pm_runtime_disable(dev, false); > > if (async_error) > @@ -1198,14 +1192,17 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as > callback = pm_late_early_op(dev->driver->pm, state); > } > > + TRACE_DEVICE_START(dev); > + TRACE_SUSPEND(0); > + > error = dpm_run_callback(callback, dev, state, info); > if (!error) > dev->power.is_late_suspended = true; > else > async_error = error; > > + TRACE_DEVICE_END(); > Complete: > - TRACE_SUSPEND(error); > complete_all(&dev->power.completion); > return error; > } > @@ -1346,9 +1343,6 @@ static int __device_suspend(struct device *dev, pm_message_t state, bool async) > int error = 0; > DECLARE_DPM_WATCHDOG_ON_STACK(wd); > > - TRACE_DEVICE(dev); > - TRACE_SUSPEND(0); > - > dpm_wait_for_children(dev, async); > > if (async_error) > @@ -1428,8 +1422,12 @@ static int __device_suspend(struct device *dev, pm_message_t state, bool async) > callback = pm_op(dev->driver->pm, state); > } > > + TRACE_DEVICE_START(dev); > + TRACE_SUSPEND(0); > + > error = dpm_run_callback(callback, dev, state, info); > > + TRACE_DEVICE_END(); > End: > if (!error) { > struct device *parent = dev->parent; > @@ -1455,7 +1453,6 @@ static int __device_suspend(struct device *dev, pm_message_t state, bool async) > if (error) > async_error = error; > > - TRACE_SUSPEND(error); > return error; > } > > diff --git a/include/linux/pm-trace.h b/include/linux/pm-trace.h > index ecbde7a..aa480b1 100644 > --- a/include/linux/pm-trace.h > +++ b/include/linux/pm-trace.h > @@ -6,29 +6,33 @@ > #include > > extern int pm_trace_enabled; > - > -static inline int pm_trace_is_enabled(void) > -{ > - return pm_trace_enabled; > -} > +extern struct mutex pt_mutex; > > struct device; > extern void set_trace_device(struct device *); > extern void generate_pm_trace(const void *tracedata, unsigned int user); > extern int show_trace_dev_match(char *buf, size_t size); > > -#define TRACE_DEVICE(dev) do { \ > +#define TRACE_DEVICE_START(dev) do { \ > if (pm_trace_enabled) \ > + mutex_lock(&pt_mutex); \ > set_trace_device(dev); \ > } while(0) > > +#define TRACE_DEVICE_END() \ > +do { \ > + if (pm_trace_enabled) { \ > + mutex_unlock(&pt_mutex); \ > + } \ > +} while (0) > + > #else > > -static inline int pm_trace_is_enabled(void) { return 0; } > +#define TRACE_DEVICE_START(dev) do { } while (0) > +#define TRACE_DEVICE_END() do { } while (0) > > -#define TRACE_DEVICE(dev) do { } while (0) > -#define TRACE_RESUME(dev) do { } while (0) > -#define TRACE_SUSPEND(dev) do { } while (0) > +#define TRACE_RESUME(user) do { } while (0) > +#define TRACE_SUSPEND(user) do { } while (0) > > #endif > > diff --git a/kernel/power/main.c b/kernel/power/main.c > index 86e8157..ffd5145 100644 > --- a/kernel/power/main.c > +++ b/kernel/power/main.c > @@ -515,6 +515,7 @@ power_attr(wake_unlock); > > #ifdef CONFIG_PM_TRACE > int pm_trace_enabled; > +struct mutex pt_mutex; > > static ssize_t pm_trace_show(struct kobject *kobj, struct kobj_attribute *attr, > char *buf) > @@ -533,6 +534,7 @@ pm_trace_store(struct kobject *kobj, struct kobj_attribute *attr, > if (pm_trace_enabled) { > pr_warn("PM: Enabling pm_trace changes system date and time during resume.\n" > "PM: Correct system time has to be restored manually after resume.\n"); > + mutex_init(&pt_mutex); > } > return n; > } > -- 1.7.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/