Re: [PATCH 0/2 v6] printk, Add monotonic and real printk timestamps

[Prarit Bhargava]

On 04/19/2016 04:56 AM, Petr Mladek wrote:
> On Mon 2016-04-18 11:30:52, Prarit Bhargava wrote:
>>
>>
>> On 03/10/2016 05:00 AM, Petr Mladek wrote:
>>> On Tue 2016-03-08 06:03:24, Prarit Bhargava wrote:
>>>>
>>>>
>>>> On 03/08/2016 02:59 AM, Thomas Gleixner wrote:
>>>>> On Tue, 23 Feb 2016, Prarit Bhargava wrote:
>>>>>
>>>>>> This patchset adds monotonic and real printk timestamps.  The first patch
>>>>>> changes CONFIG_PRINT_TIME from a bool to an int to allow for the additional
>>>>>> timestamps that are added in patch 2.
>>>>>>
>>>>>> Changes from v6: Petr Mladek pointed out that the current patch
>>>>>> fails to indicate to userspace programs which timestamp is being used.
>>>>>
>>>>> How is that solved?
>>>>
>>>> Hi Thomas,
>>>>
>>>> Userspace programs can now look at /sys/modules/printk/parameters/time which
>>>> will contain [0-3] for the timestamp clock.
>>>
>>> But it includes only the current setting that is valid only for
>>> messages printed with this setting. The ring buffer might include
>>> different messages produced with different setting.
>>>
>>> I suggest to look how dmesg handles the time stamp. I wonder how it
>>> converts the relative time into an absolute one. I wonder if you
>>> could convert all timestamps to the relative format, so that you
>>> do not need to change all userspace tools at all.
>>
>> I looked into this and the only thing I can come up with is that I modify the
>> patchset to allow users to set the timestamp type at boot but not runtime.
>> Having to set it at runtime would require additional timestamp information be
>> added to the output of /dev/kmsg.
>>
>> Petr -- the dmesg code can be found in the utils-linux package,
>> sys-utils/dmesg.c.  The timestamping displaying code is this function:
>>
>> static struct tm *record_localtime(struct dmesg_control *ctl,
>>                                    struct dmesg_record *rec,
>>                                    struct tm *tm)
>> {
>>         time_t t = ctl->boot_time + rec->tv.tv_sec;
>>         return localtime_r(&t, tm);
>> }
>>
>> If you have any suggestion on how to modify it, I'm more than willing to do it.
>>  If not, then I suggest I change the code to make the timestamp RO during runtime.
> 
> Hmm, If you allow to change the timestamp format only at boot time, it
> will make things easier. I just wonder if it would work correctly for
> early messages. For example, are there any messages printed before
> the real time clock is initialized? Which timestamp will they use?

0.0000 is printed out before the clock is initialized.   I'll make sure it does
the right thing.

> 
> Also note that you still need to modify the dmesg code. It must
> not add boot_time when real time timestamp is used.
> 

Yep.  It looks like the output of the ctime and iso timestamps need to be verified.

> And you need to modify also the other tools, e.g. crash.

I'm going to talk with anderson@xxxxxxxxxx about this tomorrow.

P.
> 
> 
> Best Regards,
> Petr
>