linux-kernel - Re: [PATCH v5] printk: Add pr_info_show

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]
Message-ID: <e444a35b-6b69-db66-4e1e-df7f792245c0@android.com>
Date:   Fri, 21 Jul 2017 08:00:27 -0700
From:   Mark Salyzyn <salyzyn@...roid.com>
To:     "Luis R. Rodriguez" <mcgrof@...nel.org>
Cc:     linux-kernel@...r.kernel.org, pavel@....cz,
        andriy.shevchenko@...ux.intel.com, joe@...ches.com,
        prarit@...hat.com, rjw@...ysocki.net, tglx@...utronix.de,
        Petr Mladek <pmladek@...e.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Kees Cook <keescook@...omium.org>,
        Anton Vorontsov <anton@...msg.org>,
        Colin Cross <ccross@...roid.com>,
        Tony Luck <tony.luck@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Mark Salyzyn <salyzyn@...gle.com>,
        Nicholas Piggin <npiggin@...il.com>,
        Olof Johansson <olof@...om.net>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [PATCH v5] printk: Add pr_info_show_time

On 07/20/2017 07:00 PM, Luis R. Rodriguez wrote:
> On Thu, Jul 20, 2017 at 11:24:22AM -0700, Mark Salyzyn wrote:
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 98fe715522e8..0d63c3fb4e24 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -30,6 +30,58 @@ config CONSOLE_LOGLEVEL_DEFAULT
>>   	  usage in the kernel. That is controlled by the MESSAGE_LOGLEVEL_DEFAULT
>>   	  option.
>>   
>> +# set if time is being printed in pr_info_show_time()
>> +config PR_INFO_SHOW_TIME
>> +	bool
>> +
>> +choice
>> +	prompt "pr_info_show_time() alternate time message prefix"
>> +	help
>> +	  Activate alternate time prefix in pr_info_show_time
>> +
>> +	  The primary use of the instrumentation is to aid field
>> +	  analysis of Battery and Power usage.  The instrumentation
>> +	  may also help triage and synchronize kernel logs and user
>> +	  space activity logs at key displacements.
>> +	config PR_INFO_SHOW_TIME_MONOTONIC
>> +		bool "monotonic"
>> +		help
>> +		  Deactivate alternate time prefix in pr_info_show_time.
>> +		  Doing so because monotonic time is part of the normal
>> +		  printk time logging.
>> +
>> +		  Print only the supplied message in pr_info_show_time,
>> +		  indistinguishable from pr_info.
>> +	config PR_INFO_SHOW_TIME_REALTIME
>> +		bool "realtime"
>> +		select PR_INFO_SHOW_TIME
>> +		help
>> +		  Activate alternate time prefix in pr_info_show_time
>> +
>> +		  The primary use of the instrumentation is to aid field
>> +		  analysis of Battery and Power usage.  The instrumentation
>> +		  may also help triage and synchronize kernel logs and user
>> +		  space activity logs at key displacements.  For instance
>> +		  CLOCK_MONOTONIC stops while suspended, while CLOCK_REALTIME
>> +		  continues, and the timestamps help re-orient post-analysis.
>> +
>> +		  Prefix realtime [<epoch>.<ns>U] timestamp in pr_info_show_time
>> +	config PR_INFO_SHOW_TIME_BOOTTIME
>> +		bool "boottime"
>> +		select PR_INFO_SHOW_TIME
>> +		help
>> +		  Activate alternate time prefix in pr_info_show_time
>> +
>> +		  The primary use of the instrumentation is to aid field
>> +		  analysis of Battery and Power usage.  The instrumentation
>> +		  may also help triage and synchronize kernel logs and user
>> +		  space activity logs at key displacements.  For instance
>> +		  CLOCK_MONOTONIC stops while suspended, while CLOCK_BOOTTIME
>> +		  continues, and the timestamps help re-orient post-analysis.
>> +
>> +		  Prefix boottime [<epoch>.<ns>B] timestamp in pr_info_show_time
>> +endchoice
> There is no depends magic anywhere here, so none of this actually has complex
> dependencies, this is just boot time preference setup, and for that I think a
> boot param and sysctl value could easily not only enable the same but *also*
> enable run time ability to swap between these. Even for the branch performance
> consideration can't jump labels be used to address that if there is a concern
> for that?
Can we walk before we run? config parameters would still be needed to 
establish the default given that hibernate restore() happens before 
init() is even started. Boot parameters are on some Android platforms 
stretched to the limit and are under the control of the BootLoader which 
differs from the tools needs.

In Android's battery analysis case, there is no reason to change it at 
runtime, and would lead to confusion or DOSing of the facilities.

Once we are outside of the pr_info macros and the convenience of 
##__VA_ARGS__, we will have to code up vsprintf functionality and off to 
a buffer, so I am still pressuring myself to inline the facility even if 
we add boot params and/or sysctl.

Those changes can be added later?
> I think that would also make the code easier to read and remove all this kconfig
> extra stuff. Then its just a run time thing.
>
> Also, should there be no checks for time being available and ready before this
> is used?

Agreed, I started out in PM, RTC and others, it was safe. Once I declare 
a General Use function in printk.h I doomed the function(s) to abuse. 
(However, all the use cases are currently _only_ when time is 
available). Will look into protecting it.
>
>    Luis

-- Mark