Re: [PATCH 2/2 v7] printk: Add monotonic, boottime, and realtime timestamps

From: John Stultz
Date: Wed Aug 23 2017 - 14:56:28 EST


On Wed, Aug 23, 2017 at 11:31 AM, Prarit Bhargava <prarit@xxxxxxxxxx> wrote:
> On 08/23/2017 04:45 AM, Petr Mladek wrote:
>> I know that it would make the code more complicated. But
>> I really like the approach used by /sys/power/disk or
>> /sys/power/pm_test. They list all possible values
>> and put the selected one into [] brackets.
>>
>
> I like the idea too, however, I think that complicates things with userspace
> (which as you point out is something that we really need to be concerned
> with). I am using /sys/modules/printk/parameters/time to indicate the
> timestamp to userspace.
>
> With my code, userspace would see "realtime" or "disabled". With your
> suggested change they would see "disabled [realtime]". I think that
> file needs to be a simple as possible and be a single entry for userspace
> to consume. IMHO.
>
>> Well, this might be done in a separate patch.
>> I am not 100% sure that this actually does not break some
>> rules for sysfs.
>>
>
> I don't see anything that says I must list every available option
> but I only did a quick overview of the docs.

I think the issue is that sysfs uses a "one value per file" rule. So
a value can be a list of possible options or the current option being
used, but mixing a list of options with some formatting to also
communicate the current option isn't strictly following the rule.
This is why we have available_clocksources and current_clocksource
entries.

That said, its not something that has been perfectly followed, so I'm
not sure how much of an issue it really is. I suspect GregKH would be
the gatekeeper on that.

thanks
-john