All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace
@ 2015-01-26  5:07 Fu, Zhonghui
  2015-01-26  7:59 ` Fu, Zhonghui
  2015-01-26 10:23 ` Change behaviour when tracing ... nasty trap (was Re: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace) Pavel Machek
  0 siblings, 2 replies; 9+ messages in thread
From: Fu, Zhonghui @ 2015-01-26  5:07 UTC (permalink / raw)
  To: Rafael J. Wysocki, len.brown, pavel, Greg Kroah-Hartman,
	linux-pm, linux-kernel

>From f9c841d1f943d81b5ab0aac7483e794a7f966296 Mon Sep 17 00:00:00 2001
From: Zhonghui Fu <zhonghui.fu@linux.intel.com>
Date: Mon, 26 Jan 2015 11:27:08 +0800
Subject: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace

There are some kind of dependency between devices in some
hardware platforms. So, asynchronous resuming devices may
hang system due to wrong resume order. As a result, should
not fore synchronously resuming devices during tracing
PM events.

Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
---
 drivers/base/power/main.c    |    3 +--
 include/linux/resume-trace.h |    7 -------
 2 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 9717d5f..5df148b 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -517,8 +517,7 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 
 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)
diff --git a/include/linux/resume-trace.h b/include/linux/resume-trace.h
index f31db23..fd0866e 100644
--- a/include/linux/resume-trace.h
+++ b/include/linux/resume-trace.h
@@ -7,11 +7,6 @@
 
 extern int pm_trace_enabled;
 
-static inline int pm_trace_is_enabled(void)
-{
-       return pm_trace_enabled;
-}
-
 struct device;
 extern void set_trace_device(struct device *);
 extern void generate_resume_trace(const void *tracedata, unsigned int user);
@@ -24,8 +19,6 @@ extern int show_trace_dev_match(char *buf, size_t size);
 
 #else
 
-static inline int pm_trace_is_enabled(void) { return 0; }
-
 #define TRACE_DEVICE(dev) do { } while (0)
 #define TRACE_RESUME(dev) do { } while (0)
 
-- 1.7.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace
  2015-01-26  5:07 [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace Fu, Zhonghui
@ 2015-01-26  7:59 ` Fu, Zhonghui
  2015-01-26 13:44   ` Rafael J. Wysocki
  2015-01-26 10:23 ` Change behaviour when tracing ... nasty trap (was Re: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace) Pavel Machek
  1 sibling, 1 reply; 9+ messages in thread
From: Fu, Zhonghui @ 2015-01-26  7:59 UTC (permalink / raw)
  To: Rafael J. Wysocki, len.brown, pavel, Greg Kroah-Hartman,
	linux-pm, linux-kernel


This is a incorrect patch , please ignore it.


Thanks,
Zhonghui

On 2015/1/26 13:07, Fu, Zhonghui wrote:
> From f9c841d1f943d81b5ab0aac7483e794a7f966296 Mon Sep 17 00:00:00 2001
> From: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> Date: Mon, 26 Jan 2015 11:27:08 +0800
> Subject: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace
>
> There are some kind of dependency between devices in some
> hardware platforms. So, asynchronous resuming devices may
> hang system due to wrong resume order. As a result, should
> not fore synchronously resuming devices during tracing
> PM events.
>
> Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> ---
>  drivers/base/power/main.c    |    3 +--
>  include/linux/resume-trace.h |    7 -------
>  2 files changed, 1 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 9717d5f..5df148b 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -517,8 +517,7 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
>  
>  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)
> diff --git a/include/linux/resume-trace.h b/include/linux/resume-trace.h
> index f31db23..fd0866e 100644
> --- a/include/linux/resume-trace.h
> +++ b/include/linux/resume-trace.h
> @@ -7,11 +7,6 @@
>  
>  extern int pm_trace_enabled;
>  
> -static inline int pm_trace_is_enabled(void)
> -{
> -       return pm_trace_enabled;
> -}
> -
>  struct device;
>  extern void set_trace_device(struct device *);
>  extern void generate_resume_trace(const void *tracedata, unsigned int user);
> @@ -24,8 +19,6 @@ extern int show_trace_dev_match(char *buf, size_t size);
>  
>  #else
>  
> -static inline int pm_trace_is_enabled(void) { return 0; }
> -
>  #define TRACE_DEVICE(dev) do { } while (0)
>  #define TRACE_RESUME(dev) do { } while (0)
>  
> -- 1.7.1
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Change behaviour when tracing ... nasty trap (was Re: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace)
  2015-01-26  5:07 [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace Fu, Zhonghui
  2015-01-26  7:59 ` Fu, Zhonghui
@ 2015-01-26 10:23 ` Pavel Machek
  2015-01-26 10:39   ` Liu, Chuansheng
  1 sibling, 1 reply; 9+ messages in thread
From: Pavel Machek @ 2015-01-26 10:23 UTC (permalink / raw)
  To: Fu, Zhonghui
  Cc: Rafael J. Wysocki, len.brown, Greg Kroah-Hartman, linux-pm,
	linux-kernel, chuansheng.liu

On Mon 2015-01-26 13:07:03, Fu, Zhonghui wrote:
> >From f9c841d1f943d81b5ab0aac7483e794a7f966296 Mon Sep 17 00:00:00 2001
> From: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> Date: Mon, 26 Jan 2015 11:27:08 +0800
> Subject: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace
> 
> There are some kind of dependency between devices in some
> hardware platforms. So, asynchronous resuming devices may
> hang system due to wrong resume order. As a result, should
> not fore synchronously resuming devices during tracing
> PM events.
> 
> Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> ---
>  drivers/base/power/main.c    |    3 +--
>  include/linux/resume-trace.h |    7 -------
>  2 files changed, 1 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 9717d5f..5df148b 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -517,8 +517,7 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
>  
>  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;
>  }
>

Actually... whoever did the original patch was evil person. Changing
behaviour when tracing is requested is evil, evil, evil. Git blame
tells me

Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>

went to the dark side.

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: Change behaviour when tracing ... nasty trap (was Re: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace)
  2015-01-26 10:23 ` Change behaviour when tracing ... nasty trap (was Re: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace) Pavel Machek
@ 2015-01-26 10:39   ` Liu, Chuansheng
  2015-01-26 11:05     ` Pavel Machek
  0 siblings, 1 reply; 9+ messages in thread
From: Liu, Chuansheng @ 2015-01-26 10:39 UTC (permalink / raw)
  To: Pavel Machek, Fu, Zhonghui
  Cc: Rafael J. Wysocki, Brown, Len, Greg Kroah-Hartman, linux-pm,
	linux-kernel

Hello Pavel,

> -----Original Message-----
> From: Pavel Machek [mailto:pavel@ucw.cz]
> Sent: Monday, January 26, 2015 6:24 PM
> To: Fu, Zhonghui
> Cc: Rafael J. Wysocki; Brown, Len; Greg Kroah-Hartman;
> linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; Liu, Chuansheng
> Subject: Change behaviour when tracing ... nasty trap (was Re: [PATCH]
> PM/Trace: get rid of synchronous resume limit during PM trace)
> 
> On Mon 2015-01-26 13:07:03, Fu, Zhonghui wrote:
> > >From f9c841d1f943d81b5ab0aac7483e794a7f966296 Mon Sep 17 00:00:00
> 2001
> > From: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> > Date: Mon, 26 Jan 2015 11:27:08 +0800
> > Subject: [PATCH] PM/Trace: get rid of synchronous resume limit during PM
> trace
> >
> > There are some kind of dependency between devices in some
> > hardware platforms. So, asynchronous resuming devices may
> > hang system due to wrong resume order. As a result, should
> > not fore synchronously resuming devices during tracing
> > PM events.
> >
> > Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> > ---
> >  drivers/base/power/main.c    |    3 +--
> >  include/linux/resume-trace.h |    7 -------
> >  2 files changed, 1 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> > index 9717d5f..5df148b 100644
> > --- a/drivers/base/power/main.c
> > +++ b/drivers/base/power/main.c
> > @@ -517,8 +517,7 @@ static int device_resume_noirq(struct device *dev,
> pm_message_t state, bool asyn
> >
> >  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;
> >  }
> >
> 
> Actually... whoever did the original patch was evil person. Changing
> behaviour when tracing is requested is evil, evil, evil. Git blame
> tells me
> 
> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> 
> went to the dark side.

Although I didn't get where is something wrong, but the is_async() is not created by my commit,
it is from commit (PM: Start asynchronous resume threads upfront), I just moved it ahead.

And like other phases, I added it into resum/suspend_noirq()...

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Change behaviour when tracing ... nasty trap (was Re: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace)
  2015-01-26 10:39   ` Liu, Chuansheng
@ 2015-01-26 11:05     ` Pavel Machek
  2015-01-26 13:41       ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ messages in thread
From: Pavel Machek @ 2015-01-26 11:05 UTC (permalink / raw)
  To: Liu, Chuansheng
  Cc: Fu, Zhonghui, Rafael J. Wysocki, Brown, Len, Greg Kroah-Hartman,
	linux-pm, linux-kernel

On Mon 2015-01-26 10:39:04, Liu, Chuansheng wrote:
> Hello Pavel,
> 
> > > There are some kind of dependency between devices in some
> > > hardware platforms. So, asynchronous resuming devices may
> > > hang system due to wrong resume order. As a result, should
> > > not fore synchronously resuming devices during tracing
> > > PM events.
> > >
> > > Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> > > ---
> > >  drivers/base/power/main.c    |    3 +--
> > >  include/linux/resume-trace.h |    7 -------
> > >  2 files changed, 1 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> > > index 9717d5f..5df148b 100644
> > > --- a/drivers/base/power/main.c
> > > +++ b/drivers/base/power/main.c
> > > @@ -517,8 +517,7 @@ static int device_resume_noirq(struct device *dev,
> > pm_message_t state, bool asyn
> > >
> > >  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;
> > >  }
> > >
> > 
> > Actually... whoever did the original patch was evil person. Changing
> > behaviour when tracing is requested is evil, evil, evil. Git blame
> > tells me
> > 
> > Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> > 
> > went to the dark side.
> 
> Although I didn't get where is something wrong, but the is_async() is not created by my commit,
> it is from commit (PM: Start asynchronous resume threads upfront), I just moved it ahead.
> 
> And like other phases, I added it into resum/suspend_noirq()...

I see, blame blamed wrong person. It looks like Rafael is evil:

commit 97df8c12995c5bac73e3bfeea4c5be155c1f4401
Author: Rafael J. Wysocki <rjw@sisk.pl>
Date:   Sat Jan 23 22:25:31 2010 +0100

    PM: Start asynchronous resume threads upfront

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Change behaviour when tracing ... nasty trap (was Re: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace)
  2015-01-26 11:05     ` Pavel Machek
@ 2015-01-26 13:41       ` Rafael J. Wysocki
  2015-01-26 13:43         ` Pavel Machek
  0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2015-01-26 13:41 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Liu, Chuansheng, Fu, Zhonghui, Brown, Len, Greg Kroah-Hartman,
	linux-pm, linux-kernel

On Monday, January 26, 2015 12:05:16 PM Pavel Machek wrote:
> On Mon 2015-01-26 10:39:04, Liu, Chuansheng wrote:
> > Hello Pavel,
> > 
> > > > There are some kind of dependency between devices in some
> > > > hardware platforms. So, asynchronous resuming devices may
> > > > hang system due to wrong resume order. As a result, should
> > > > not fore synchronously resuming devices during tracing
> > > > PM events.
> > > >
> > > > Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> > > > ---
> > > >  drivers/base/power/main.c    |    3 +--
> > > >  include/linux/resume-trace.h |    7 -------
> > > >  2 files changed, 1 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> > > > index 9717d5f..5df148b 100644
> > > > --- a/drivers/base/power/main.c
> > > > +++ b/drivers/base/power/main.c
> > > > @@ -517,8 +517,7 @@ static int device_resume_noirq(struct device *dev,
> > > pm_message_t state, bool asyn
> > > >
> > > >  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;
> > > >  }
> > > >
> > > 
> > > Actually... whoever did the original patch was evil person. Changing
> > > behaviour when tracing is requested is evil, evil, evil. Git blame
> > > tells me
> > > 
> > > Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> > > 
> > > went to the dark side.
> > 
> > Although I didn't get where is something wrong, but the is_async() is not created by my commit,
> > it is from commit (PM: Start asynchronous resume threads upfront), I just moved it ahead.
> > 
> > And like other phases, I added it into resum/suspend_noirq()...
> 
> I see, blame blamed wrong person. It looks like Rafael is evil:
> 
> commit 97df8c12995c5bac73e3bfeea4c5be155c1f4401
> Author: Rafael J. Wysocki <rjw@sisk.pl>
> Date:   Sat Jan 23 22:25:31 2010 +0100
> 
>     PM: Start asynchronous resume threads upfront

This only means we won't use asyc suspend/resume at all when the RTC-based
resume debug is enabled, because it wouldn't make sense (the RTC-based
debug requires strict ordering of callbacks between devices or we may find
that device A hanged the resume while actually device B that was running in
parallel with A did that).

And I shouldn't even need to explain this ...  Sad.

Rafael


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Change behaviour when tracing ... nasty trap (was Re: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace)
  2015-01-26 13:41       ` Rafael J. Wysocki
@ 2015-01-26 13:43         ` Pavel Machek
  2015-01-30  0:53           ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ messages in thread
From: Pavel Machek @ 2015-01-26 13:43 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Liu, Chuansheng, Fu, Zhonghui, Brown, Len, Greg Kroah-Hartman,
	linux-pm, linux-kernel, trivial

Document pm_tracing actually affecting suspend in non-trivial way.


Signed-off-by: Pavel Machek <pavel@ucw.cz>

---

On Mon 2015-01-26 14:41:02, Rafael J. Wysocki wrote:
> On Monday, January 26, 2015 12:05:16 PM Pavel Machek wrote:
> > On Mon 2015-01-26 10:39:04, Liu, Chuansheng wrote:

> > > > > @@ -517,8 +517,7 @@ static int device_resume_noirq(struct device *dev,
> > > > pm_message_t state, bool asyn
> > > > >
> > > > >  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;
> > > > >  }
> > > > >
> > > > 
> > > > Actually... whoever did the original patch was evil person. Changing
> > > > behaviour when tracing is requested is evil, evil, evil. Git blame
> > > > tells me
> > > > 
> > > > Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> > > > 
> > > > went to the dark side.
> > > 
> > > Although I didn't get where is something wrong, but the is_async() is not created by my commit,
> > > it is from commit (PM: Start asynchronous resume threads upfront), I just moved it ahead.
> > > 
> > > And like other phases, I added it into resum/suspend_noirq()...
> > 
> > I see, blame blamed wrong person. It looks like Rafael is evil:
> > 
> > commit 97df8c12995c5bac73e3bfeea4c5be155c1f4401
> > Author: Rafael J. Wysocki <rjw@sisk.pl>
> > Date:   Sat Jan 23 22:25:31 2010 +0100
> > 
> >     PM: Start asynchronous resume threads upfront
> 
> This only means we won't use asyc suspend/resume at all when the RTC-based
> resume debug is enabled, because it wouldn't make sense (the RTC-based
> debug requires strict ordering of callbacks between devices or we may find
> that device A hanged the resume while actually device B that was running in
> parallel with A did that).
> 
> And I shouldn't even need to explain this ...  Sad.

Well, I forgot that pm_trace_is_enabled() is the simple, RTC based
one, and believe it would be worth a comment...

diff --git a/Documentation/power/s2ram.txt b/Documentation/power/s2ram.txt
index 1bdfa04..4685aee 100644
--- a/Documentation/power/s2ram.txt
+++ b/Documentation/power/s2ram.txt
@@ -69,6 +69,10 @@ Reason for this is that the RTC is the only reliably available piece of
 hardware during resume operations where a value can be set that will
 survive a reboot.
 
+pm_trace is not compatible with asynchronous suspend, so it turns
+asynchronous suspend off (which may work around timing or
+ordering-sensitive bugs).
+
 Consequence is that after a resume (even if it is successful) your system
 clock will have a value corresponding to the magic number instead of the
 correct date/time! It is therefore advisable to use a program like ntp-date


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace
  2015-01-26  7:59 ` Fu, Zhonghui
@ 2015-01-26 13:44   ` Rafael J. Wysocki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2015-01-26 13:44 UTC (permalink / raw)
  To: Fu, Zhonghui; +Cc: len.brown, pavel, Greg Kroah-Hartman, linux-pm, linux-kernel

On Monday, January 26, 2015 03:59:59 PM Fu, Zhonghui wrote:
> 
> This is a incorrect patch , please ignore it.

Of course it is incorrect.

If it "fixes" anything for you, this means you need to reorder devices in dpm_list
on the given system.

> On 2015/1/26 13:07, Fu, Zhonghui wrote:
> > From f9c841d1f943d81b5ab0aac7483e794a7f966296 Mon Sep 17 00:00:00 2001
> > From: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> > Date: Mon, 26 Jan 2015 11:27:08 +0800
> > Subject: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace
> >
> > There are some kind of dependency between devices in some
> > hardware platforms. So, asynchronous resuming devices may
> > hang system due to wrong resume order. As a result, should
> > not fore synchronously resuming devices during tracing
> > PM events.
> >
> > Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> > ---
> >  drivers/base/power/main.c    |    3 +--
> >  include/linux/resume-trace.h |    7 -------
> >  2 files changed, 1 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> > index 9717d5f..5df148b 100644
> > --- a/drivers/base/power/main.c
> > +++ b/drivers/base/power/main.c
> > @@ -517,8 +517,7 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
> >  
> >  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)
> > diff --git a/include/linux/resume-trace.h b/include/linux/resume-trace.h
> > index f31db23..fd0866e 100644
> > --- a/include/linux/resume-trace.h
> > +++ b/include/linux/resume-trace.h
> > @@ -7,11 +7,6 @@
> >  
> >  extern int pm_trace_enabled;
> >  
> > -static inline int pm_trace_is_enabled(void)
> > -{
> > -       return pm_trace_enabled;
> > -}
> > -
> >  struct device;
> >  extern void set_trace_device(struct device *);
> >  extern void generate_resume_trace(const void *tracedata, unsigned int user);
> > @@ -24,8 +19,6 @@ extern int show_trace_dev_match(char *buf, size_t size);
> >  
> >  #else
> >  
> > -static inline int pm_trace_is_enabled(void) { return 0; }
> > -
> >  #define TRACE_DEVICE(dev) do { } while (0)
> >  #define TRACE_RESUME(dev) do { } while (0)
> >  
> > -- 1.7.1
> >
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Change behaviour when tracing ... nasty trap (was Re: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace)
  2015-01-26 13:43         ` Pavel Machek
@ 2015-01-30  0:53           ` Rafael J. Wysocki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2015-01-30  0:53 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Liu, Chuansheng, Fu, Zhonghui, Brown, Len, Greg Kroah-Hartman,
	linux-pm, linux-kernel, trivial

On Monday, January 26, 2015 02:43:04 PM Pavel Machek wrote:
> Document pm_tracing actually affecting suspend in non-trivial way.
> 
> 
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> ---
> 
> On Mon 2015-01-26 14:41:02, Rafael J. Wysocki wrote:
> > On Monday, January 26, 2015 12:05:16 PM Pavel Machek wrote:
> > > On Mon 2015-01-26 10:39:04, Liu, Chuansheng wrote:
> 
> > > > > > @@ -517,8 +517,7 @@ static int device_resume_noirq(struct device *dev,
> > > > > pm_message_t state, bool asyn
> > > > > >
> > > > > >  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;
> > > > > >  }
> > > > > >
> > > > > 
> > > > > Actually... whoever did the original patch was evil person. Changing
> > > > > behaviour when tracing is requested is evil, evil, evil. Git blame
> > > > > tells me
> > > > > 
> > > > > Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> > > > > 
> > > > > went to the dark side.
> > > > 
> > > > Although I didn't get where is something wrong, but the is_async() is not created by my commit,
> > > > it is from commit (PM: Start asynchronous resume threads upfront), I just moved it ahead.
> > > > 
> > > > And like other phases, I added it into resum/suspend_noirq()...
> > > 
> > > I see, blame blamed wrong person. It looks like Rafael is evil:
> > > 
> > > commit 97df8c12995c5bac73e3bfeea4c5be155c1f4401
> > > Author: Rafael J. Wysocki <rjw@sisk.pl>
> > > Date:   Sat Jan 23 22:25:31 2010 +0100
> > > 
> > >     PM: Start asynchronous resume threads upfront
> > 
> > This only means we won't use asyc suspend/resume at all when the RTC-based
> > resume debug is enabled, because it wouldn't make sense (the RTC-based
> > debug requires strict ordering of callbacks between devices or we may find
> > that device A hanged the resume while actually device B that was running in
> > parallel with A did that).
> > 
> > And I shouldn't even need to explain this ...  Sad.
> 
> Well, I forgot that pm_trace_is_enabled() is the simple, RTC based
> one, and believe it would be worth a comment...

A comment won't hurt. :-)

Applied, thanks!

> diff --git a/Documentation/power/s2ram.txt b/Documentation/power/s2ram.txt
> index 1bdfa04..4685aee 100644
> --- a/Documentation/power/s2ram.txt
> +++ b/Documentation/power/s2ram.txt
> @@ -69,6 +69,10 @@ Reason for this is that the RTC is the only reliably available piece of
>  hardware during resume operations where a value can be set that will
>  survive a reboot.
>  
> +pm_trace is not compatible with asynchronous suspend, so it turns
> +asynchronous suspend off (which may work around timing or
> +ordering-sensitive bugs).
> +
>  Consequence is that after a resume (even if it is successful) your system
>  clock will have a value corresponding to the magic number instead of the
>  correct date/time! It is therefore advisable to use a program like ntp-date
> 
> 
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-01-30  0:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-26  5:07 [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace Fu, Zhonghui
2015-01-26  7:59 ` Fu, Zhonghui
2015-01-26 13:44   ` Rafael J. Wysocki
2015-01-26 10:23 ` Change behaviour when tracing ... nasty trap (was Re: [PATCH] PM/Trace: get rid of synchronous resume limit during PM trace) Pavel Machek
2015-01-26 10:39   ` Liu, Chuansheng
2015-01-26 11:05     ` Pavel Machek
2015-01-26 13:41       ` Rafael J. Wysocki
2015-01-26 13:43         ` Pavel Machek
2015-01-30  0:53           ` Rafael J. Wysocki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.