[PATCH v2] power: Add warning message about incorrect system date and time after resume when pm_trace is enabled

[PATCH v2] power: Add warning message about incorrect system date and time after resume when pm_trace is enabled

[Shuah Khan]

pm_trace uses the system's Real Time Clock (RTC) to save the magic number.
Reason for this is that the RTC is the only reliably available piece of
hardware during resume operations where a value can be set that will
survive a reboot.

Consequence is that after a resume (even if it is successful) your system
clock will have a value corresponding to the magic number instead of the
correct date/time! It is therefore advisable to use a program like ntp-date
or rdate to reset the correct date/time from an external time source when
using this trace option.

There is no run-time message to warn users of the consequences of enabling
pm_trace. Adding a warning message to pm_trace_store() will serve as a
reminder to users to set the system date and time after resume.

Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
---
 kernel/power/main.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/power/main.c b/kernel/power/main.c
index d77663b..fd744d1 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -528,6 +528,8 @@ pm_trace_store(struct kobject *kobj, struct kobj_attribute *attr,
 
 	if (sscanf(buf, "%d", &val) == 1) {
 		pm_trace_enabled = !!val;
+		pr_warn("PM: Enabling pm_trace changes system date and time after resume.\n");
+		pr_cont("PM: Remember to set correct time after resume\n");
 		return n;
 	}
 	return -EINVAL;
-- 
1.7.10.4

Re: [PATCH v2] power: Add warning message about incorrect system date and time after resume when pm_trace is enabled

[Rafael J. Wysocki]

On Monday, June 24, 2013 05:25:18 PM Shuah Khan wrote:
> pm_trace uses the system's Real Time Clock (RTC) to save the magic number.
> Reason for this is that the RTC is the only reliably available piece of
> hardware during resume operations where a value can be set that will
> survive a reboot.
> 
> Consequence is that after a resume (even if it is successful) your system
> clock will have a value corresponding to the magic number instead of the
> correct date/time! It is therefore advisable to use a program like ntp-date
> or rdate to reset the correct date/time from an external time source when
> using this trace option.
> 
> There is no run-time message to warn users of the consequences of enabling
> pm_trace. Adding a warning message to pm_trace_store() will serve as a
> reminder to users to set the system date and time after resume.
> 
> Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
> ---
>  kernel/power/main.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index d77663b..fd744d1 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -528,6 +528,8 @@ pm_trace_store(struct kobject *kobj, struct kobj_attribute *attr,
>  
>  	if (sscanf(buf, "%d", &val) == 1) {
>  		pm_trace_enabled = !!val;
> +		pr_warn("PM: Enabling pm_trace changes system date and time after resume.\n");
> +		pr_cont("PM: Remember to set correct time after resume\n");

You can do it like this:

+	pr_warn("PM: Enabling pm_trace changes system date and time after resume.\n"
+		  "PM: Remember to set correct time after resume\n");

No need to use pr_cont(). :-)

And I'd say "during" instead of "after" in the first sentence.

I'd also say something like "Correct system time has to be restored manually after resume."
instead of the second one.

>  		return n;
>  	}
>  	return -EINVAL;

Thanks,
Rafael


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

Re: [PATCH v2] power: Add warning message about incorrect system date and time after resume when pm_trace is enabled

[Shuah Khan]

On 06/25/2013 05:01 PM, Rafael J. Wysocki wrote:
> On Monday, June 24, 2013 05:25:18 PM Shuah Khan wrote:
>> pm_trace uses the system's Real Time Clock (RTC) to save the magic number.
>> Reason for this is that the RTC is the only reliably available piece of
>> hardware during resume operations where a value can be set that will
>> survive a reboot.
>>
>> Consequence is that after a resume (even if it is successful) your system
>> clock will have a value corresponding to the magic number instead of the
>> correct date/time! It is therefore advisable to use a program like ntp-date
>> or rdate to reset the correct date/time from an external time source when
>> using this trace option.
>>
>> There is no run-time message to warn users of the consequences of enabling
>> pm_trace. Adding a warning message to pm_trace_store() will serve as a
>> reminder to users to set the system date and time after resume.
>>
>> Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
>> ---
>>   kernel/power/main.c |    2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/kernel/power/main.c b/kernel/power/main.c
>> index d77663b..fd744d1 100644
>> --- a/kernel/power/main.c
>> +++ b/kernel/power/main.c
>> @@ -528,6 +528,8 @@ pm_trace_store(struct kobject *kobj, struct kobj_attribute *attr,
>>
>>   	if (sscanf(buf, "%d", &val) == 1) {
>>   		pm_trace_enabled = !!val;
>> +		pr_warn("PM: Enabling pm_trace changes system date and time after resume.\n");
>> +		pr_cont("PM: Remember to set correct time after resume\n");
>
> You can do it like this:
>
> +	pr_warn("PM: Enabling pm_trace changes system date and time after resume.\n"
> +		  "PM: Remember to set correct time after resume\n");
>
> No need to use pr_cont(). :-)

:) Yeah.

>
> And I'd say "during" instead of "after" in the first sentence.
>
> I'd also say something like "Correct system time has to be restored manually after resume."
> instead of the second one.
>

Will do. I think I should add a check and print this warning when 
pm_trace is enabled and not when it is disabled. The way this patch is 
done, it prints this warning for enable and disable. For both:

echo 1 > pm_trace and echo 0 > pm_trace.

Any thoughts?

-- Shuah

Shuah Khan, Linux Kernel Developer - Open Source Group Samsung Research 
America (Silicon Valley) shuah.kh@samsung.com | (970) 672-0658

Re: [PATCH v2] power: Add warning message about incorrect system date and time after resume when pm_trace is enabled

[Rafael J. Wysocki]

On Wednesday, June 26, 2013 09:12:51 PM Shuah Khan wrote:
> On 06/25/2013 05:01 PM, Rafael J. Wysocki wrote:
> > On Monday, June 24, 2013 05:25:18 PM Shuah Khan wrote:
> >> pm_trace uses the system's Real Time Clock (RTC) to save the magic number.
> >> Reason for this is that the RTC is the only reliably available piece of
> >> hardware during resume operations where a value can be set that will
> >> survive a reboot.
> >>
> >> Consequence is that after a resume (even if it is successful) your system
> >> clock will have a value corresponding to the magic number instead of the
> >> correct date/time! It is therefore advisable to use a program like ntp-date
> >> or rdate to reset the correct date/time from an external time source when
> >> using this trace option.
> >>
> >> There is no run-time message to warn users of the consequences of enabling
> >> pm_trace. Adding a warning message to pm_trace_store() will serve as a
> >> reminder to users to set the system date and time after resume.
> >>
> >> Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
> >> ---
> >>   kernel/power/main.c |    2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/kernel/power/main.c b/kernel/power/main.c
> >> index d77663b..fd744d1 100644
> >> --- a/kernel/power/main.c
> >> +++ b/kernel/power/main.c
> >> @@ -528,6 +528,8 @@ pm_trace_store(struct kobject *kobj, struct kobj_attribute *attr,
> >>
> >>   	if (sscanf(buf, "%d", &val) == 1) {
> >>   		pm_trace_enabled = !!val;
> >> +		pr_warn("PM: Enabling pm_trace changes system date and time after resume.\n");
> >> +		pr_cont("PM: Remember to set correct time after resume\n");
> >
> > You can do it like this:
> >
> > +	pr_warn("PM: Enabling pm_trace changes system date and time after resume.\n"
> > +		  "PM: Remember to set correct time after resume\n");
> >
> > No need to use pr_cont(). :-)
> 
> :) Yeah.
> 
> >
> > And I'd say "during" instead of "after" in the first sentence.
> >
> > I'd also say something like "Correct system time has to be restored manually after resume."
> > instead of the second one.
> >
> 
> Will do. I think I should add a check and print this warning when 
> pm_trace is enabled and not when it is disabled. The way this patch is 
> done, it prints this warning for enable and disable. For both:
> 
> echo 1 > pm_trace and echo 0 > pm_trace.
> 
> Any thoughts?

Yeah, please only make it be printed when pm_trace is being enabled.

Thanks,
Rafael


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