Re: [PATCH] (was: Re: [PATCH v2] power: add knob for printing deviceresume times)

[Sameer Nanda]

On Sun, Jun 17, 2012 at 10:45 PM, Srivatsa S. Bhat
<srivatsa.bhat@xxxxxxxxxxxxxxxxxx> wrote:
> On 06/17/2012 02:06 AM, Rafael J. Wysocki wrote:
>
>> On Saturday, June 16, 2012, Rafael J. Wysocki wrote:
>>> On Friday, June 15, 2012, Greg KH wrote:
>>>> On Fri, Jun 15, 2012 at 12:00:20PM +0200, Rafael J. Wysocki wrote:
>>>>> On Friday, June 15, 2012, Greg KH wrote:
>>>>>> On Wed, May 23, 2012 at 09:45:32AM -0700, Sameer Nanda wrote:
>>>>>>> Added a new knob called /sys/power/pm_print_times. Setting it to 1
>>>>>>> enables printing of time taken by devices to suspend and resume.
>>>>>>> Setting it to 0 disables this printing (unless overridden by
>>>>>>> initcall_debug kernel command line option).
>>>>>>>
>>>>>>> Signed-off-by: Sameer Nanda <snanda@xxxxxxxxxxxx>
>>>>>>> cc: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
>>>>>>> cc: Rafael J. Wysocki <rjw@xxxxxxx>
>>>>>>> cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
>>>>>>> ---
>>>>>>> ÂDocumentation/ABI/testing/sysfs-power | Â 13 ++++++++++++
>>>>>>> Âdrivers/base/power/main.c       |  Â4 +-
>>>>>>> Âdrivers/base/power/power.h      Â|  11 ++++++++++
>>>>>>> Âkernel/power/main.c          |  34 +++++++++++++++++++++++++++++++++
>>>>>>> Â4 files changed, 60 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> This patch fails against the linux-next tree, care to fix it up so that
>>>>>> I can apply it?
>>>>>
>>>>> I'd prefer it to go through my tree if you don't mind.
>>>>
>>>> Not at all, feel free to put:
>>>>
>>>> Acked-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>>>>
>>>> When it shows up again.
>>>
>>> I've just used the original one, it only needed a documentation merge conflict
>>> fixed.
>>>
>>> Queued up for 3.6.
>>
>> I propose the following patch on top of this one. ÂThe details are in the
>> changelog.
>>
>> If anyone has any objections, please let me know.

No objections!  Thanks for picking this up.

>>
>> Thanks,
>> Rafael
>>
>> ---
>> From: Rafael J. Wysocki <rjw@xxxxxxx>
>> Subject: PM / Sleep: Separate printing suspend times from initcall_debug
>>
>> Change the behavior of the newly introduced
>> /sys/power/pm_print_times attribute so that its initial value
>> depends on initcall_debug, but setting it to 0 will cause device
>> suspend/resume times not to be printed, even if initcall_debug has
>> been set. ÂThis way, the people who use initcall_debug for reasons
>> other than PM debugging will be able to switch the suspend/resume
>> times printing off, if need be.
>>
>> Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx>
>
>
> Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@xxxxxxxxxxxxxxxxxx>

Reviewed-by: Sameer Nanda <snanda@xxxxxxxxxxxx>

>
> Regards,
> Srivatsa S. Bhat
>
>> ---
>> Âdrivers/base/power/main.c Â| Â Â4 ++--
>> Âdrivers/base/power/power.h | Â 11 -----------
>> Âinclude/linux/suspend.h  Â|  Â4 ++++
>> Âkernel/power/main.c    Â|  14 +++++++++++---
>> Â4 files changed, 17 insertions(+), 16 deletions(-)
>>
>> Index: linux/kernel/power/main.c
>> ===================================================================
>> --- linux.orig/kernel/power/main.c
>> +++ linux/kernel/power/main.c
>> @@ -139,7 +139,7 @@ power_attr(pm_test);
>> Â * show() returns whether printing of suspend and resume times is enabled.
>> Â * store() accepts 0 or 1. Â0 disables printing and 1 enables it.
>> Â */
>> -int pm_print_times_enabled;
>> +bool pm_print_times_enabled;
>>
>> Âstatic ssize_t pm_print_times_show(struct kobject *kobj,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âstruct kobj_attribute *attr, char *buf)
>> @@ -159,12 +159,19 @@ static ssize_t pm_print_times_store(stru
>> Â Â Â if (val > 1)
>> Â Â Â Â Â Â Â return -EINVAL;
>>
>> - Â Â pm_print_times_enabled = val;
>> + Â Â pm_print_times_enabled = !!val;
>> Â Â Â return n;
>> Â}
>>
>> +static inline void pm_print_times_init(void)
>> +{
>> + Â Â pm_print_times_enabled = !!initcall_debug;
>> +}
>> +
>> Âpower_attr(pm_print_times);
>> -#endif /* CONFIG_PM_DEBUG */
>> +#else /* !CONFIG_PM_DEBUG */
>> +static inline void pm_print_times_init(void) {}
>> +#endif /* !CONFIG_PM_DEBUG */
>>
>> Â#ifdef CONFIG_DEBUG_FS
>> Âstatic char *suspend_step_name(enum suspend_stat_step step)
>> @@ -599,6 +606,7 @@ static int __init pm_init(void)
>> Â Â Â error = sysfs_create_group(power_kobj, &attr_group);
>> Â Â Â if (error)
>> Â Â Â Â Â Â Â return error;
>> + Â Â pm_print_times_init();
>> Â Â Â return pm_autosleep_init();
>> Â}
>>
>> Index: linux/include/linux/suspend.h
>> ===================================================================
>> --- linux.orig/include/linux/suspend.h
>> +++ linux/include/linux/suspend.h
>> @@ -387,6 +387,8 @@ static inline void unlock_system_sleep(v
>> Â Â Â mutex_unlock(&pm_mutex);
>> Â}
>>
>> +extern bool pm_print_times_enabled;
>> +
>> Â#else /* !CONFIG_PM_SLEEP */
>>
>> Âstatic inline int register_pm_notifier(struct notifier_block *nb)
>> @@ -406,6 +408,8 @@ static inline bool pm_wakeup_pending(voi
>> Âstatic inline void lock_system_sleep(void) {}
>> Âstatic inline void unlock_system_sleep(void) {}
>>
>> +#define pm_print_times_enabled    (false)
>> +
>> Â#endif /* !CONFIG_PM_SLEEP */
>>
>> Â#ifdef CONFIG_PM_AUTOSLEEP
>> Index: linux/drivers/base/power/main.c
>> ===================================================================
>> --- linux.orig/drivers/base/power/main.c
>> +++ linux/drivers/base/power/main.c
>> @@ -166,7 +166,7 @@ static ktime_t initcall_debug_start(stru
>> Â{
>> Â Â Â ktime_t calltime = ktime_set(0, 0);
>>
>> - Â Â if (pm_print_times) {
>> + Â Â if (pm_print_times_enabled) {
>> Â Â Â Â Â Â Â pr_info("calling Â%s+ @ %i, parent: %s\n",
>> Â Â Â Â Â Â Â Â Â Â Â dev_name(dev), task_pid_nr(current),
>> Â Â Â Â Â Â Â Â Â Â Â dev->parent ? dev_name(dev->parent) : "none");
>> @@ -181,7 +181,7 @@ static void initcall_debug_report(struct
>> Â{
>> Â Â Â ktime_t delta, rettime;
>>
>> - Â Â if (pm_print_times) {
>> + Â Â if (pm_print_times_enabled) {
>> Â Â Â Â Â Â Â rettime = ktime_get();
>> Â Â Â Â Â Â Â delta = ktime_sub(rettime, calltime);
>> Â Â Â Â Â Â Â pr_info("call %s+ returned %d after %Ld usecs\n", dev_name(dev),
>> Index: linux/drivers/base/power/power.h
>> ===================================================================
>> --- linux.orig/drivers/base/power/power.h
>> +++ linux/drivers/base/power/power.h
>> @@ -85,14 +85,3 @@ static inline int pm_qos_sysfs_add(struc
>> Âstatic inline void pm_qos_sysfs_remove(struct device *dev) {}
>>
>> Â#endif
>> -
>> -#ifdef CONFIG_PM_DEBUG
>> -
>> -extern int pm_print_times_enabled;
>> -#define pm_print_times (initcall_debug || pm_print_times_enabled)
>> -
>> -#else /* CONFIG_PM_DEBUG */
>> -
>> -#define pm_print_times initcall_debug
>> -
>> -#endif /* CONFIG_PM_DEBUG */
>
>



-- 
Sameer
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/