Re: [PATCH v2] power: Add warning message about incorrect system dateand 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@xxxxxxxxxxx>
>> ---
>>   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@xxxxxxxxxxx | (970) 672-0658
--
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/