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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 26 Jan 2016 15:52:21 -0500
From:	Prarit Bhargava <prarit@...hat.com>
To:	Thomas Gleixner <tglx@...utronix.de>
CC:	linux-kernel@...r.kernel.org, John Stultz <john.stultz@...aro.org>,
	Xunlei Pang <pang.xunlei@...aro.org>,
	Baolin Wang <baolin.wang@...aro.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Petr Mladek <pmladek@...e.cz>, Tejun Heo <tj@...nel.org>,
	Peter Hurley <peter@...leysoftware.com>,
	Vasily Averin <vvs@...tuozzo.com>,
	Joe Perches <joe@...ches.com>
Subject: Re: [PATCH] printk, allow different timestamps for printk.time



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
> 
> 
> 
>