Re: [PATCH] printk, allow different timestamps for printk.time

[Prarit Bhargava <prarit@redhat.com>]

On 01/26/2016 03:34 PM, Thomas Gleixner wrote:
> Prarit,
> 
> On Mon, 25 Jan 2016, Prarit Bhargava wrote:
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 87d40a7..2cf6692 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -3060,8 +3060,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>  			Format: <bool>  (1/Y/y=enable, 0/N/n=disable)
>>  			default: disabled
>>  
>> -	printk.time=	Show timing data prefixed to each printk message line
>> -			Format: <bool>  (1/Y/y=enable, 0/N/n=disable)
>> +	printk.time=	Show timestamp prefixed to each printk message line
>> +			Format: [0-2]  (0 = disable,
>> +					1 = monotonic clock, 2 = real clock)
> 
> Doesn't this break systems, which have 'Y/y' on the command line?

I'll fix this by converting to using module_param_cb() and comparing input
strings instead.

Aside: One thing I would also like to add is the ability to also do
printk.time=[real] for real timestamping.  I think that's easier to parse for
end users.

> 
>>  static DEFINE_RAW_SPINLOCK(timekeeper_lock);
>>  static struct timekeeper shadow_timekeeper;
>> +static int timekeeping_active;
>>  
>>  /**
>>   * struct tk_fast - NMI safe timekeeper
>> @@ -401,6 +402,16 @@ u64 ktime_get_raw_fast_ns(void)
>>  }
>>  EXPORT_SYMBOL_GPL(ktime_get_raw_fast_ns);
>>  
>> +u64 ktime_get_log_ts(u64 *offset_real)
>> +{
>> +	*offset_real = ktime_to_ns(tk_core.timekeeper.offs_real);
>> +
>> +	if (timekeeping_active)
>> +		return ktime_get_mono_fast_ns();
>> +	else
>> +		return local_clock();
> 
> Just for the record: The fast timekeeper stops across the inner workings of
> suspend/resume. But I think that doesn't matter much.

I've had to debug s/r before using the sysfs interface.  It is nice to have that
with a proper timestamp because a lot of the failures are in the driver suspend
and resume paths.  I could easily change the options to:

0 = disable
1 = local clock (which along with 0 would maintain the current state of things)
2 = monotonic
3 = real

P.

> 
> Otherwise, this looks good now.
> 
> Thanks,
> 
> 	tglx
> 
> 
> 
>