Linux Power Management development
 help / color / mirror / Atom feed
* Re: [PATCH] PM: add statistics sysfs file for suspend to ram
From: Pavel Machek @ 2011-08-10 22:27 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Liu, ShuoX, yanmin_zhang, Greg KH,
	linux-pm@lists.linux-foundation.org, Brown, Len
In-Reply-To: <201108090009.42342.rjw@sisk.pl>

On Tue 2011-08-09 00:09:42, Rafael J. Wysocki wrote:
> On Monday, August 08, 2011, Pavel Machek wrote:
> > Hi!
> > 
> > > > > > > Thanks for the nice pointer. I checked dynamic debug. It's really a good debug tool.
> > > > > > > With the dynamic debug:
> > > > > > > 1) user need write a user space parser to process the syslog output;
> > > > > > > 2) Our testing scenario is we leave the mobile for at least hours. Then, check its status.
> > > > > > > No serial console available during the testing. One is because console would be suspended,
> > > > > > > and the other is serial console connecting with spi or HSU devices would consume power. These
> > > > > > > devices are powered off at suspend-2-ram.
> > ...
> > > Not in the case described by Yanmin.
> > 
> > Really? I see the description above.
> > 
> > Yes, they'd need to set up syslog to only log >= KERN_ERR, then parse
> > the (small) results. Even hours worth of suspends should not cause
> > _that_ many errors.
> > 
> > Serial console is irrelevant. You need live machine to dump dmesg, but
> > again, you need live machine to access debugfs, too.
> 
> This sounds like substantial overhead to collect statistics that we can
> collect at a much lower cost in the kernel.

That's always the case, right? Everyone wants different subsets of
syslog, and doing selection in kernel is always lowest overhead.

> The patch isn't very intrusive and rather straightforward.

No, it is not _too_ bad; but

1) the code stays there when not debugging

2) different users want different syslog subsets. Putting all the
"interesting" subsets into /sys/debug/my_syslog_subset just does not
seem like a way to go.

(If anything simpler could be done to help debugging, like generating
udev event on each failed suspend, so that can trigger their
processing, I guess that would be acceptable.)

									Pavel

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

^ permalink raw reply

* Re: [PATCH v4] PM: add statistics debugfs file for suspend to ram
From: Pavel Machek @ 2011-08-10 21:44 UTC (permalink / raw)
  To: Liu, ShuoX
  Cc: Brown, Len, Yanmin_zhang@linux.intel.com, Greg KH,
	linux-kernel@vger.kernel.org, linux-pm@lists.linux-foundation.org
In-Reply-To: <6E3BC7F7C9A4BF4286DD4C043110F30B5B847F42F9@shsmsx502.ccr.corp.intel.com>

Hi!

> If not using this patch, we have other methods to get info about
> what device fails. One is to turn on  CONFIG_PM_DEBUG, but users
> would get too much info and testers need recompile the system.
> 
> In addition, dynamic debug is another good tool to dump debug info.
> But it still doesn't match our utilization scenario closely.
> 1) user need write a user space parser to process the syslog output;
> 2) Our testing scenario is we leave the mobile for at least hours.
>    Then, check its status. No serial console available during the
>    testing. One is because console would be suspended, and the other
>    is serial console connecting with spi or HSU devices would consume
>    power. These devices are powered off at suspend-2-ram.

1) yes you need parser

1a) Yes, you need CONFIG_PM_DEBUG; but that's better than forcing 200
lines of pure debugging code onto everyone

2)  You can do that. Just check and parse dmesg from userland after
each resume.

If dmesg provides too little/too much information, improve loglevels
so that it is not spammed.
 
								Pavel


> Signed-off-by: ShuoX Liu <shuox.liu@intel.com>
> ---
>  Documentation/power/basic-pm-debugging.txt |   24 +++++++
>  drivers/base/power/main.c                  |   31 +++++++--
>  include/linux/suspend.h                    |   52 ++++++++++++++
>  kernel/power/main.c                        |  102 ++++++++++++++++++++++++++++
>  kernel/power/suspend.c                     |   17 ++++-
>  5 files changed, 218 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/power/basic-pm-debugging.txt b/Documentation/power/basic-pm-debugging.txt
> index ddd7817..62eca08 100644
> --- a/Documentation/power/basic-pm-debugging.txt
> +++ b/Documentation/power/basic-pm-debugging.txt
> @@ -201,3 +201,27 @@ case, you may be able to search for failing drivers by following the procedure
>  analogous to the one described in section 1.  If you find some failing drivers,
>  you will have to unload them every time before an STR transition (ie. before
>  you run s2ram), and please report the problems with them.
> +
> +There is a debugfs entry which shows the suspend to RAM statistics. Here is an
> +example of its output.
> +	# mount -t debugfs none /sys/kernel/debug
> +	# cat /sys/kernel/debug/suspend_stats
> +	success: 20
> +	fail: 5
> +	failed_freeze: 0
> +	failed_prepare: 0
> +	failed_suspend: 5
> +	failed_suspend_noirq: 0
> +	failed_resume: 0
> +	failed_resume_noirq: 0
> +	failures:
> +	  last_failed_dev:	alarm
> +				adc
> +	  last_failed_errno:	-16
> +				-16
> +	  last_failed_step:	suspend
> +				suspend
> +Field success means the success number of suspend to RAM, and field fail means
> +the failure number. Others are the failure number of different steps of suspend
> +to RAM. suspend_stats just lists the last 2 failed devices, error number and
> +failed step of suspend.

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

^ permalink raw reply

* Re: PM / hibernate xfs lock up / xfs_reclaim_inodes_ag
From: Pavel Machek @ 2011-08-10 21:43 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Christoph, Linux PM mailing list, Dave Chinner, xfs
In-Reply-To: <201107271135.13297.rjw@sisk.pl>

Hi!

> > > Why don't you simply submit a patch to do that?
> > 
> > a) I don't know how to test suspend/hibernate
> > b) I don't have any hardware I can test it on.

hibernation should work on any hardware. Just add cmdline parameter
resume=<your swap> and use echo disk > /sys/power/state.

suspend is supported on most PCs, and certainly on any notebook. Just
echo mem > /sys/power/state.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH v5 0/3] DEVFREQ, DVFS Framework for Non-CPU Devices.
From: Rafael J. Wysocki @ 2011-08-10 21:09 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm,
	Thomas Gleixner
In-Reply-To: <1312794188-9823-1-git-send-email-myungjoo.ham@samsung.com>

On Monday, August 08, 2011, MyungJoo Ham wrote:
> For a usage example, please look at
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq
> 
> In the above git tree, DVFS (dynamic voltage and frequency scaling) mechanism
> is applied to the memory bus of Exynos4210 for Exynos4210-NURI boards.
> In the example, the LPDDR2 DRAM frequency changes between 133, 266, and 400MHz
> and other related clocks simply follow the determined DDR RAM clock.
> 
> The DEVFREQ driver for Exynos4210 memory bus is at
> /arch/arm/mach-exynos4/devfreq_bus.c in the git tree.
> 
> In the dd (writing and reading 360MiB) test with NURI board, the memory
> throughput was not changed (the performance is not deteriorated) while
> the SoC power consumption has been reduced by 1%. When the memory access
> is not that intense while the CPU is heavily used, the SoC power consumption
> has been reduced by 6%. The power consumption has been compared with the
> case using the conventional Exynos4210 CPUFREQ driver, which sets memory
> bus frequency according to the CPU core frequency. Besides, when the CPU core
> running slow and the memory access is intense, the performance (memory
> throughput) has been increased by 11% (with higher SoC power consumption of
> 5%). The tested governor is "simple-ondemand".
> 
> MyungJoo Ham (3):
>   PM: Introduce DEVFREQ: generic DVFS framework with device-specific
>     OPPs
>   PM / DEVFREQ: add basic governors
>   PM / DEVFREQ: add sysfs interface
> 
>  Documentation/ABI/testing/sysfs-devices-power |   45 ++
>  drivers/base/power/Makefile                   |    1 +
>  drivers/base/power/devfreq.c                  |  555 +++++++++++++++++++++++++
>  drivers/base/power/opp.c                      |    9 +
>  include/linux/devfreq.h                       |  111 +++++
>  kernel/power/Kconfig                          |   34 ++
>  6 files changed, 755 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/base/power/devfreq.c
>  create mode 100644 include/linux/devfreq.h

Are there still any objections to this version of the patchset?

Rafael

^ permalink raw reply

* Re: [PATCH v4] PM: add statistics debugfs file for suspend to ram
From: Rafael J. Wysocki @ 2011-08-10 21:07 UTC (permalink / raw)
  To: Liu, ShuoX
  Cc: Brown, Len, Yanmin_zhang@linux.intel.com, Greg KH,
	linux-kernel@vger.kernel.org, linux-pm@lists.linux-foundation.org
In-Reply-To: <6E3BC7F7C9A4BF4286DD4C043110F30B5B847F42F9@shsmsx502.ccr.corp.intel.com>

On Wednesday, August 10, 2011, Liu, ShuoX wrote:
> From: ShuoX Liu <shuox.liu@intel.com>
> 
> Record S3 failure time about each reason and the latest two failed
> devices' names in S3 progress.
> We can check it through 'suspend_stats' entry in debugfs.
> 
> The motivation of the patch:
> 
> We are enabling power features on Medfield. Comparing with PC/notebook,
> a mobile enters/exits suspend-2-ram (we call it s3 on Medfield) far
> more frequently. If it can't enter suspend-2-ram in time, the power
> might be used up soon.
> 
> We often find sometimes, a device suspend fails. Then, system retries
> s3 over and over again. As display is off, testers and developers
> don't know what happens.
> 
> Some testers and developers complain they don't know if system
> tries suspend-2-ram, and what device fails to suspend. They need
> such info for a quick check. The patch adds suspend_stats under
> debugfs for users to check suspend to RAM statistics quickly.
> 
> If not using this patch, we have other methods to get info about
> what device fails. One is to turn on  CONFIG_PM_DEBUG, but users
> would get too much info and testers need recompile the system.
> 
> In addition, dynamic debug is another good tool to dump debug info.
> But it still doesn't match our utilization scenario closely.
> 1) user need write a user space parser to process the syslog output;
> 2) Our testing scenario is we leave the mobile for at least hours.
>    Then, check its status. No serial console available during the
>    testing. One is because console would be suspended, and the other
>    is serial console connecting with spi or HSU devices would consume
>    power. These devices are powered off at suspend-2-ram.
> 
> Thank Greg, MyungJoo and Rafael for their great comments.
> 
> Change Log:
>   V4: Improve output format.
>   V3: Change some programming styles.
>   V2:
> 	1) Change the sysfs entry to debugfs.
> 	2) Add some explanation in document file.
> 
> Contribution:
> 	yanmin_zhang@linux.intel.com
> 
> Signed-off-by: ShuoX Liu <shuox.liu@intel.com>

Applied to linux-pm/linux-next as 3.2 material.

Thanks,
Rafael


> ---
>  Documentation/power/basic-pm-debugging.txt |   24 +++++++
>  drivers/base/power/main.c                  |   31 +++++++--
>  include/linux/suspend.h                    |   52 ++++++++++++++
>  kernel/power/main.c                        |  102 ++++++++++++++++++++++++++++
>  kernel/power/suspend.c                     |   17 ++++-
>  5 files changed, 218 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/power/basic-pm-debugging.txt b/Documentation/power/basic-pm-debugging.txt
> index ddd7817..62eca08 100644
> --- a/Documentation/power/basic-pm-debugging.txt
> +++ b/Documentation/power/basic-pm-debugging.txt
> @@ -201,3 +201,27 @@ case, you may be able to search for failing drivers by following the procedure
>  analogous to the one described in section 1.  If you find some failing drivers,
>  you will have to unload them every time before an STR transition (ie. before
>  you run s2ram), and please report the problems with them.
> +
> +There is a debugfs entry which shows the suspend to RAM statistics. Here is an
> +example of its output.
> +	# mount -t debugfs none /sys/kernel/debug
> +	# cat /sys/kernel/debug/suspend_stats
> +	success: 20
> +	fail: 5
> +	failed_freeze: 0
> +	failed_prepare: 0
> +	failed_suspend: 5
> +	failed_suspend_noirq: 0
> +	failed_resume: 0
> +	failed_resume_noirq: 0
> +	failures:
> +	  last_failed_dev:	alarm
> +				adc
> +	  last_failed_errno:	-16
> +				-16
> +	  last_failed_step:	suspend
> +				suspend
> +Field success means the success number of suspend to RAM, and field fail means
> +the failure number. Others are the failure number of different steps of suspend
> +to RAM. suspend_stats just lists the last 2 failed devices, error number and
> +failed step of suspend.
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index a854591..23b755d 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -46,6 +46,7 @@ LIST_HEAD(dpm_prepared_list);
>  LIST_HEAD(dpm_suspended_list);
>  LIST_HEAD(dpm_noirq_list);
>  
> +struct suspend_stats suspend_stats;
>  static DEFINE_MUTEX(dpm_list_mtx);
>  static pm_message_t pm_transition;
>  
> @@ -464,8 +465,12 @@ void dpm_resume_noirq(pm_message_t state)
>  		mutex_unlock(&dpm_list_mtx);
>  
>  		error = device_resume_noirq(dev, state);
> -		if (error)
> +		if (error) {
> +			suspend_stats.failed_resume_noirq++;
> +			dpm_save_failed_step(SUSPEND_RESUME_NOIRQ);
> +			dpm_save_failed_dev(dev_name(dev));
>  			pm_dev_err(dev, state, " early", error);
> +		}
>  
>  		mutex_lock(&dpm_list_mtx);
>  		put_device(dev);
> @@ -626,8 +631,12 @@ void dpm_resume(pm_message_t state)
>  			mutex_unlock(&dpm_list_mtx);
>  
>  			error = device_resume(dev, state, false);
> -			if (error)
> +			if (error) {
> +				suspend_stats.failed_resume++;
> +				dpm_save_failed_step(SUSPEND_RESUME);
> +				dpm_save_failed_dev(dev_name(dev));
>  				pm_dev_err(dev, state, "", error);
> +			}
>  
>  			mutex_lock(&dpm_list_mtx);
>  		}
> @@ -802,6 +811,9 @@ int dpm_suspend_noirq(pm_message_t state)
>  		mutex_lock(&dpm_list_mtx);
>  		if (error) {
>  			pm_dev_err(dev, state, " late", error);
> +			suspend_stats.failed_suspend_noirq++;
> +			dpm_save_failed_step(SUSPEND_SUSPEND_NOIRQ);
> +			dpm_save_failed_dev(dev_name(dev));
>  			put_device(dev);
>  			break;
>  		}
> @@ -923,8 +935,10 @@ static void async_suspend(void *data, async_cookie_t cookie)
>  	int error;
>  
>  	error = __device_suspend(dev, pm_transition, true);
> -	if (error)
> +	if (error) {
> +		dpm_save_failed_dev(dev_name(dev));
>  		pm_dev_err(dev, pm_transition, " async", error);
> +	}
>  
>  	put_device(dev);
>  }
> @@ -967,6 +981,7 @@ int dpm_suspend(pm_message_t state)
>  		mutex_lock(&dpm_list_mtx);
>  		if (error) {
>  			pm_dev_err(dev, state, "", error);
> +			dpm_save_failed_dev(dev_name(dev));
>  			put_device(dev);
>  			break;
>  		}
> @@ -980,7 +995,10 @@ int dpm_suspend(pm_message_t state)
>  	async_synchronize_full();
>  	if (!error)
>  		error = async_error;
> -	if (!error)
> +	if (error) {
> +		suspend_stats.failed_suspend++;
> +		dpm_save_failed_step(SUSPEND_SUSPEND);
> +	} else
>  		dpm_show_time(starttime, state, NULL);
>  	return error;
>  }
> @@ -1088,7 +1106,10 @@ int dpm_suspend_start(pm_message_t state)
>  	int error;
>  
>  	error = dpm_prepare(state);
> -	if (!error)
> +	if (error) {
> +		suspend_stats.failed_prepare++;
> +		dpm_save_failed_step(SUSPEND_PREPARE);
> +	} else
>  		error = dpm_suspend(state);
>  	return error;
>  }
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 6bbcef2..76f42e4 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -34,6 +34,58 @@ typedef int __bitwise suspend_state_t;
>  #define PM_SUSPEND_MEM		((__force suspend_state_t) 3)
>  #define PM_SUSPEND_MAX		((__force suspend_state_t) 4)
>  
> +enum suspend_stat_step {
> +	SUSPEND_FREEZE = 1,
> +	SUSPEND_PREPARE,
> +	SUSPEND_SUSPEND,
> +	SUSPEND_SUSPEND_NOIRQ,
> +	SUSPEND_RESUME_NOIRQ,
> +	SUSPEND_RESUME
> +};
> +
> +struct suspend_stats {
> +	int	success;
> +	int	fail;
> +	int	failed_freeze;
> +	int	failed_prepare;
> +	int	failed_suspend;
> +	int	failed_suspend_noirq;
> +	int	failed_resume;
> +	int	failed_resume_noirq;
> +#define	REC_FAILED_NUM	2
> +	int	last_failed_dev;
> +	char	failed_devs[REC_FAILED_NUM][40];
> +	int	last_failed_errno;
> +	int	errno[REC_FAILED_NUM];
> +	int	last_failed_step;
> +	enum suspend_stat_step	failed_steps[REC_FAILED_NUM];
> +};
> +
> +extern struct suspend_stats suspend_stats;
> +
> +static inline void dpm_save_failed_dev(const char *name)
> +{
> +	strlcpy(suspend_stats.failed_devs[suspend_stats.last_failed_dev],
> +		name,
> +		sizeof(suspend_stats.failed_devs[0]));
> +	suspend_stats.last_failed_dev++;
> +	suspend_stats.last_failed_dev %= REC_FAILED_NUM;
> +}
> +
> +static inline void dpm_save_failed_errno(int err)
> +{
> +	suspend_stats.errno[suspend_stats.last_failed_errno] = err;
> +	suspend_stats.last_failed_errno++;
> +	suspend_stats.last_failed_errno %= REC_FAILED_NUM;
> +}
> +
> +static inline void dpm_save_failed_step(enum suspend_stat_step step)
> +{
> +	suspend_stats.failed_steps[suspend_stats.last_failed_step] = step;
> +	suspend_stats.last_failed_step++;
> +	suspend_stats.last_failed_step %= REC_FAILED_NUM;
> +}
> +
>  /**
>   * struct platform_suspend_ops - Callbacks for managing platform dependent
>   *	system sleep states.
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 6c601f8..2757acb 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -12,6 +12,8 @@
>  #include <linux/string.h>
>  #include <linux/resume-trace.h>
>  #include <linux/workqueue.h>
> +#include <linux/debugfs.h>
> +#include <linux/seq_file.h>
>  
>  #include "power.h"
>  
> @@ -133,6 +135,101 @@ power_attr(pm_test);
>  
>  #endif /* CONFIG_PM_SLEEP */
>  
> +#ifdef CONFIG_DEBUG_FS
> +static char *suspend_step_name(enum suspend_stat_step step)
> +{
> +	switch (step) {
> +	case SUSPEND_FREEZE:
> +		return "freeze";
> +	case SUSPEND_PREPARE:
> +		return "prepare";
> +	case SUSPEND_SUSPEND:
> +		return "suspend";
> +	case SUSPEND_SUSPEND_NOIRQ:
> +		return "suspend_noirq";
> +	case SUSPEND_RESUME_NOIRQ:
> +		return "resume_noirq";
> +	case SUSPEND_RESUME:
> +		return "resume";
> +	default:
> +		return "";
> +	}
> +}
> +
> +static int suspend_stats_show(struct seq_file *s, void *unused)
> +{
> +	int i, index, last_dev, last_errno, last_step;
> +
> +	last_dev = suspend_stats.last_failed_dev + REC_FAILED_NUM - 1;
> +	last_dev %= REC_FAILED_NUM;
> +	last_errno = suspend_stats.last_failed_errno + REC_FAILED_NUM - 1;
> +	last_errno %= REC_FAILED_NUM;
> +	last_step = suspend_stats.last_failed_step + REC_FAILED_NUM - 1;
> +	last_step %= REC_FAILED_NUM;
> +	seq_printf(s, "%s: %d\n%s: %d\n%s: %d\n%s: %d\n"
> +			"%s: %d\n%s: %d\n%s: %d\n%s: %d\n",
> +			"success", suspend_stats.success,
> +			"fail", suspend_stats.fail,
> +			"failed_freeze", suspend_stats.failed_freeze,
> +			"failed_prepare", suspend_stats.failed_prepare,
> +			"failed_suspend", suspend_stats.failed_suspend,
> +			"failed_suspend_noirq",
> +				suspend_stats.failed_suspend_noirq,
> +			"failed_resume", suspend_stats.failed_resume,
> +			"failed_resume_noirq",
> +				suspend_stats.failed_resume_noirq);
> +	seq_printf(s,	"failures:\n  last_failed_dev:\t%-s\n",
> +			suspend_stats.failed_devs[last_dev]);
> +	for (i = 1; i < REC_FAILED_NUM; i++) {
> +		index = last_dev + REC_FAILED_NUM - i;
> +		index %= REC_FAILED_NUM;
> +		seq_printf(s, "\t\t\t%-s\n",
> +			suspend_stats.failed_devs[index]);
> +	}
> +	seq_printf(s,	"  last_failed_errno:\t%-d\n",
> +			suspend_stats.errno[last_errno]);
> +	for (i = 1; i < REC_FAILED_NUM; i++) {
> +		index = last_errno + REC_FAILED_NUM - i;
> +		index %= REC_FAILED_NUM;
> +		seq_printf(s, "\t\t\t%-d\n",
> +			suspend_stats.errno[index]);
> +	}
> +	seq_printf(s,	"  last_failed_step:\t%-s\n",
> +			suspend_step_name(
> +				suspend_stats.failed_steps[last_step]));
> +	for (i = 1; i < REC_FAILED_NUM; i++) {
> +		index = last_step + REC_FAILED_NUM - i;
> +		index %= REC_FAILED_NUM;
> +		seq_printf(s, "\t\t\t%-s\n",
> +			suspend_step_name(
> +				suspend_stats.failed_steps[index]));
> +	}
> +
> +	return 0;
> +}
> +
> +static int suspend_stats_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, suspend_stats_show, NULL);
> +}
> +
> +static const struct file_operations suspend_stats_operations = {
> +	.open           = suspend_stats_open,
> +	.read           = seq_read,
> +	.llseek         = seq_lseek,
> +	.release        = single_release,
> +};
> +
> +static int __init pm_debugfs_init(void)
> +{
> +	debugfs_create_file("suspend_stats", S_IFREG | S_IRUGO,
> +			NULL, NULL, &suspend_stats_operations);
> +	return 0;
> +}
> +
> +late_initcall(pm_debugfs_init);
> +#endif /* CONFIG_DEBUG_FS */
> +
>  struct kobject *power_kobj;
>  
>  /**
> @@ -194,6 +291,11 @@ static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr,
>  	}
>  	if (state < PM_SUSPEND_MAX && *s)
>  		error = enter_state(state);
> +		if (error) {
> +			suspend_stats.fail++;
> +			dpm_save_failed_errno(error);
> +		} else
> +			suspend_stats.success++;
>  #endif
>  
>   Exit:
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index b6b71ad..595a3dd 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -104,7 +104,10 @@ static int suspend_prepare(void)
>  		goto Finish;
>  
>  	error = suspend_freeze_processes();
> -	if (!error)
> +	if (error) {
> +		suspend_stats.failed_freeze++;
> +		dpm_save_failed_step(SUSPEND_FREEZE);
> +	} else
>  		return 0;
>  
>  	suspend_thaw_processes();
> @@ -315,8 +318,16 @@ int enter_state(suspend_state_t state)
>   */
>  int pm_suspend(suspend_state_t state)
>  {
> -	if (state > PM_SUSPEND_ON && state <= PM_SUSPEND_MAX)
> -		return enter_state(state);
> +	int ret;
> +	if (state > PM_SUSPEND_ON && state <= PM_SUSPEND_MAX) {
> +		ret = enter_state(state);
> +		if (ret) {
> +			suspend_stats.fail++;
> +			dpm_save_failed_errno(ret);
> +		} else
> +			suspend_stats.success++;
> +		return ret;
> +	}
>  	return -EINVAL;
>  }
>  EXPORT_SYMBOL(pm_suspend);
> 

^ permalink raw reply

* Re: [PATCH v2 2/2] cpu: update cpu_hotpluggable_mask in register_cpu
From: Daniel Lezcano @ 2011-08-10 20:54 UTC (permalink / raw)
  To: Mike Turquette; +Cc: linux-pm, gregkh, linaro-dev, linux-kernel, patches
In-Reply-To: <1313006614-28702-3-git-send-email-mturquette@ti.com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/10/2011 10:03 PM, Mike Turquette wrote:
> Update the cpu_hotpluggable_mask for each registered CPU which supports
> hotplug.  This makes it trivial for kernel code to know which CPUs
> support hotplug operations.
> 
> Signed-off-by: Mike Turquette <mturquette@ti.com>

Reviewed-by: <Daniel Lezcano> daniel.lezcano@linaro.org

- -- 
 <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJOQvAUAAoJEAKBbMCpUGYADTYH/3jGeK5pryiVKaxQ7wUBwsG6
7ugSLCCgBVUL6ueuVmlphhfW8Vv9Uz72LlJnaJUmJ0deK/Rf5x1ccFulBTtIsyjK
Zbw4Wg0MYxfNo0OAi7OSjWMUw31PC8kKVBSUZd3zHeqi6TanKUtxR5im4EWGhQbc
+Njhx2jAqZBEEcPnODiRx+CbYJX17N2Cesmk8Z+Tk00LIpM30iGGX15ZWXnV6KWM
zD3vG7ATLa/rv1/xPa67SDXdZIo99wDodjtjTFPClygM/XTSNBiIxsm9LnnUkmyw
gw1Q6Ng+wo2XyhxJ//b+kM0natnc7Tl6RtDIL7T+j9rIAdf8Sz9DPgmafWQRxYY=
=hwkF
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [PATCH v2 1/2] cpumask: introduce cpumask for hotpluggable CPUs
From: Daniel Lezcano @ 2011-08-10 20:53 UTC (permalink / raw)
  To: Mike Turquette; +Cc: linux-pm, gregkh, linaro-dev, linux-kernel, patches
In-Reply-To: <1313006614-28702-2-git-send-email-mturquette@ti.com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/10/2011 10:03 PM, Mike Turquette wrote:
> On some platforms it is possible to have some CPUs which support CPU
> hotplug and some which do not.  Currently the prescence of an 'online'
> sysfs entry in userspace is adequate for applications to know that a CPU
> supports hotplug, but there is no convenient way to make the same
> determination in the kernel.
> 
> To better model this relationship this patch introduces a new cpumask to
> track CPUs that support CPU hotplug operations.
> 
> This new cpumask is populated at boot-time and remains static for the
> life of the machine.  Bits set in the mask indicate a CPU which supports
> hotplug, but make no guarantees about whether that CPU is currently
> online or not.  Likewise a cleared bit in the mask indicates either a
> CPU which cannot hotplug or a lack of a populated CPU.
> 
> The purpose of this new cpumask is to aid kernel code which uses CPU to
> take CPUs online and offline.  Possible uses are as a thermal event
> mitigation technique or as a power capping mechanism.
> 
> Signed-off-by: Mike Turquette <mturquette@ti.com>

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

- -- 
 <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJOQu/KAAoJEAKBbMCpUGYAxnoH/0+WtrRGq0oOjkU8jIxzAEUP
OW6S7EQT13v97YapaEwUZpHOzrdeEa9bV5sVI3uc3H9n6dnQbh25eMuhiH/dTwF6
LHV1Tdv7/+ghiSc4NJunpXjAsObezUgTV9n2Zoip3kAfhMdV5DntJu0Izkdb5lkA
AD5CqV6oHqC0spa2/nMMeOYsdp3hLP3hA5wCyE8a2rxvbytxl2jRVommXOpF3AnI
s3OS7oyVFLpdvxVdAio1cXvM6vILtJFArAqw88AJUBsUsuehRxeOYVvTR3/z4RZD
LNEH72IkGY/8OVYp9/VoixWgWTUhXUR5BZwAjCP7a+xQk/1qK1LAM+/CXimwND0=
=90gJ
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [PATCH v4] PM: add statistics debugfs file for suspend to ram
From: Mansoor, Illyas @ 2011-08-10 20:25 UTC (permalink / raw)
  To: Mansoor, Illyas, Greg KH
  Cc: Liu, ShuoX, linux-pm@lists.linux-foundation.org,
	Yanmin_zhang@linux.intel.com, Brown, Len,
	linux-kernel@vger.kernel.org
In-Reply-To: <EB6073F1DF78F9439DFDCC701BC6466007B36A0D14@bgsmsx502.gar.corp.intel.com>

> 
> > On Thu, Aug 11, 2011 at 01:22:56AM +0530, Mansoor, Illyas wrote:
> > > static pm_message_t pm_transition;
> > > > > >
> > > > > > @@ -464,8 +465,12 @@ void dpm_resume_noirq(pm_message_t state)
> > > > > >  		mutex_unlock(&dpm_list_mtx);
> > > > > >
> > > > > >  		error = device_resume_noirq(dev, state);
> > > > > > -		if (error)
> > > > > > +		if (error) {
> > > > > > +			suspend_stats.failed_resume_noirq++;
> > > > > > +			dpm_save_failed_step(SUSPEND_RESUME_NOIRQ);
> > > > > > +			dpm_save_failed_dev(dev_name(dev));
> > > > >
> > > > > Please make these statistics conditionally enabled, so on a production
> > system
> > > > > If we need to disable these statistics code we should be able to do so.
> > > >
> > > > Why, are they taking time or space that is needed for something else?
> > > > What's the downside here of just not always having this enabled?
> > >
> > > Why have something that is not required/Used?
> >
> > Because someone might need it and rebuilding a kernel isn't possible on
> > lots of devices.
> 
> Agreed.
> 
> >
> > > Its only useful if DEBUGFS is configured anyways
> >
> > Almost all systems these days have debugfs enabled, so that's a moot
> > point.
> 
> We could do the same for this as well, since DEBUGFS is still a compile time
> Option even thou many enable it by default we don't make it part of the kernel
> isn't it.
> 

May be it's not worth debating on this one, I'm okay if it's part of the kernel as well.

-Illyas

^ permalink raw reply

* Re: [PATCH v4] PM: add statistics debugfs file for suspend to ram
From: Mansoor, Illyas @ 2011-08-10 20:20 UTC (permalink / raw)
  To: Greg KH
  Cc: Liu, ShuoX, linux-pm@lists.linux-foundation.org,
	Yanmin_zhang@linux.intel.com, linux-kernel@vger.kernel.org,
	Brown, Len
In-Reply-To: <20110810195854.GA5939@suse.de>

> On Thu, Aug 11, 2011 at 01:22:56AM +0530, Mansoor, Illyas wrote:
> > static pm_message_t pm_transition;
> > > > >
> > > > > @@ -464,8 +465,12 @@ void dpm_resume_noirq(pm_message_t state)
> > > > >  		mutex_unlock(&dpm_list_mtx);
> > > > >
> > > > >  		error = device_resume_noirq(dev, state);
> > > > > -		if (error)
> > > > > +		if (error) {
> > > > > +			suspend_stats.failed_resume_noirq++;
> > > > > +			dpm_save_failed_step(SUSPEND_RESUME_NOIRQ);
> > > > > +			dpm_save_failed_dev(dev_name(dev));
> > > >
> > > > Please make these statistics conditionally enabled, so on a production
> system
> > > > If we need to disable these statistics code we should be able to do so.
> > >
> > > Why, are they taking time or space that is needed for something else?
> > > What's the downside here of just not always having this enabled?
> >
> > Why have something that is not required/Used?
> 
> Because someone might need it and rebuilding a kernel isn't possible on
> lots of devices.

Agreed.

> 
> > Its only useful if DEBUGFS is configured anyways
> 
> Almost all systems these days have debugfs enabled, so that's a moot
> point.

We could do the same for this as well, since DEBUGFS is still a compile time
Option even thou many enable it by default we don't make it part of the kernel isn't it.

-Illyas

^ permalink raw reply

* [PATCH v2 2/2] cpu: update cpu_hotpluggable_mask in register_cpu
From: Mike Turquette @ 2011-08-10 20:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-pm, gregkh, linaro-dev, patches
In-Reply-To: <1313006614-28702-1-git-send-email-mturquette@ti.com>

Update the cpu_hotpluggable_mask for each registered CPU which supports
hotplug.  This makes it trivial for kernel code to know which CPUs
support hotplug operations.

Signed-off-by: Mike Turquette <mturquette@ti.com>
---
Change log:
v2: no change

 drivers/base/cpu.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 251acea..91ddcf8 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -224,8 +224,10 @@ int __cpuinit register_cpu(struct cpu *cpu, int num)
 
 	error = sysdev_register(&cpu->sysdev);
 
-	if (!error && cpu->hotpluggable)
+	if (!error && cpu->hotpluggable) {
 		register_cpu_control(cpu);
+		set_cpu_hotpluggable(num, true);
+	}
 	if (!error)
 		per_cpu(cpu_sys_devices, num) = &cpu->sysdev;
 	if (!error)
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH v2 1/2] cpumask: introduce cpumask for hotpluggable CPUs
From: Mike Turquette @ 2011-08-10 20:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-pm, gregkh, linaro-dev, patches
In-Reply-To: <1313006614-28702-1-git-send-email-mturquette@ti.com>

On some platforms it is possible to have some CPUs which support CPU
hotplug and some which do not.  Currently the prescence of an 'online'
sysfs entry in userspace is adequate for applications to know that a CPU
supports hotplug, but there is no convenient way to make the same
determination in the kernel.

To better model this relationship this patch introduces a new cpumask to
track CPUs that support CPU hotplug operations.

This new cpumask is populated at boot-time and remains static for the
life of the machine.  Bits set in the mask indicate a CPU which supports
hotplug, but make no guarantees about whether that CPU is currently
online or not.  Likewise a cleared bit in the mask indicates either a
CPU which cannot hotplug or a lack of a populated CPU.

The purpose of this new cpumask is to aid kernel code which uses CPU to
take CPUs online and offline.  Possible uses are as a thermal event
mitigation technique or as a power capping mechanism.

Signed-off-by: Mike Turquette <mturquette@ti.com>
---
Change log:
v2: fixed missing parentheses in cpumask_test_cpu and improved grammar
    in comments

 include/linux/cpumask.h |   27 ++++++++++++++++++++++-----
 kernel/cpu.c            |   18 ++++++++++++++++++
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index b24ac56..52e64a7 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -39,10 +39,11 @@ extern int nr_cpu_ids;
  * The following particular system cpumasks and operations manage
  * possible, present, active and online cpus.
  *
- *     cpu_possible_mask- has bit 'cpu' set iff cpu is populatable
- *     cpu_present_mask - has bit 'cpu' set iff cpu is populated
- *     cpu_online_mask  - has bit 'cpu' set iff cpu available to scheduler
- *     cpu_active_mask  - has bit 'cpu' set iff cpu available to migration
+ *     cpu_possible_mask     - has bit 'cpu' set iff cpu is populatable
+ *     cpu_hotpluggable_mask - has bit 'cpu' set iff cpu is hotpluggable
+ *     cpu_present_mask      - has bit 'cpu' set iff cpu is populated
+ *     cpu_online_mask       - has bit 'cpu' set iff cpu available to scheduler
+ *     cpu_active_mask       - has bit 'cpu' set iff cpu available to migration
  *
  *  If !CONFIG_HOTPLUG_CPU, present == possible, and active == online.
  *
@@ -51,7 +52,11 @@ extern int nr_cpu_ids;
  *  life of that system boot.  The cpu_present_mask is dynamic(*),
  *  representing which CPUs are currently plugged in.  And
  *  cpu_online_mask is the dynamic subset of cpu_present_mask,
- *  indicating those CPUs available for scheduling.
+ *  indicating those CPUs available for scheduling.  The
+ *  cpu_hotpluggable_mask is also fixed at boot time as the set of CPU
+ *  id's which are possible AND can hotplug.  Cleared bits in this mask
+ *  mean that either the CPU is not possible, or it is possible but does
+ *  not support CPU hotplug operations.
  *
  *  If HOTPLUG is enabled, then cpu_possible_mask is forced to have
  *  all NR_CPUS bits set, otherwise it is just the set of CPUs that
@@ -61,6 +66,9 @@ extern int nr_cpu_ids;
  *  depending on what ACPI reports as currently plugged in, otherwise
  *  cpu_present_mask is just a copy of cpu_possible_mask.
  *
+ *  If HOTPLUG is not enabled then cpu_hotpluggable_mask is the empty
+ *  set.
+ *
  *  (*) Well, cpu_present_mask is dynamic in the hotplug case.  If not
  *      hotplug, it's a copy of cpu_possible_mask, hence fixed at boot.
  *
@@ -76,6 +84,7 @@ extern int nr_cpu_ids;
  */
 
 extern const struct cpumask *const cpu_possible_mask;
+extern const struct cpumask *const cpu_hotpluggable_mask;
 extern const struct cpumask *const cpu_online_mask;
 extern const struct cpumask *const cpu_present_mask;
 extern const struct cpumask *const cpu_active_mask;
@@ -85,19 +94,23 @@ extern const struct cpumask *const cpu_active_mask;
 #define num_possible_cpus()	cpumask_weight(cpu_possible_mask)
 #define num_present_cpus()	cpumask_weight(cpu_present_mask)
 #define num_active_cpus()	cpumask_weight(cpu_active_mask)
+#define num_hotpluggable_cpus()	cpumask_weight(cpu_hotpluggable_mask)
 #define cpu_online(cpu)		cpumask_test_cpu((cpu), cpu_online_mask)
 #define cpu_possible(cpu)	cpumask_test_cpu((cpu), cpu_possible_mask)
 #define cpu_present(cpu)	cpumask_test_cpu((cpu), cpu_present_mask)
 #define cpu_active(cpu)		cpumask_test_cpu((cpu), cpu_active_mask)
+#define cpu_hotpluggable(cpu)	cpumask_test_cpu((cpu, cpu_hotpluggable_mask))
 #else
 #define num_online_cpus()	1U
 #define num_possible_cpus()	1U
 #define num_present_cpus()	1U
 #define num_active_cpus()	1U
+#define num_hotpluggable_cpus()	0
 #define cpu_online(cpu)		((cpu) == 0)
 #define cpu_possible(cpu)	((cpu) == 0)
 #define cpu_present(cpu)	((cpu) == 0)
 #define cpu_active(cpu)		((cpu) == 0)
+#define cpu_hotpluggable(cpu)	0
 #endif
 
 /* verify cpu argument to cpumask_* operators */
@@ -678,16 +691,20 @@ extern const DECLARE_BITMAP(cpu_all_bits, NR_CPUS);
 #define cpu_none_mask to_cpumask(cpu_bit_bitmap[0])
 
 #define for_each_possible_cpu(cpu) for_each_cpu((cpu), cpu_possible_mask)
+#define for_each_hotpluggable_cpu(cpu) \
+				   for_each_cpu((cpu), cpu_hotpluggable_mask)
 #define for_each_online_cpu(cpu)   for_each_cpu((cpu), cpu_online_mask)
 #define for_each_present_cpu(cpu)  for_each_cpu((cpu), cpu_present_mask)
 
 /* Wrappers for arch boot code to manipulate normally-constant masks */
 void set_cpu_possible(unsigned int cpu, bool possible);
+void set_cpu_hotpluggable(unsigned int cpu, bool hotpluggable);
 void set_cpu_present(unsigned int cpu, bool present);
 void set_cpu_online(unsigned int cpu, bool online);
 void set_cpu_active(unsigned int cpu, bool active);
 void init_cpu_present(const struct cpumask *src);
 void init_cpu_possible(const struct cpumask *src);
+void init_cpu_hotpluggable(const struct cpumask *src);
 void init_cpu_online(const struct cpumask *src);
 
 /**
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 12b7458..8c397c9 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -536,6 +536,11 @@ static DECLARE_BITMAP(cpu_possible_bits, CONFIG_NR_CPUS) __read_mostly;
 const struct cpumask *const cpu_possible_mask = to_cpumask(cpu_possible_bits);
 EXPORT_SYMBOL(cpu_possible_mask);
 
+static DECLARE_BITMAP(cpu_hotpluggable_bits, CONFIG_NR_CPUS) __read_mostly;
+const struct cpumask *const cpu_hotpluggable_mask =
+					to_cpumask(cpu_hotpluggable_bits);
+EXPORT_SYMBOL(cpu_hotpluggable_mask);
+
 static DECLARE_BITMAP(cpu_online_bits, CONFIG_NR_CPUS) __read_mostly;
 const struct cpumask *const cpu_online_mask = to_cpumask(cpu_online_bits);
 EXPORT_SYMBOL(cpu_online_mask);
@@ -556,6 +561,14 @@ void set_cpu_possible(unsigned int cpu, bool possible)
 		cpumask_clear_cpu(cpu, to_cpumask(cpu_possible_bits));
 }
 
+void set_cpu_hotpluggable(unsigned int cpu, bool hotpluggable)
+{
+	if (hotpluggable)
+		cpumask_set_cpu(cpu, to_cpumask(cpu_hotpluggable_bits));
+	else
+		cpumask_clear_cpu(cpu, to_cpumask(cpu_hotpluggable_bits));
+}
+
 void set_cpu_present(unsigned int cpu, bool present)
 {
 	if (present)
@@ -590,6 +603,11 @@ void init_cpu_possible(const struct cpumask *src)
 	cpumask_copy(to_cpumask(cpu_possible_bits), src);
 }
 
+void init_cpu_hotpluggable(const struct cpumask *src)
+{
+	cpumask_copy(to_cpumask(cpu_hotpluggable_bits), src);
+}
+
 void init_cpu_online(const struct cpumask *src)
 {
 	cpumask_copy(to_cpumask(cpu_online_bits), src);
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH v2 0/2] new cpumask for hotpluggable CPUs
From: Mike Turquette @ 2011-08-10 20:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-pm, gregkh, linaro-dev, patches

This patch series introduces a new cpumask which tracks CPUs that
support hotplugging.  The purpose of this patch series is to provide a
simple method for kernel code to know which CPUs can be hotplugged and
which ones cannot.  Potential users of this code might be a thermal
mitigation technique which uses hotplug to lower temperature, or a power
capping mechanism which uses hotplug to lower power consumption.

All the of usual cpumask helper functions are created for this new mask.
The second patch in this series simply sets the bit for elligible CPUs
while they are being registered.  The cpumask itself is static after
boot and should not change (like the possbile mask).

Mike Turquette (2):
  cpumask: introduce cpumask for hotpluggable CPUs
  cpu: update cpu_hotpluggable_mask in register_cpu

 drivers/base/cpu.c      |    4 +++-
 include/linux/cpumask.h |   27 ++++++++++++++++++++++-----
 kernel/cpu.c            |   18 ++++++++++++++++++
 3 files changed, 43 insertions(+), 6 deletions(-)

-- 
1.7.4.1

^ permalink raw reply

* Re: [PATCH v4] PM: add statistics debugfs file for suspend to ram
From: Greg KH @ 2011-08-10 19:58 UTC (permalink / raw)
  To: Mansoor, Illyas
  Cc: Liu, ShuoX, linux-pm@lists.linux-foundation.org,
	Yanmin_zhang@linux.intel.com, linux-kernel@vger.kernel.org,
	Brown, Len
In-Reply-To: <EB6073F1DF78F9439DFDCC701BC6466007B36A0D12@bgsmsx502.gar.corp.intel.com>

On Thu, Aug 11, 2011 at 01:22:56AM +0530, Mansoor, Illyas wrote:
> static pm_message_t pm_transition;
> > > >
> > > > @@ -464,8 +465,12 @@ void dpm_resume_noirq(pm_message_t state)
> > > >  		mutex_unlock(&dpm_list_mtx);
> > > >
> > > >  		error = device_resume_noirq(dev, state);
> > > > -		if (error)
> > > > +		if (error) {
> > > > +			suspend_stats.failed_resume_noirq++;
> > > > +			dpm_save_failed_step(SUSPEND_RESUME_NOIRQ);
> > > > +			dpm_save_failed_dev(dev_name(dev));
> > >
> > > Please make these statistics conditionally enabled, so on a production system
> > > If we need to disable these statistics code we should be able to do so.
> > 
> > Why, are they taking time or space that is needed for something else?
> > What's the downside here of just not always having this enabled?
> 
> Why have something that is not required/Used?

Because someone might need it and rebuilding a kernel isn't possible on
lots of devices.

> Its only useful if DEBUGFS is configured anyways

Almost all systems these days have debugfs enabled, so that's a moot
point.

greg k-h

^ permalink raw reply

* Re: [PATCH v4] PM: add statistics debugfs file for suspend to ram
From: Mansoor, Illyas @ 2011-08-10 19:52 UTC (permalink / raw)
  To: Greg KH
  Cc: Liu, ShuoX, linux-pm@lists.linux-foundation.org,
	Yanmin_zhang@linux.intel.com, linux-kernel@vger.kernel.org,
	Brown, Len
In-Reply-To: <20110810194529.GA5590@suse.de>

static pm_message_t pm_transition;
> > >
> > > @@ -464,8 +465,12 @@ void dpm_resume_noirq(pm_message_t state)
> > >  		mutex_unlock(&dpm_list_mtx);
> > >
> > >  		error = device_resume_noirq(dev, state);
> > > -		if (error)
> > > +		if (error) {
> > > +			suspend_stats.failed_resume_noirq++;
> > > +			dpm_save_failed_step(SUSPEND_RESUME_NOIRQ);
> > > +			dpm_save_failed_dev(dev_name(dev));
> >
> > Please make these statistics conditionally enabled, so on a production system
> > If we need to disable these statistics code we should be able to do so.
> 
> Why, are they taking time or space that is needed for something else?
> What's the downside here of just not always having this enabled?

Why have something that is not required/Used?
Its only useful if DEBUGFS is configured anyways

-Illyas

^ permalink raw reply

* Re: [PATCH v4] PM: add statistics debugfs file for suspend to ram
From: Greg KH @ 2011-08-10 19:45 UTC (permalink / raw)
  To: Mansoor, Illyas
  Cc: Liu, ShuoX, linux-pm@lists.linux-foundation.org,
	Yanmin_zhang@linux.intel.com, linux-kernel@vger.kernel.org,
	Brown, Len
In-Reply-To: <EB6073F1DF78F9439DFDCC701BC6466007B36A0CCE@bgsmsx502.gar.corp.intel.com>

On Wed, Aug 10, 2011 at 11:30:40PM +0530, Mansoor, Illyas wrote:
> > +struct suspend_stats suspend_stats;
> >  static DEFINE_MUTEX(dpm_list_mtx);
> >  static pm_message_t pm_transition;
> > 
> > @@ -464,8 +465,12 @@ void dpm_resume_noirq(pm_message_t state)
> >  		mutex_unlock(&dpm_list_mtx);
> > 
> >  		error = device_resume_noirq(dev, state);
> > -		if (error)
> > +		if (error) {
> > +			suspend_stats.failed_resume_noirq++;
> > +			dpm_save_failed_step(SUSPEND_RESUME_NOIRQ);
> > +			dpm_save_failed_dev(dev_name(dev));
> 
> Please make these statistics conditionally enabled, so on a production system
> If we need to disable these statistics code we should be able to do so.

Why, are they taking time or space that is needed for something else?
What's the downside here of just not always having this enabled?

greg k-h

^ permalink raw reply

* Re: [PATCH v4] PM: add statistics debugfs file for suspend to ram
From: Mansoor, Illyas @ 2011-08-10 18:00 UTC (permalink / raw)
  To: Liu, ShuoX, linux-pm@lists.linux-foundation.org
  Cc: Brown, Len, Yanmin_zhang@linux.intel.com, Greg KH,
	linux-kernel@vger.kernel.org
In-Reply-To: <6E3BC7F7C9A4BF4286DD4C043110F30B5B847F42F9@shsmsx502.ccr.corp.intel.com>

> +struct suspend_stats suspend_stats;
>  static DEFINE_MUTEX(dpm_list_mtx);
>  static pm_message_t pm_transition;
> 
> @@ -464,8 +465,12 @@ void dpm_resume_noirq(pm_message_t state)
>  		mutex_unlock(&dpm_list_mtx);
> 
>  		error = device_resume_noirq(dev, state);
> -		if (error)
> +		if (error) {
> +			suspend_stats.failed_resume_noirq++;
> +			dpm_save_failed_step(SUSPEND_RESUME_NOIRQ);
> +			dpm_save_failed_dev(dev_name(dev));

Please make these statistics conditionally enabled, so on a production system
If we need to disable these statistics code we should be able to do so.

May be CONFIG_SUSPEND_STATS and convert those lines of code into function calls
That can be stubbed out when the config is not enabled.

-Illyas

^ permalink raw reply

* Re: [PATCH 1/2] cpumask: introduce cpumask for hotpluggable CPUs
From: Turquette, Mike @ 2011-08-10 17:25 UTC (permalink / raw)
  To: Mike Turquette, linux-kernel, linux-pm, linaro-dev, patches
In-Reply-To: <20110810084248.GB7673@matterhorn1>

On Wed, Aug 10, 2011 at 1:42 AM, Amit Kucheria <amit.kucheria@linaro.org> wrote:
> On 11 Aug 09, Mike Turquette wrote:
>> On some platforms it is possible to have some CPUs which support CPU
>> hotplug and some which do not.  Currently the prescence of an 'online'
>> sysfs entry in userspace is adequate for applications to know that a CPU
>> supports hotplug, but there is no convenient way to make the same
>> determination in the kernel.
>>
>> To better model this relationship this patch introduces a new cpumask to
>> track CPUs that support CPU hotplug operations.
>>
>> This new cpumask is populated at boot-time and remains static for the
>> life of the machine.  Bits set in the mask indicate a CPU which supports
>> hotplug, but make no guarantees about whether that CPU is currently
>> online or not.  Likewise a cleared bit in the mask indicates either a
>> CPU which cannot hotplug or a lack of a populated CPU.
>>
>> The purpose of this new cpumask is to aid kernel code which uses CPU to
>> take CPUs online and offline.  Possible uses are as a thermal event
>> mitigation technique or as a power capping mechanism.
>>
>> Signed-off-by: Mike Turquette <mturquette@ti.com>
>> ---
>>  include/linux/cpumask.h |   27 ++++++++++++++++++++++-----
>>  kernel/cpu.c            |   18 ++++++++++++++++++
>>  2 files changed, 40 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
>> index b24ac56..9eed444 100644
>> --- a/include/linux/cpumask.h
>> +++ b/include/linux/cpumask.h
>> @@ -39,10 +39,11 @@ extern int nr_cpu_ids;
>>   * The following particular system cpumasks and operations manage
>>   * possible, present, active and online cpus.
>>   *
>> - *     cpu_possible_mask- has bit 'cpu' set iff cpu is populatable
>> - *     cpu_present_mask - has bit 'cpu' set iff cpu is populated
>> - *     cpu_online_mask  - has bit 'cpu' set iff cpu available to scheduler
>> - *     cpu_active_mask  - has bit 'cpu' set iff cpu available to migration
>> + *     cpu_possible_mask     - has bit 'cpu' set iff cpu is populatable
>> + *     cpu_hotpluggable_mask - has bit 'cpu' set iff cpu is hotpluggable
>> + *     cpu_present_mask      - has bit 'cpu' set iff cpu is populated
>> + *     cpu_online_mask       - has bit 'cpu' set iff cpu available to scheduler
>> + *     cpu_active_mask       - has bit 'cpu' set iff cpu available to migration
>>   *
>>   *  If !CONFIG_HOTPLUG_CPU, present == possible, and active == online.
>>   *
>> @@ -51,7 +52,11 @@ extern int nr_cpu_ids;
>>   *  life of that system boot.  The cpu_present_mask is dynamic(*),
>>   *  representing which CPUs are currently plugged in.  And
>>   *  cpu_online_mask is the dynamic subset of cpu_present_mask,
>> - *  indicating those CPUs available for scheduling.
>> + *  indicating those CPUs available for scheduling.  The
>> + *  cpu_hotpluggable_mask is also fixed at boot time, as the set of CPU
>> + *  id's which are possible AND can hotplug.  Cleared bits in this mask
>> + *  mean that either the CPU is not possible, or it is possible but does
>> + *  not support CPU hotplug operations.
>>   *
>>   *  If HOTPLUG is enabled, then cpu_possible_mask is forced to have
>>   *  all NR_CPUS bits set, otherwise it is just the set of CPUs that
>> @@ -61,6 +66,9 @@ extern int nr_cpu_ids;
>>   *  depending on what ACPI reports as currently plugged in, otherwise
>>   *  cpu_present_mask is just a copy of cpu_possible_mask.
>>   *
>> + *  If CONFIG_HOTPLUG_CPU is enabled, then cpu_hotpluggable_mask matches
>> + *  the description above, otherwise it is the empty set.
>> + *
>>   *  (*) Well, cpu_present_mask is dynamic in the hotplug case.  If not
>>   *      hotplug, it's a copy of cpu_possible_mask, hence fixed at boot.
>>   *
>> @@ -76,6 +84,7 @@ extern int nr_cpu_ids;
>>   */
>>
>>  extern const struct cpumask *const cpu_possible_mask;
>> +extern const struct cpumask *const cpu_hotpluggable_mask;
>>  extern const struct cpumask *const cpu_online_mask;
>>  extern const struct cpumask *const cpu_present_mask;
>>  extern const struct cpumask *const cpu_active_mask;
>> @@ -85,19 +94,23 @@ extern const struct cpumask *const cpu_active_mask;
>>  #define num_possible_cpus()  cpumask_weight(cpu_possible_mask)
>>  #define num_present_cpus()   cpumask_weight(cpu_present_mask)
>>  #define num_active_cpus()    cpumask_weight(cpu_active_mask)
>> +#define num_hotpluggable_cpus()      cpumask_weight(cpu_hotpluggable_mask)
>>  #define cpu_online(cpu)              cpumask_test_cpu((cpu), cpu_online_mask)
>>  #define cpu_possible(cpu)    cpumask_test_cpu((cpu), cpu_possible_mask)
>>  #define cpu_present(cpu)     cpumask_test_cpu((cpu), cpu_present_mask)
>>  #define cpu_active(cpu)              cpumask_test_cpu((cpu), cpu_active_mask)
>> +#define cpu_hotpluggable(cpu)        cpumask_test_cpu((cpu, cpu_hotpluggable_mask)
>
>                                 missing closing bracket? ^^^^

Oops.  Will fix in V2.

Thanks,
Mike

>>  #else
>>  #define num_online_cpus()    1U
>>  #define num_possible_cpus()  1U
>>  #define num_present_cpus()   1U
>>  #define num_active_cpus()    1U
>> +#define num_hotpluggable_cpus()      0
>>  #define cpu_online(cpu)              ((cpu) == 0)
>>  #define cpu_possible(cpu)    ((cpu) == 0)
>>  #define cpu_present(cpu)     ((cpu) == 0)
>>  #define cpu_active(cpu)              ((cpu) == 0)
>> +#define cpu_hotpluggable(cpu)        0
>>  #endif
>>
>>  /* verify cpu argument to cpumask_* operators */
>> @@ -678,16 +691,20 @@ extern const DECLARE_BITMAP(cpu_all_bits, NR_CPUS);
>>  #define cpu_none_mask to_cpumask(cpu_bit_bitmap[0])
>>
>>  #define for_each_possible_cpu(cpu) for_each_cpu((cpu), cpu_possible_mask)
>> +#define for_each_hotpluggable_cpu(cpu) \
>> +                                for_each_cpu((cpu), cpu_hotpluggable_mask)
>>  #define for_each_online_cpu(cpu)   for_each_cpu((cpu), cpu_online_mask)
>>  #define for_each_present_cpu(cpu)  for_each_cpu((cpu), cpu_present_mask)
>>
>>  /* Wrappers for arch boot code to manipulate normally-constant masks */
>>  void set_cpu_possible(unsigned int cpu, bool possible);
>> +void set_cpu_hotpluggable(unsigned int cpu, bool hotpluggable);
>>  void set_cpu_present(unsigned int cpu, bool present);
>>  void set_cpu_online(unsigned int cpu, bool online);
>>  void set_cpu_active(unsigned int cpu, bool active);
>>  void init_cpu_present(const struct cpumask *src);
>>  void init_cpu_possible(const struct cpumask *src);
>> +void init_cpu_hotpluggable(const struct cpumask *src);
>>  void init_cpu_online(const struct cpumask *src);
>>
>>  /**
>> diff --git a/kernel/cpu.c b/kernel/cpu.c
>> index 12b7458..8c397c9 100644
>> --- a/kernel/cpu.c
>> +++ b/kernel/cpu.c
>> @@ -536,6 +536,11 @@ static DECLARE_BITMAP(cpu_possible_bits, CONFIG_NR_CPUS) __read_mostly;
>>  const struct cpumask *const cpu_possible_mask = to_cpumask(cpu_possible_bits);
>>  EXPORT_SYMBOL(cpu_possible_mask);
>>
>> +static DECLARE_BITMAP(cpu_hotpluggable_bits, CONFIG_NR_CPUS) __read_mostly;
>> +const struct cpumask *const cpu_hotpluggable_mask =
>> +                                     to_cpumask(cpu_hotpluggable_bits);
>> +EXPORT_SYMBOL(cpu_hotpluggable_mask);
>> +
>>  static DECLARE_BITMAP(cpu_online_bits, CONFIG_NR_CPUS) __read_mostly;
>>  const struct cpumask *const cpu_online_mask = to_cpumask(cpu_online_bits);
>>  EXPORT_SYMBOL(cpu_online_mask);
>> @@ -556,6 +561,14 @@ void set_cpu_possible(unsigned int cpu, bool possible)
>>               cpumask_clear_cpu(cpu, to_cpumask(cpu_possible_bits));
>>  }
>>
>> +void set_cpu_hotpluggable(unsigned int cpu, bool hotpluggable)
>> +{
>> +     if (hotpluggable)
>> +             cpumask_set_cpu(cpu, to_cpumask(cpu_hotpluggable_bits));
>> +     else
>> +             cpumask_clear_cpu(cpu, to_cpumask(cpu_hotpluggable_bits));
>> +}
>> +
>>  void set_cpu_present(unsigned int cpu, bool present)
>>  {
>>       if (present)
>> @@ -590,6 +603,11 @@ void init_cpu_possible(const struct cpumask *src)
>>       cpumask_copy(to_cpumask(cpu_possible_bits), src);
>>  }
>>
>> +void init_cpu_hotpluggable(const struct cpumask *src)
>> +{
>> +     cpumask_copy(to_cpumask(cpu_hotpluggable_bits), src);
>> +}
>> +
>>  void init_cpu_online(const struct cpumask *src)
>>  {
>>       cpumask_copy(to_cpumask(cpu_online_bits), src);
>> --
>> 1.7.4.1
>>
>>
>> _______________________________________________
>> linaro-dev mailing list
>> linaro-dev@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/linaro-dev
>

^ permalink raw reply

* Re: [PATCH 1/2] cpumask: introduce cpumask for hotpluggable CPUs
From: Amit Kucheria @ 2011-08-10  8:42 UTC (permalink / raw)
  To: Mike Turquette; +Cc: linux-pm, linaro-dev, linux-kernel, patches
In-Reply-To: <1312940007-26069-2-git-send-email-mturquette@ti.com>

On 11 Aug 09, Mike Turquette wrote:
> On some platforms it is possible to have some CPUs which support CPU
> hotplug and some which do not.  Currently the prescence of an 'online'
> sysfs entry in userspace is adequate for applications to know that a CPU
> supports hotplug, but there is no convenient way to make the same
> determination in the kernel.
> 
> To better model this relationship this patch introduces a new cpumask to
> track CPUs that support CPU hotplug operations.
> 
> This new cpumask is populated at boot-time and remains static for the
> life of the machine.  Bits set in the mask indicate a CPU which supports
> hotplug, but make no guarantees about whether that CPU is currently
> online or not.  Likewise a cleared bit in the mask indicates either a
> CPU which cannot hotplug or a lack of a populated CPU.
> 
> The purpose of this new cpumask is to aid kernel code which uses CPU to
> take CPUs online and offline.  Possible uses are as a thermal event
> mitigation technique or as a power capping mechanism.
> 
> Signed-off-by: Mike Turquette <mturquette@ti.com>
> ---
>  include/linux/cpumask.h |   27 ++++++++++++++++++++++-----
>  kernel/cpu.c            |   18 ++++++++++++++++++
>  2 files changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index b24ac56..9eed444 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -39,10 +39,11 @@ extern int nr_cpu_ids;
>   * The following particular system cpumasks and operations manage
>   * possible, present, active and online cpus.
>   *
> - *     cpu_possible_mask- has bit 'cpu' set iff cpu is populatable
> - *     cpu_present_mask - has bit 'cpu' set iff cpu is populated
> - *     cpu_online_mask  - has bit 'cpu' set iff cpu available to scheduler
> - *     cpu_active_mask  - has bit 'cpu' set iff cpu available to migration
> + *     cpu_possible_mask     - has bit 'cpu' set iff cpu is populatable
> + *     cpu_hotpluggable_mask - has bit 'cpu' set iff cpu is hotpluggable
> + *     cpu_present_mask      - has bit 'cpu' set iff cpu is populated
> + *     cpu_online_mask       - has bit 'cpu' set iff cpu available to scheduler
> + *     cpu_active_mask       - has bit 'cpu' set iff cpu available to migration
>   *
>   *  If !CONFIG_HOTPLUG_CPU, present == possible, and active == online.
>   *
> @@ -51,7 +52,11 @@ extern int nr_cpu_ids;
>   *  life of that system boot.  The cpu_present_mask is dynamic(*),
>   *  representing which CPUs are currently plugged in.  And
>   *  cpu_online_mask is the dynamic subset of cpu_present_mask,
> - *  indicating those CPUs available for scheduling.
> + *  indicating those CPUs available for scheduling.  The
> + *  cpu_hotpluggable_mask is also fixed at boot time, as the set of CPU
> + *  id's which are possible AND can hotplug.  Cleared bits in this mask
> + *  mean that either the CPU is not possible, or it is possible but does
> + *  not support CPU hotplug operations.
>   *
>   *  If HOTPLUG is enabled, then cpu_possible_mask is forced to have
>   *  all NR_CPUS bits set, otherwise it is just the set of CPUs that
> @@ -61,6 +66,9 @@ extern int nr_cpu_ids;
>   *  depending on what ACPI reports as currently plugged in, otherwise
>   *  cpu_present_mask is just a copy of cpu_possible_mask.
>   *
> + *  If CONFIG_HOTPLUG_CPU is enabled, then cpu_hotpluggable_mask matches
> + *  the description above, otherwise it is the empty set.
> + *
>   *  (*) Well, cpu_present_mask is dynamic in the hotplug case.  If not
>   *      hotplug, it's a copy of cpu_possible_mask, hence fixed at boot.
>   *
> @@ -76,6 +84,7 @@ extern int nr_cpu_ids;
>   */
>  
>  extern const struct cpumask *const cpu_possible_mask;
> +extern const struct cpumask *const cpu_hotpluggable_mask;
>  extern const struct cpumask *const cpu_online_mask;
>  extern const struct cpumask *const cpu_present_mask;
>  extern const struct cpumask *const cpu_active_mask;
> @@ -85,19 +94,23 @@ extern const struct cpumask *const cpu_active_mask;
>  #define num_possible_cpus()	cpumask_weight(cpu_possible_mask)
>  #define num_present_cpus()	cpumask_weight(cpu_present_mask)
>  #define num_active_cpus()	cpumask_weight(cpu_active_mask)
> +#define num_hotpluggable_cpus()	cpumask_weight(cpu_hotpluggable_mask)
>  #define cpu_online(cpu)		cpumask_test_cpu((cpu), cpu_online_mask)
>  #define cpu_possible(cpu)	cpumask_test_cpu((cpu), cpu_possible_mask)
>  #define cpu_present(cpu)	cpumask_test_cpu((cpu), cpu_present_mask)
>  #define cpu_active(cpu)		cpumask_test_cpu((cpu), cpu_active_mask)
> +#define cpu_hotpluggable(cpu)	cpumask_test_cpu((cpu, cpu_hotpluggable_mask)

                                 missing closing bracket? ^^^^ 							   

>  #else
>  #define num_online_cpus()	1U
>  #define num_possible_cpus()	1U
>  #define num_present_cpus()	1U
>  #define num_active_cpus()	1U
> +#define num_hotpluggable_cpus()	0
>  #define cpu_online(cpu)		((cpu) == 0)
>  #define cpu_possible(cpu)	((cpu) == 0)
>  #define cpu_present(cpu)	((cpu) == 0)
>  #define cpu_active(cpu)		((cpu) == 0)
> +#define cpu_hotpluggable(cpu)	0
>  #endif
>  
>  /* verify cpu argument to cpumask_* operators */
> @@ -678,16 +691,20 @@ extern const DECLARE_BITMAP(cpu_all_bits, NR_CPUS);
>  #define cpu_none_mask to_cpumask(cpu_bit_bitmap[0])
>  
>  #define for_each_possible_cpu(cpu) for_each_cpu((cpu), cpu_possible_mask)
> +#define for_each_hotpluggable_cpu(cpu) \
> +				   for_each_cpu((cpu), cpu_hotpluggable_mask)
>  #define for_each_online_cpu(cpu)   for_each_cpu((cpu), cpu_online_mask)
>  #define for_each_present_cpu(cpu)  for_each_cpu((cpu), cpu_present_mask)
>  
>  /* Wrappers for arch boot code to manipulate normally-constant masks */
>  void set_cpu_possible(unsigned int cpu, bool possible);
> +void set_cpu_hotpluggable(unsigned int cpu, bool hotpluggable);
>  void set_cpu_present(unsigned int cpu, bool present);
>  void set_cpu_online(unsigned int cpu, bool online);
>  void set_cpu_active(unsigned int cpu, bool active);
>  void init_cpu_present(const struct cpumask *src);
>  void init_cpu_possible(const struct cpumask *src);
> +void init_cpu_hotpluggable(const struct cpumask *src);
>  void init_cpu_online(const struct cpumask *src);
>  
>  /**
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 12b7458..8c397c9 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -536,6 +536,11 @@ static DECLARE_BITMAP(cpu_possible_bits, CONFIG_NR_CPUS) __read_mostly;
>  const struct cpumask *const cpu_possible_mask = to_cpumask(cpu_possible_bits);
>  EXPORT_SYMBOL(cpu_possible_mask);
>  
> +static DECLARE_BITMAP(cpu_hotpluggable_bits, CONFIG_NR_CPUS) __read_mostly;
> +const struct cpumask *const cpu_hotpluggable_mask =
> +					to_cpumask(cpu_hotpluggable_bits);
> +EXPORT_SYMBOL(cpu_hotpluggable_mask);
> +
>  static DECLARE_BITMAP(cpu_online_bits, CONFIG_NR_CPUS) __read_mostly;
>  const struct cpumask *const cpu_online_mask = to_cpumask(cpu_online_bits);
>  EXPORT_SYMBOL(cpu_online_mask);
> @@ -556,6 +561,14 @@ void set_cpu_possible(unsigned int cpu, bool possible)
>  		cpumask_clear_cpu(cpu, to_cpumask(cpu_possible_bits));
>  }
>  
> +void set_cpu_hotpluggable(unsigned int cpu, bool hotpluggable)
> +{
> +	if (hotpluggable)
> +		cpumask_set_cpu(cpu, to_cpumask(cpu_hotpluggable_bits));
> +	else
> +		cpumask_clear_cpu(cpu, to_cpumask(cpu_hotpluggable_bits));
> +}
> +
>  void set_cpu_present(unsigned int cpu, bool present)
>  {
>  	if (present)
> @@ -590,6 +603,11 @@ void init_cpu_possible(const struct cpumask *src)
>  	cpumask_copy(to_cpumask(cpu_possible_bits), src);
>  }
>  
> +void init_cpu_hotpluggable(const struct cpumask *src)
> +{
> +	cpumask_copy(to_cpumask(cpu_hotpluggable_bits), src);
> +}
> +
>  void init_cpu_online(const struct cpumask *src)
>  {
>  	cpumask_copy(to_cpumask(cpu_online_bits), src);
> -- 
> 1.7.4.1
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

^ permalink raw reply

* [PATCH v4] PM: add statistics debugfs file for suspend to ram
From: Liu, ShuoX @ 2011-08-10  7:28 UTC (permalink / raw)
  To: linux-pm@lists.linux-foundation.org
  Cc: Brown, Len, Yanmin_zhang@linux.intel.com, Greg KH,
	linux-kernel@vger.kernel.org

From: ShuoX Liu <shuox.liu@intel.com>

Record S3 failure time about each reason and the latest two failed
devices' names in S3 progress.
We can check it through 'suspend_stats' entry in debugfs.

The motivation of the patch:

We are enabling power features on Medfield. Comparing with PC/notebook,
a mobile enters/exits suspend-2-ram (we call it s3 on Medfield) far
more frequently. If it can't enter suspend-2-ram in time, the power
might be used up soon.

We often find sometimes, a device suspend fails. Then, system retries
s3 over and over again. As display is off, testers and developers
don't know what happens.

Some testers and developers complain they don't know if system
tries suspend-2-ram, and what device fails to suspend. They need
such info for a quick check. The patch adds suspend_stats under
debugfs for users to check suspend to RAM statistics quickly.

If not using this patch, we have other methods to get info about
what device fails. One is to turn on  CONFIG_PM_DEBUG, but users
would get too much info and testers need recompile the system.

In addition, dynamic debug is another good tool to dump debug info.
But it still doesn't match our utilization scenario closely.
1) user need write a user space parser to process the syslog output;
2) Our testing scenario is we leave the mobile for at least hours.
   Then, check its status. No serial console available during the
   testing. One is because console would be suspended, and the other
   is serial console connecting with spi or HSU devices would consume
   power. These devices are powered off at suspend-2-ram.

Thank Greg, MyungJoo and Rafael for their great comments.

Change Log:
  V4: Improve output format.
  V3: Change some programming styles.
  V2:
	1) Change the sysfs entry to debugfs.
	2) Add some explanation in document file.

Contribution:
	yanmin_zhang@linux.intel.com

Signed-off-by: ShuoX Liu <shuox.liu@intel.com>
---
 Documentation/power/basic-pm-debugging.txt |   24 +++++++
 drivers/base/power/main.c                  |   31 +++++++--
 include/linux/suspend.h                    |   52 ++++++++++++++
 kernel/power/main.c                        |  102 ++++++++++++++++++++++++++++
 kernel/power/suspend.c                     |   17 ++++-
 5 files changed, 218 insertions(+), 8 deletions(-)

diff --git a/Documentation/power/basic-pm-debugging.txt b/Documentation/power/basic-pm-debugging.txt
index ddd7817..62eca08 100644
--- a/Documentation/power/basic-pm-debugging.txt
+++ b/Documentation/power/basic-pm-debugging.txt
@@ -201,3 +201,27 @@ case, you may be able to search for failing drivers by following the procedure
 analogous to the one described in section 1.  If you find some failing drivers,
 you will have to unload them every time before an STR transition (ie. before
 you run s2ram), and please report the problems with them.
+
+There is a debugfs entry which shows the suspend to RAM statistics. Here is an
+example of its output.
+	# mount -t debugfs none /sys/kernel/debug
+	# cat /sys/kernel/debug/suspend_stats
+	success: 20
+	fail: 5
+	failed_freeze: 0
+	failed_prepare: 0
+	failed_suspend: 5
+	failed_suspend_noirq: 0
+	failed_resume: 0
+	failed_resume_noirq: 0
+	failures:
+	  last_failed_dev:	alarm
+				adc
+	  last_failed_errno:	-16
+				-16
+	  last_failed_step:	suspend
+				suspend
+Field success means the success number of suspend to RAM, and field fail means
+the failure number. Others are the failure number of different steps of suspend
+to RAM. suspend_stats just lists the last 2 failed devices, error number and
+failed step of suspend.
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index a854591..23b755d 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -46,6 +46,7 @@ LIST_HEAD(dpm_prepared_list);
 LIST_HEAD(dpm_suspended_list);
 LIST_HEAD(dpm_noirq_list);
 
+struct suspend_stats suspend_stats;
 static DEFINE_MUTEX(dpm_list_mtx);
 static pm_message_t pm_transition;
 
@@ -464,8 +465,12 @@ void dpm_resume_noirq(pm_message_t state)
 		mutex_unlock(&dpm_list_mtx);
 
 		error = device_resume_noirq(dev, state);
-		if (error)
+		if (error) {
+			suspend_stats.failed_resume_noirq++;
+			dpm_save_failed_step(SUSPEND_RESUME_NOIRQ);
+			dpm_save_failed_dev(dev_name(dev));
 			pm_dev_err(dev, state, " early", error);
+		}
 
 		mutex_lock(&dpm_list_mtx);
 		put_device(dev);
@@ -626,8 +631,12 @@ void dpm_resume(pm_message_t state)
 			mutex_unlock(&dpm_list_mtx);
 
 			error = device_resume(dev, state, false);
-			if (error)
+			if (error) {
+				suspend_stats.failed_resume++;
+				dpm_save_failed_step(SUSPEND_RESUME);
+				dpm_save_failed_dev(dev_name(dev));
 				pm_dev_err(dev, state, "", error);
+			}
 
 			mutex_lock(&dpm_list_mtx);
 		}
@@ -802,6 +811,9 @@ int dpm_suspend_noirq(pm_message_t state)
 		mutex_lock(&dpm_list_mtx);
 		if (error) {
 			pm_dev_err(dev, state, " late", error);
+			suspend_stats.failed_suspend_noirq++;
+			dpm_save_failed_step(SUSPEND_SUSPEND_NOIRQ);
+			dpm_save_failed_dev(dev_name(dev));
 			put_device(dev);
 			break;
 		}
@@ -923,8 +935,10 @@ static void async_suspend(void *data, async_cookie_t cookie)
 	int error;
 
 	error = __device_suspend(dev, pm_transition, true);
-	if (error)
+	if (error) {
+		dpm_save_failed_dev(dev_name(dev));
 		pm_dev_err(dev, pm_transition, " async", error);
+	}
 
 	put_device(dev);
 }
@@ -967,6 +981,7 @@ int dpm_suspend(pm_message_t state)
 		mutex_lock(&dpm_list_mtx);
 		if (error) {
 			pm_dev_err(dev, state, "", error);
+			dpm_save_failed_dev(dev_name(dev));
 			put_device(dev);
 			break;
 		}
@@ -980,7 +995,10 @@ int dpm_suspend(pm_message_t state)
 	async_synchronize_full();
 	if (!error)
 		error = async_error;
-	if (!error)
+	if (error) {
+		suspend_stats.failed_suspend++;
+		dpm_save_failed_step(SUSPEND_SUSPEND);
+	} else
 		dpm_show_time(starttime, state, NULL);
 	return error;
 }
@@ -1088,7 +1106,10 @@ int dpm_suspend_start(pm_message_t state)
 	int error;
 
 	error = dpm_prepare(state);
-	if (!error)
+	if (error) {
+		suspend_stats.failed_prepare++;
+		dpm_save_failed_step(SUSPEND_PREPARE);
+	} else
 		error = dpm_suspend(state);
 	return error;
 }
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 6bbcef2..76f42e4 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -34,6 +34,58 @@ typedef int __bitwise suspend_state_t;
 #define PM_SUSPEND_MEM		((__force suspend_state_t) 3)
 #define PM_SUSPEND_MAX		((__force suspend_state_t) 4)
 
+enum suspend_stat_step {
+	SUSPEND_FREEZE = 1,
+	SUSPEND_PREPARE,
+	SUSPEND_SUSPEND,
+	SUSPEND_SUSPEND_NOIRQ,
+	SUSPEND_RESUME_NOIRQ,
+	SUSPEND_RESUME
+};
+
+struct suspend_stats {
+	int	success;
+	int	fail;
+	int	failed_freeze;
+	int	failed_prepare;
+	int	failed_suspend;
+	int	failed_suspend_noirq;
+	int	failed_resume;
+	int	failed_resume_noirq;
+#define	REC_FAILED_NUM	2
+	int	last_failed_dev;
+	char	failed_devs[REC_FAILED_NUM][40];
+	int	last_failed_errno;
+	int	errno[REC_FAILED_NUM];
+	int	last_failed_step;
+	enum suspend_stat_step	failed_steps[REC_FAILED_NUM];
+};
+
+extern struct suspend_stats suspend_stats;
+
+static inline void dpm_save_failed_dev(const char *name)
+{
+	strlcpy(suspend_stats.failed_devs[suspend_stats.last_failed_dev],
+		name,
+		sizeof(suspend_stats.failed_devs[0]));
+	suspend_stats.last_failed_dev++;
+	suspend_stats.last_failed_dev %= REC_FAILED_NUM;
+}
+
+static inline void dpm_save_failed_errno(int err)
+{
+	suspend_stats.errno[suspend_stats.last_failed_errno] = err;
+	suspend_stats.last_failed_errno++;
+	suspend_stats.last_failed_errno %= REC_FAILED_NUM;
+}
+
+static inline void dpm_save_failed_step(enum suspend_stat_step step)
+{
+	suspend_stats.failed_steps[suspend_stats.last_failed_step] = step;
+	suspend_stats.last_failed_step++;
+	suspend_stats.last_failed_step %= REC_FAILED_NUM;
+}
+
 /**
  * struct platform_suspend_ops - Callbacks for managing platform dependent
  *	system sleep states.
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 6c601f8..2757acb 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -12,6 +12,8 @@
 #include <linux/string.h>
 #include <linux/resume-trace.h>
 #include <linux/workqueue.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
 
 #include "power.h"
 
@@ -133,6 +135,101 @@ power_attr(pm_test);
 
 #endif /* CONFIG_PM_SLEEP */
 
+#ifdef CONFIG_DEBUG_FS
+static char *suspend_step_name(enum suspend_stat_step step)
+{
+	switch (step) {
+	case SUSPEND_FREEZE:
+		return "freeze";
+	case SUSPEND_PREPARE:
+		return "prepare";
+	case SUSPEND_SUSPEND:
+		return "suspend";
+	case SUSPEND_SUSPEND_NOIRQ:
+		return "suspend_noirq";
+	case SUSPEND_RESUME_NOIRQ:
+		return "resume_noirq";
+	case SUSPEND_RESUME:
+		return "resume";
+	default:
+		return "";
+	}
+}
+
+static int suspend_stats_show(struct seq_file *s, void *unused)
+{
+	int i, index, last_dev, last_errno, last_step;
+
+	last_dev = suspend_stats.last_failed_dev + REC_FAILED_NUM - 1;
+	last_dev %= REC_FAILED_NUM;
+	last_errno = suspend_stats.last_failed_errno + REC_FAILED_NUM - 1;
+	last_errno %= REC_FAILED_NUM;
+	last_step = suspend_stats.last_failed_step + REC_FAILED_NUM - 1;
+	last_step %= REC_FAILED_NUM;
+	seq_printf(s, "%s: %d\n%s: %d\n%s: %d\n%s: %d\n"
+			"%s: %d\n%s: %d\n%s: %d\n%s: %d\n",
+			"success", suspend_stats.success,
+			"fail", suspend_stats.fail,
+			"failed_freeze", suspend_stats.failed_freeze,
+			"failed_prepare", suspend_stats.failed_prepare,
+			"failed_suspend", suspend_stats.failed_suspend,
+			"failed_suspend_noirq",
+				suspend_stats.failed_suspend_noirq,
+			"failed_resume", suspend_stats.failed_resume,
+			"failed_resume_noirq",
+				suspend_stats.failed_resume_noirq);
+	seq_printf(s,	"failures:\n  last_failed_dev:\t%-s\n",
+			suspend_stats.failed_devs[last_dev]);
+	for (i = 1; i < REC_FAILED_NUM; i++) {
+		index = last_dev + REC_FAILED_NUM - i;
+		index %= REC_FAILED_NUM;
+		seq_printf(s, "\t\t\t%-s\n",
+			suspend_stats.failed_devs[index]);
+	}
+	seq_printf(s,	"  last_failed_errno:\t%-d\n",
+			suspend_stats.errno[last_errno]);
+	for (i = 1; i < REC_FAILED_NUM; i++) {
+		index = last_errno + REC_FAILED_NUM - i;
+		index %= REC_FAILED_NUM;
+		seq_printf(s, "\t\t\t%-d\n",
+			suspend_stats.errno[index]);
+	}
+	seq_printf(s,	"  last_failed_step:\t%-s\n",
+			suspend_step_name(
+				suspend_stats.failed_steps[last_step]));
+	for (i = 1; i < REC_FAILED_NUM; i++) {
+		index = last_step + REC_FAILED_NUM - i;
+		index %= REC_FAILED_NUM;
+		seq_printf(s, "\t\t\t%-s\n",
+			suspend_step_name(
+				suspend_stats.failed_steps[index]));
+	}
+
+	return 0;
+}
+
+static int suspend_stats_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, suspend_stats_show, NULL);
+}
+
+static const struct file_operations suspend_stats_operations = {
+	.open           = suspend_stats_open,
+	.read           = seq_read,
+	.llseek         = seq_lseek,
+	.release        = single_release,
+};
+
+static int __init pm_debugfs_init(void)
+{
+	debugfs_create_file("suspend_stats", S_IFREG | S_IRUGO,
+			NULL, NULL, &suspend_stats_operations);
+	return 0;
+}
+
+late_initcall(pm_debugfs_init);
+#endif /* CONFIG_DEBUG_FS */
+
 struct kobject *power_kobj;
 
 /**
@@ -194,6 +291,11 @@ static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr,
 	}
 	if (state < PM_SUSPEND_MAX && *s)
 		error = enter_state(state);
+		if (error) {
+			suspend_stats.fail++;
+			dpm_save_failed_errno(error);
+		} else
+			suspend_stats.success++;
 #endif
 
  Exit:
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index b6b71ad..595a3dd 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -104,7 +104,10 @@ static int suspend_prepare(void)
 		goto Finish;
 
 	error = suspend_freeze_processes();
-	if (!error)
+	if (error) {
+		suspend_stats.failed_freeze++;
+		dpm_save_failed_step(SUSPEND_FREEZE);
+	} else
 		return 0;
 
 	suspend_thaw_processes();
@@ -315,8 +318,16 @@ int enter_state(suspend_state_t state)
  */
 int pm_suspend(suspend_state_t state)
 {
-	if (state > PM_SUSPEND_ON && state <= PM_SUSPEND_MAX)
-		return enter_state(state);
+	int ret;
+	if (state > PM_SUSPEND_ON && state <= PM_SUSPEND_MAX) {
+		ret = enter_state(state);
+		if (ret) {
+			suspend_stats.fail++;
+			dpm_save_failed_errno(ret);
+		} else
+			suspend_stats.success++;
+		return ret;
+	}
 	return -EINVAL;
 }
 EXPORT_SYMBOL(pm_suspend);
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH 1/2] cpumask: introduce cpumask for hotpluggable CPUs
From: Turquette, Mike @ 2011-08-10  2:03 UTC (permalink / raw)
  To: Christian Robottom Reis; +Cc: linux-pm, linaro-dev, linux-kernel, patches
In-Reply-To: <20110810020157.GA29494@anthem.async.com.br>

On Tue, Aug 9, 2011 at 7:01 PM, Christian Robottom Reis <kiko@linaro.org> wrote:
> On Tue, Aug 09, 2011 at 06:33:26PM -0700, Mike Turquette wrote:
>> - *     cpu_possible_mask- has bit 'cpu' set iff cpu is populatable
>> - *     cpu_present_mask - has bit 'cpu' set iff cpu is populated
>> - *     cpu_online_mask  - has bit 'cpu' set iff cpu available to scheduler
>> - *     cpu_active_mask  - has bit 'cpu' set iff cpu available to migration
>> + *     cpu_possible_mask     - has bit 'cpu' set iff cpu is populatable
>> + *     cpu_hotpluggable_mask - has bit 'cpu' set iff cpu is hotpluggable
>> + *     cpu_present_mask      - has bit 'cpu' set iff cpu is populated
>> + *     cpu_online_mask       - has bit 'cpu' set iff cpu available to scheduler
>> + *     cpu_active_mask       - has bit 'cpu' set iff cpu available to migration
>
> Why not fix the 'iff' typo while you're here?

iff = if and only if

http://en.wikipedia.org/wiki/If_and_only_if

Thanks!
Mike

> Christian Robottom Reis, Engineering VP
> Brazil (GMT-3) | [+55] 16 9112 6430 | [+1] 612 216 4935
> Linaro.org: Open Source Software for ARM SoCs
>

^ permalink raw reply

* Re: [PATCH 1/2] cpumask: introduce cpumask for hotpluggable CPUs
From: Christian Robottom Reis @ 2011-08-10  2:01 UTC (permalink / raw)
  To: Mike Turquette; +Cc: linux-pm, linaro-dev, linux-kernel, patches
In-Reply-To: <1312940007-26069-2-git-send-email-mturquette@ti.com>

On Tue, Aug 09, 2011 at 06:33:26PM -0700, Mike Turquette wrote:
> - *     cpu_possible_mask- has bit 'cpu' set iff cpu is populatable
> - *     cpu_present_mask - has bit 'cpu' set iff cpu is populated
> - *     cpu_online_mask  - has bit 'cpu' set iff cpu available to scheduler
> - *     cpu_active_mask  - has bit 'cpu' set iff cpu available to migration
> + *     cpu_possible_mask     - has bit 'cpu' set iff cpu is populatable
> + *     cpu_hotpluggable_mask - has bit 'cpu' set iff cpu is hotpluggable
> + *     cpu_present_mask      - has bit 'cpu' set iff cpu is populated
> + *     cpu_online_mask       - has bit 'cpu' set iff cpu available to scheduler
> + *     cpu_active_mask       - has bit 'cpu' set iff cpu available to migration

Why not fix the 'iff' typo while you're here?
-- 
Christian Robottom Reis, Engineering VP
Brazil (GMT-3) | [+55] 16 9112 6430 | [+1] 612 216 4935
Linaro.org: Open Source Software for ARM SoCs

^ permalink raw reply

* [PATCH 2/2] cpu: update cpu_hotpluggable_mask in register_cpu
From: Mike Turquette @ 2011-08-10  1:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-pm, linaro-dev, patches
In-Reply-To: <1312940007-26069-1-git-send-email-mturquette@ti.com>

Update the cpu_hotpluggable_mask for each registered CPU which supports
hotplug.  This makes it trivial for kernel code to know which CPUs
support hotplug operations.

Signed-off-by: Mike Turquette <mturquette@ti.com>
---
 drivers/base/cpu.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 251acea..91ddcf8 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -224,8 +224,10 @@ int __cpuinit register_cpu(struct cpu *cpu, int num)
 
 	error = sysdev_register(&cpu->sysdev);
 
-	if (!error && cpu->hotpluggable)
+	if (!error && cpu->hotpluggable) {
 		register_cpu_control(cpu);
+		set_cpu_hotpluggable(num, true);
+	}
 	if (!error)
 		per_cpu(cpu_sys_devices, num) = &cpu->sysdev;
 	if (!error)
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 1/2] cpumask: introduce cpumask for hotpluggable CPUs
From: Mike Turquette @ 2011-08-10  1:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-pm, linaro-dev, patches
In-Reply-To: <1312940007-26069-1-git-send-email-mturquette@ti.com>

On some platforms it is possible to have some CPUs which support CPU
hotplug and some which do not.  Currently the prescence of an 'online'
sysfs entry in userspace is adequate for applications to know that a CPU
supports hotplug, but there is no convenient way to make the same
determination in the kernel.

To better model this relationship this patch introduces a new cpumask to
track CPUs that support CPU hotplug operations.

This new cpumask is populated at boot-time and remains static for the
life of the machine.  Bits set in the mask indicate a CPU which supports
hotplug, but make no guarantees about whether that CPU is currently
online or not.  Likewise a cleared bit in the mask indicates either a
CPU which cannot hotplug or a lack of a populated CPU.

The purpose of this new cpumask is to aid kernel code which uses CPU to
take CPUs online and offline.  Possible uses are as a thermal event
mitigation technique or as a power capping mechanism.

Signed-off-by: Mike Turquette <mturquette@ti.com>
---
 include/linux/cpumask.h |   27 ++++++++++++++++++++++-----
 kernel/cpu.c            |   18 ++++++++++++++++++
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index b24ac56..9eed444 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -39,10 +39,11 @@ extern int nr_cpu_ids;
  * The following particular system cpumasks and operations manage
  * possible, present, active and online cpus.
  *
- *     cpu_possible_mask- has bit 'cpu' set iff cpu is populatable
- *     cpu_present_mask - has bit 'cpu' set iff cpu is populated
- *     cpu_online_mask  - has bit 'cpu' set iff cpu available to scheduler
- *     cpu_active_mask  - has bit 'cpu' set iff cpu available to migration
+ *     cpu_possible_mask     - has bit 'cpu' set iff cpu is populatable
+ *     cpu_hotpluggable_mask - has bit 'cpu' set iff cpu is hotpluggable
+ *     cpu_present_mask      - has bit 'cpu' set iff cpu is populated
+ *     cpu_online_mask       - has bit 'cpu' set iff cpu available to scheduler
+ *     cpu_active_mask       - has bit 'cpu' set iff cpu available to migration
  *
  *  If !CONFIG_HOTPLUG_CPU, present == possible, and active == online.
  *
@@ -51,7 +52,11 @@ extern int nr_cpu_ids;
  *  life of that system boot.  The cpu_present_mask is dynamic(*),
  *  representing which CPUs are currently plugged in.  And
  *  cpu_online_mask is the dynamic subset of cpu_present_mask,
- *  indicating those CPUs available for scheduling.
+ *  indicating those CPUs available for scheduling.  The
+ *  cpu_hotpluggable_mask is also fixed at boot time, as the set of CPU
+ *  id's which are possible AND can hotplug.  Cleared bits in this mask
+ *  mean that either the CPU is not possible, or it is possible but does
+ *  not support CPU hotplug operations.
  *
  *  If HOTPLUG is enabled, then cpu_possible_mask is forced to have
  *  all NR_CPUS bits set, otherwise it is just the set of CPUs that
@@ -61,6 +66,9 @@ extern int nr_cpu_ids;
  *  depending on what ACPI reports as currently plugged in, otherwise
  *  cpu_present_mask is just a copy of cpu_possible_mask.
  *
+ *  If CONFIG_HOTPLUG_CPU is enabled, then cpu_hotpluggable_mask matches
+ *  the description above, otherwise it is the empty set.
+ *
  *  (*) Well, cpu_present_mask is dynamic in the hotplug case.  If not
  *      hotplug, it's a copy of cpu_possible_mask, hence fixed at boot.
  *
@@ -76,6 +84,7 @@ extern int nr_cpu_ids;
  */
 
 extern const struct cpumask *const cpu_possible_mask;
+extern const struct cpumask *const cpu_hotpluggable_mask;
 extern const struct cpumask *const cpu_online_mask;
 extern const struct cpumask *const cpu_present_mask;
 extern const struct cpumask *const cpu_active_mask;
@@ -85,19 +94,23 @@ extern const struct cpumask *const cpu_active_mask;
 #define num_possible_cpus()	cpumask_weight(cpu_possible_mask)
 #define num_present_cpus()	cpumask_weight(cpu_present_mask)
 #define num_active_cpus()	cpumask_weight(cpu_active_mask)
+#define num_hotpluggable_cpus()	cpumask_weight(cpu_hotpluggable_mask)
 #define cpu_online(cpu)		cpumask_test_cpu((cpu), cpu_online_mask)
 #define cpu_possible(cpu)	cpumask_test_cpu((cpu), cpu_possible_mask)
 #define cpu_present(cpu)	cpumask_test_cpu((cpu), cpu_present_mask)
 #define cpu_active(cpu)		cpumask_test_cpu((cpu), cpu_active_mask)
+#define cpu_hotpluggable(cpu)	cpumask_test_cpu((cpu, cpu_hotpluggable_mask)
 #else
 #define num_online_cpus()	1U
 #define num_possible_cpus()	1U
 #define num_present_cpus()	1U
 #define num_active_cpus()	1U
+#define num_hotpluggable_cpus()	0
 #define cpu_online(cpu)		((cpu) == 0)
 #define cpu_possible(cpu)	((cpu) == 0)
 #define cpu_present(cpu)	((cpu) == 0)
 #define cpu_active(cpu)		((cpu) == 0)
+#define cpu_hotpluggable(cpu)	0
 #endif
 
 /* verify cpu argument to cpumask_* operators */
@@ -678,16 +691,20 @@ extern const DECLARE_BITMAP(cpu_all_bits, NR_CPUS);
 #define cpu_none_mask to_cpumask(cpu_bit_bitmap[0])
 
 #define for_each_possible_cpu(cpu) for_each_cpu((cpu), cpu_possible_mask)
+#define for_each_hotpluggable_cpu(cpu) \
+				   for_each_cpu((cpu), cpu_hotpluggable_mask)
 #define for_each_online_cpu(cpu)   for_each_cpu((cpu), cpu_online_mask)
 #define for_each_present_cpu(cpu)  for_each_cpu((cpu), cpu_present_mask)
 
 /* Wrappers for arch boot code to manipulate normally-constant masks */
 void set_cpu_possible(unsigned int cpu, bool possible);
+void set_cpu_hotpluggable(unsigned int cpu, bool hotpluggable);
 void set_cpu_present(unsigned int cpu, bool present);
 void set_cpu_online(unsigned int cpu, bool online);
 void set_cpu_active(unsigned int cpu, bool active);
 void init_cpu_present(const struct cpumask *src);
 void init_cpu_possible(const struct cpumask *src);
+void init_cpu_hotpluggable(const struct cpumask *src);
 void init_cpu_online(const struct cpumask *src);
 
 /**
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 12b7458..8c397c9 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -536,6 +536,11 @@ static DECLARE_BITMAP(cpu_possible_bits, CONFIG_NR_CPUS) __read_mostly;
 const struct cpumask *const cpu_possible_mask = to_cpumask(cpu_possible_bits);
 EXPORT_SYMBOL(cpu_possible_mask);
 
+static DECLARE_BITMAP(cpu_hotpluggable_bits, CONFIG_NR_CPUS) __read_mostly;
+const struct cpumask *const cpu_hotpluggable_mask =
+					to_cpumask(cpu_hotpluggable_bits);
+EXPORT_SYMBOL(cpu_hotpluggable_mask);
+
 static DECLARE_BITMAP(cpu_online_bits, CONFIG_NR_CPUS) __read_mostly;
 const struct cpumask *const cpu_online_mask = to_cpumask(cpu_online_bits);
 EXPORT_SYMBOL(cpu_online_mask);
@@ -556,6 +561,14 @@ void set_cpu_possible(unsigned int cpu, bool possible)
 		cpumask_clear_cpu(cpu, to_cpumask(cpu_possible_bits));
 }
 
+void set_cpu_hotpluggable(unsigned int cpu, bool hotpluggable)
+{
+	if (hotpluggable)
+		cpumask_set_cpu(cpu, to_cpumask(cpu_hotpluggable_bits));
+	else
+		cpumask_clear_cpu(cpu, to_cpumask(cpu_hotpluggable_bits));
+}
+
 void set_cpu_present(unsigned int cpu, bool present)
 {
 	if (present)
@@ -590,6 +603,11 @@ void init_cpu_possible(const struct cpumask *src)
 	cpumask_copy(to_cpumask(cpu_possible_bits), src);
 }
 
+void init_cpu_hotpluggable(const struct cpumask *src)
+{
+	cpumask_copy(to_cpumask(cpu_hotpluggable_bits), src);
+}
+
 void init_cpu_online(const struct cpumask *src)
 {
 	cpumask_copy(to_cpumask(cpu_online_bits), src);
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 0/2] new cpumask for hotpluggable CPUs
From: Mike Turquette @ 2011-08-10  1:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-pm, linaro-dev, patches

This patch series introduces a new cpumask which tracks CPUs that
support hotplugging.  The purpose of this patch series is to provide a
simple method for kernel code to know which CPUs can be hotplugged and
which ones cannot.  Potential users of this code might be a thermal
mitigation technique which uses hotplug to lower temperature, or a power
capping mechanism which uses hotplug to lower power consumption.

All the of usual cpumask helper functions are created for this new mask.
The second patch in this series simply sets the bit for elligible CPUs
while they are being registered.  The cpumask itself is static after
boot and should not change (like the possbile mask).

Mike Turquette (2):
  cpumask: introduce cpumask for hotpluggable CPUs
  cpu: update cpu_hotpluggable_mask in register_cpu

 drivers/base/cpu.c      |    4 +++-
 include/linux/cpumask.h |   27 ++++++++++++++++++++++-----
 kernel/cpu.c            |   18 ++++++++++++++++++
 3 files changed, 43 insertions(+), 6 deletions(-)

-- 
1.7.4.1

^ permalink raw reply

* Re: [PATCH v3] PM: add statistics debugfs file for suspend to ram
From: Rafael J. Wysocki @ 2011-08-09 20:02 UTC (permalink / raw)
  To: yanmin_zhang
  Cc: Brown, Len, Liu, ShuoX, Greg KH, linux-kernel@vger.kernel.org,
	linux-pm@lists.linux-foundation.org
In-Reply-To: <1312868292.1743.15.camel@ymzhang>

On Tuesday, August 09, 2011, Yanmin Zhang wrote:
> On Tue, 2011-08-09 at 13:49 +0900, MyungJoo Ham wrote:
> > On Tue, Aug 9, 2011 at 11:27 AM, Liu, ShuoX <shuox.liu@intel.com> wrote:
> > > From: ShuoX Liu <shuox.liu@intel.com>
> > >
> > > Record S3 failure time about each reason and the latest two failed
> > > devices' names in S3 progress.
> > > We can check it through 'suspend_stats' entry in debugfs.
> > >
> > > The motivation of the patch:
> > >
> > > We are enabling power features on Medfield. Comparing with PC/notebook,
> > > a mobile enters/exits suspend-2-ram (we call it s3 on Medfield) far
> > > more frequently. If it can't enter suspend-2-ram in time, the power
> > > might be used up soon.
> > >
> > > We often find sometimes, a device suspend fails. Then, system retries
> > > s3 over and over again. As display is off, testers and developers
> > > don't know what happens.
> > >
> > > Some testers and developers complain they don't know if system
> > > tries suspend-2-ram, and what device fails to suspend. They need
> > > such info for a quick check. The patch adds suspend_stats under
> > > debugfs for users to check suspend to RAM statistics quickly.
> > >
> > > If not using this patch, we have other methods to get info about
> > > what device fails. One is to turn on  CONFIG_PM_DEBUG, but users
> > > would get too much info and testers need recompile the system.
> > >
> > > In addition, dynamic debug is another good tool to dump debug info.
> > > But it still doesn't match our utilization scenario closely.
> > > 1) user need write a user space parser to process the syslog output;
> > > 2) Our testing scenario is we leave the mobile for at least hours.
> > >   Then, check its status. No serial console available during the
> > >   testing. One is because console would be suspended, and the other
> > >   is serial console connecting with spi or HSU devices would consume
> > >   power. These devices are powered off at suspend-2-ram.
> > >
> > > Thank Greg and Rafael for their great comments.
> > >
> > > Change Log:
> > >  V3: Change some programming styles.
> > >  V2:
> > >        1) Change the sysfs entry to debugfs.
> > >        2) Add some explanation in document file.
> > >
> > > Contribution:
> > >        yanmin_zhang@linux.intel.com
> > >
> > > Signed-off-by: ShuoX Liu <shuox.liu@intel.com>
> > > ---
> > >  Documentation/power/basic-pm-debugging.txt |   19 +++++++++
> > >  drivers/base/power/main.c                  |   35 ++++++++++++++--
> > >  include/linux/suspend.h                    |   16 +++++++
> > >  kernel/power/main.c                        |   59 ++++++++++++++++++++++++++++
> > >  kernel/power/suspend.c                     |   15 ++++++-
> > >  5 files changed, 136 insertions(+), 8 deletions(-)
> > >
> > 
> > This patch looks very helpful. Great! :)
> > 
> > Anyway, how about including the error value induced by the device
> > along with the device name?
> > For example,
> > 
> > failed_devs:
> >          last_failed: alarm -5
> >                       adc -10
> > 
> > or
> > 
> > failed_devs:
> >          last_failed: alarm
> >                       adc
> >          last_errorno: -5
> >                       -10
> > 
> 
> > Or, even, we may consider including the failed step:
> > failed_devs:
> >          last_failed: alarm
> >                       adc
> >          last_errorno: -5
> >                       -10
> >          last_step: suspend
> >                       suspend_noirq
> > 
> It's a good idea! We would change it to output like above format.

OK, I'm expecting a new version of the patch.

Thanks,
Rafael

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox