Re: [PATCH] acpi, spcr: Make SPCR available to x86

2018-02-12 Thread Prarit Bhargava


On 02/12/2018 10:44 AM, Peter Zijlstra wrote:
> On Mon, Feb 12, 2018 at 10:18:06AM -0500, Prarit Bhargava wrote:
>>> But when I specify "earlyprintk=serial,ttyS0,115200" this SPCR crud will
>>> not interfere?
>>>
>>
>> I tested "earlyprintk=serial,ttyS0,115200" on a system which is known to 
>> have a
>> functional console with "console=ttyS0,1152008N1" both with and without
>> CONFIG_ACPI_SPCR_TABLE enabled.
> 
> OK, but my point was that SPCR will never override explicit earlyprintk
> stuff. There is absolutely nothing worse than breaking working setups.
> 

Oh, I see.  SPCR will not override earlyprintk config.

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] acpi, spcr: Make SPCR available to x86

2018-02-12 Thread Prarit Bhargava


On 02/12/2018 09:56 AM, Peter Zijlstra wrote:
> On Mon, Feb 12, 2018 at 08:47:57AM -0500, Prarit Bhargava wrote:
>>
>>
>> On 02/12/2018 08:34 AM, Peter Zijlstra wrote:
>>> On Thu, Jan 18, 2018 at 10:09:51AM -0500, Prarit Bhargava wrote:
>>>>  config ACPI_SPCR_TABLE
>>>> +  bool "ACPI Serial Port Console Redirection Support"
>>>> +  default y if X86
>>>> +  help
>>>> +Enable support for Serial Port Console Redirection (SPCR) Table.
>>>> +This table provides information about the configuration of the
>>>> +earlycon console.
>>>
>>> I just got asked this by oldconfig, which left me rather puzzled, WTF
>>> does this do?
>>
>> Odd -- I thought I had taken care of that :(  My apologies Peter.
>>
>> ACPI SPCR is used by a vendor to define the serial console for a system.  If
>> SPCR exists a user can add kernel parameter "earlycon" (no extra kernel
>> parameters) and the console will work out-of-the-box.
>>
>> The serial console configuration varies from vendor to vendor.  This takes 
>> the
>> guess-work out of defining a serial console.
> 
> But when I specify "earlyprintk=serial,ttyS0,115200" this SPCR crud will
> not interfere?
> 

I tested "earlyprintk=serial,ttyS0,115200" on a system which is known to have a
functional console with "console=ttyS0,1152008N1" both with and without
CONFIG_ACPI_SPCR_TABLE enabled.

I do not see any significant difference in output.

diff of boot logs:

--- earlyprintk_no_spcr_enable  2018-02-12 10:11:00.477288423 -0500
+++ earlyprintk_spcr_enable 2018-02-12 10:16:02.282574797 -0500
@@ -1,5 +1,5 @@
 microcode: microcode updated early to revision 0x235, date = 2017-10-17
-Linux version 4.15.0+ (r...@intel-purley-lr-02.khw.lab.eng.bos.redhat.com) (gcc
version 4.8.5 20150623 (Red Hat 4.8.5-28) (GCC)) #5 SMP Mon Feb 12 10:06:08 EST 
2018
+Linux version 4.15.0+ (r...@intel-purley-lr-02.khw.lab.eng.bos.redhat.com) (gcc
version 4.8.5 20150623 (Red Hat 4.8.5-28) (GCC)) #6 SMP Mon Feb 12 10:11:42 EST 
2018
 Command line: BOOT_IMAGE=/vmlinuz-4.15.0+
root=/dev/mapper/rhel_intel--purley--lr--02-root ro crashkernel=auto
rd.lvm.lv=rhel_intel-purley-lr-02/root rd.lvm.lv=rhel_intel-purley-lr-02/swap
earlyprintk=serial,ttyS0,115200 LANG=en_US.UTF-8
 x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point registers'
 x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
@@ -205,7 +205,7 @@
 e820: last_pfn = 0x6f800 max_arch_pfn = 0x4
 Using GB pages for direct mapping
 Secure boot disabled
-RAMDISK: [mem 0x3a563000-0x3bba2fff]
+RAMDISK: [mem 0x3a5da000-0x3bba2fff]
 ACPI: Early table checksum verification disabled
 ACPI: RSDP 0x6922A014 24 (v02 INTEL )
 ACPI: XSDT 0x690DD188 000114 (v01 INTEL  EDK2   
0113)
@@ -519,6 +519,8 @@
 ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
 Using ACPI (MADT) for SMP configuration information
 ACPI: HPET id: 0x8086a701 base: 0xfed0
+ACPI: SPCR: SPCR table version 1
+ACPI: SPCR: console: uart,io,0x3f8,115200
 smpboot: Allowing 432 CPUs, 224 hotplug CPUs
 PM: Registered nosave memory: [mem 0x-0x0fff]
 PM: Registered nosave memory: [mem 0x000a-0x000f]
@@ -555,10 +557,10 @@
 log_buf_len total cpu_extra contributions: 1765376 bytes
 log_buf_len min size: 524288 bytes
 log_buf_len: 4194304 bytes
-early log buf free: 477984(91%)
-Memory: 394593504K/401282564K available (10252K kernel code, 2147K rwdata,
3664K rodata, 2504K init, 1800K bss, 6689060K reserved, 0K cma-reserved)
+early log buf free: 477876(91%)
+Memory: 394593976K/401282564K available (10252K kernel code, 2147K rwdata,
3664K rodata, 2504K init, 1800K bss, 6688588K reserved, 0K cma-reserved)
 Kernel/User page tables isolation: enabled
-ftrace: allocating 35205 entries in 138 pages
+ftrace: allocating 35206 entries in 138 pages
 Hierarchical RCU implementation.
RCU restricting CPUs from NR_CPUS=8192 to nr_cpu_ids=432.
Tasks RCU enabled.

P.

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] acpi, spcr: Make SPCR available to x86

2018-02-12 Thread Prarit Bhargava


On 02/12/2018 09:43 AM, Timur Tabi wrote:
> On 2/12/18 7:47 AM, Prarit Bhargava wrote:
>> ACPI SPCR is used by a vendor to define the serial console for a system.  If
>> SPCR exists a user can add kernel parameter "earlycon" (no extra kernel
>> parameters) and the console will work out-of-the-box.
> 
> "earlycon" is needed only for an *early* console.  You don't need any command
> line parameters for a normal console to work with SPCR.

That's true on ARM64.  I specifically did not enable the console by default on
x86.  There are users who do not want a console active during boot & runtime.

P.

> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] acpi, spcr: Make SPCR available to x86

2018-02-12 Thread Prarit Bhargava


On 02/12/2018 08:34 AM, Peter Zijlstra wrote:
> On Thu, Jan 18, 2018 at 10:09:51AM -0500, Prarit Bhargava wrote:
>>  config ACPI_SPCR_TABLE
>> +bool "ACPI Serial Port Console Redirection Support"
>> +default y if X86
>> +help
>> +  Enable support for Serial Port Console Redirection (SPCR) Table.
>> +  This table provides information about the configuration of the
>> +  earlycon console.
> 
> I just got asked this by oldconfig, which left me rather puzzled, WTF
> does this do?

Odd -- I thought I had taken care of that :(  My apologies Peter.

ACPI SPCR is used by a vendor to define the serial console for a system.  If
SPCR exists a user can add kernel parameter "earlycon" (no extra kernel
parameters) and the console will work out-of-the-box.

The serial console configuration varies from vendor to vendor.  This takes the
guess-work out of defining a serial console.

P.

> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] acpi, spcr: Make SPCR available to x86

2018-01-23 Thread Prarit Bhargava


On 01/22/2018 04:49 PM, Timur Tabi wrote:
> On 01/18/2018 09:09 AM, Prarit Bhargava wrote:
>>   if (acpi_disabled) {
>> -if (earlycon_init_is_deferred)
>> +if (earlycon_acpi_spcr_enable)
> 
> This patch works for me, so I can ACK it, but first you might want to rename
> earlycon_acpi_spcr_enable, because these two lines don't make much sense.
> 
> "If ACPI is disabled, and ACPI SCPR is enabled, then "
> 
> If ACPI is disabled, then how can a variable called 
> "earlycon_acpi_spcr_enable"
> be true?
> 
> Would it make more sense to rename it to earlycon_spcr_enable?
> 

acpi_disabled is a global runtime flag that can be set via "acpi=off" on the
command line.  It does not disable the tables, only the reading and interpreting
of the data in the tables.

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] acpi, spcr: Make SPCR available to x86

2018-01-18 Thread Prarit Bhargava
Note on testing:  I've tested this on several ARM64 and x86 boxes (only
earlycon, console=ttyS0,115200, and with both options), verified that
functionality on ARM64 has not changed (ie, CONFIG_ACPI_SPCR_TABLE is
always =y), and verified functionality when !CONFIG_ACPI_SPCR_TABLE on
x86.

P.

8<

SPCR is currently only enabled or ARM64 and x86 can use SPCR to setup an
early console.

General fixes include updating Documentation & Kconfig (for x86), updating
comments, and changing parse_spcr() to acpi_parse_spcr(), and
earlycon_init_is_deferred to earlycon_acpi_spcr_enable to be more
descriptive.

On x86, many systems have a valid SPCR table but the table version is not
2 so the table version check must be a warning.

On ARM64 when the kernel parameter earlycon is used both the early console
and console are enabled.  On x86, only the earlycon should be enabled by
by default.  Modify acpi_parse_spcr() to allow options for initializing
the early console and console separately.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: linux-a...@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux...@vger.kernel.org
Cc: linux-ser...@vger.kernel.org
Cc: Bhupesh Sharma <bhsha...@redhat.com>
Cc: Lv Zheng <lv.zh...@intel.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: x...@kernel.org
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Catalin Marinas <catalin.mari...@arm.com>
Cc: Will Deacon <will.dea...@arm.com>
Cc: "Rafael J. Wysocki" m...@rjwysocki.net>
Cc: Timur Tabi <ti...@codeaurora.org>
Cc: graeme.greg...@linaro.org
Cc: mark.sal...@redhat.com
---
 Documentation/admin-guide/kernel-parameters.txt |  9 +---
 arch/arm64/kernel/acpi.c|  4 ++--
 arch/x86/kernel/acpi/boot.c |  3 +++
 drivers/acpi/Kconfig|  7 +-
 drivers/acpi/spcr.c | 29 +
 drivers/tty/serial/earlycon.c   | 15 +
 include/linux/acpi.h|  7 --
 include/linux/serial_core.h |  4 ++--
 8 files changed, 44 insertions(+), 34 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 46b26bfee27b..6f519230ed34 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -915,9 +915,12 @@
 
earlycon=   [KNL] Output early console device and options.
 
-   When used with no options, the early console is
-   determined by the stdout-path property in device
-   tree's chosen node.
+   [ARM64] The early console is determined by the
+   stdout-path property in device tree's chosen node,
+   or determined by the ACPI SPCR table.
+
+   [X86] When used with no options the early console is
+   determined by the ACPI SPCR table.
 
cdns,[,options]
Start an early, polled-mode console on a Cadence
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3162715ed78..2aa5609def27 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -230,10 +230,10 @@ void __init acpi_boot_table_init(void)
 
 done:
if (acpi_disabled) {
-   if (earlycon_init_is_deferred)
+   if (earlycon_acpi_spcr_enable)
early_init_dt_scan_chosen_stdout();
} else {
-   parse_spcr(earlycon_init_is_deferred);
+   acpi_parse_spcr(earlycon_acpi_spcr_enable, true);
if (IS_ENABLED(CONFIG_ACPI_BGRT))
acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
}
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index f4c463df8b08..0bf6a58f7a6d 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1626,6 +1627,8 @@ int __init acpi_boot_init(void)
if (!acpi_noirq)
x86_init.pci.init = pci_acpi_init;
 
+   /* Do not enable ACPI SPCR console by default */
+   acpi_parse_spcr(earlycon_acpi_spcr_enable, false);
return 0;
 }
 
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 46505396869e..ddcb5f40e8ee 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
 endif
 
 config ACPI_SPCR_TABLE
-   bool
+   bool "ACPI Serial Port Console Redirection Support"
+   default y if X86
+   help
+ Enable support for Serial Port Console Redirection (SPCR) Table.
+ 

Re: [PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

2018-01-08 Thread Prarit Bhargava
[Sorry everyone for the late response, I went away on vacation and pushed this
off until I returned.]

On 12/13/2017 07:45 AM, Lorenzo Pieralisi wrote:
> [+Mark, Graeme]
> 
> In $SUBJECT, s/avialable/available
> 
> On Mon, Dec 11, 2017 at 10:50:58AM -0500, Prarit Bhargava wrote:
>> Other architectures can use SPCR to setup an early console or console
>> but the current code is ARM64 specific.
> 
> I see nothing ARM64 specific in current code (apart from some
> ACPICA macros with an ARM tag in them) please explain to me
> what's preventing you to reuse current code on x86.

Ah, I didn't notice that.  I thought the ACPICA macros were ARM specific.  I'll
rework this patchset with that in mind.

> 
>> Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
>> function acpi_arch_setup_console() that can be used for arch-specific
>> setup.  Move flags into ACPI code.  Update the Documention on the use of
>> the SPCR.
>>
>> [v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
>> Keep ACPI_SPCR_TABLE selected for ARM64.  Fix 8-bit port access width
>> mmio value.  Move baud rate check earlier.
> 
> This does not belong in the commit log.

Yes, but some maintainers like to see what changed between v1 & v2.

> 
>> Signed-off-by: Prarit Bhargava <pra...@redhat.com>
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-ker...@vger.kernel.org
>> Cc: linux-arm-ker...@lists.infradead.org
>> Cc: linux...@vger.kernel.org
>> Cc: linux-a...@vger.kernel.org
>> Cc: linux-ser...@vger.kernel.org
>> Cc: Bhupesh Sharma <bhsha...@redhat.com>
>> Cc: Lv Zheng <lv.zh...@intel.com>
>> Cc: Thomas Gleixner <t...@linutronix.de>
>> Cc: Ingo Molnar <mi...@redhat.com>
>> Cc: "H. Peter Anvin" <h...@zytor.com>
>> Cc: x...@kernel.org
>> Cc: Jonathan Corbet <cor...@lwn.net>
>> Cc: Catalin Marinas <catalin.mari...@arm.com>
>> Cc: Will Deacon <will.dea...@arm.com>
>> Cc: "Rafael J. Wysocki" <r...@rjwysocki.net>
>> Cc: Timur Tabi <ti...@codeaurora.org>
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt |   6 +-
>>  arch/arm64/kernel/acpi.c| 128 -
>>  drivers/acpi/Kconfig|   7 +-
>>  drivers/acpi/spcr.c | 175 
>> ++--
>>  drivers/tty/serial/earlycon.c   |  15 +-
>>  include/linux/acpi.h|  11 +-
>>  include/linux/serial_core.h |   2 -
>>  7 files changed, 184 insertions(+), 160 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
>> b/Documentation/admin-guide/kernel-parameters.txt
>> index 6571fbfdb2a1..0d173289c67e 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -914,9 +914,9 @@
>>  
>>  earlycon=   [KNL] Output early console device and options.
>>  
>> -When used with no options, the early console is
>> -determined by the stdout-path property in device
>> -tree's chosen node.
>> +[ARM64] The early console is determined by the
>> +stdout-path property in device tree's chosen node,
>> +or determined by the ACPI SPCR table.
>>  
>>  cdns,[,options]
>>  Start an early, polled-mode console on a Cadence
>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>> index b3162715ed78..b3e33bbdf3b7 100644
>> --- a/arch/arm64/kernel/acpi.c
>> +++ b/arch/arm64/kernel/acpi.c
>> @@ -25,7 +25,6 @@
>>  #include 
>>  #include 
>>  #include 
>> -#include 
>>  
>>  #include 
>>  #include 
>> @@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
>>  return ret;
>>  }
>>  
>> +/*
>> + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
>> + * occasionally getting stuck as 1. To avoid the potential for a hang, check
>> + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
>> + * implementations, so only do so if an affected platform is detected in
>> + * acpi_parse_spcr().
>> + */
>> +bool qdf2400_e44_present;
>> +EXPORT_SYMBOL(qdf2400_e44_present);
> 
> My eyes, this is horrible but it is not introduced by this patch. It
> would have been much better if:
> 
> drivers/tty/serial/amba-pl011.c
> 
> parse

[PATCH] x86: Add topology_hw_smt_threads() and remove smp_num_siblings

2018-01-04 Thread Prarit Bhargava
Commit bbb65d2d365e ("x86: use cpuid vector 0xb when available for
detecting cpu topology") changed the value of smp_num_siblings from the
active number of threads in a core to the maximum number threads in a
core.  e.g.) On Intel Haswell and newer systems smp_num_siblings is
two even if SMT is disabled.

topology_max_smt_threads() already returns the active number of threads.
Introduce topology_hw_smt_threads() which returns the maximum number of
threads.  These are used to fix and replace references to smp_num_siblings.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: x...@kernel.org
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Andi Kleen <a...@linux.intel.com>
Cc: Vitaly Kuznetsov <vkuzn...@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-e...@vger.kernel.org
Cc: oprofile-l...@lists.sf.net
---
 Documentation/x86/topology.txt  | 13 +
 arch/x86/include/asm/perf_event_p4.h|  4 ++--
 arch/x86/include/asm/smp.h  |  2 --
 arch/x86/include/asm/topology.h | 10 +-
 arch/x86/kernel/cpu/amd.c   |  6 ++
 arch/x86/kernel/cpu/common.c| 18 +++---
 arch/x86/kernel/cpu/mcheck/mce-inject.c |  3 ++-
 arch/x86/kernel/cpu/topology.c  |  4 ++--
 arch/x86/kernel/itmt.c  |  2 +-
 arch/x86/kernel/process.c   |  3 ++-
 arch/x86/kernel/smpboot.c   | 22 --
 arch/x86/oprofile/nmi_int.c |  2 +-
 arch/x86/oprofile/op_model_p4.c |  4 ++--
 13 files changed, 47 insertions(+), 46 deletions(-)

diff --git a/Documentation/x86/topology.txt b/Documentation/x86/topology.txt
index f3e9d7e9ed6c..9fa07a4460df 100644
--- a/Documentation/x86/topology.txt
+++ b/Documentation/x86/topology.txt
@@ -83,13 +83,18 @@ The topology of a system is described in the units of:
 
   Core-related topology information in the kernel:
 
-  - smp_num_siblings:
+  - topology_hw_smt_threads:
 
-The number of threads in a core. The number of threads in a package can be
-calculated by:
+The maximum number of threads that a core's hardware supports.  For
+example, on Intel Haswell and newer systems this is 2 even if SMT is
+disabled.
 
-   threads_per_package = cpuinfo_x86.x86_max_cores * smp_num_siblings
+  - topology_max_smt_threads:
 
+The number of threads/core available at runtime.  The number of threads in
+a package can be calculated by:
+
+threads_per_package = cpuinfo_x86.x86_max_cores * topology_max_smt_threads
 
 * Threads:
 
diff --git a/arch/x86/include/asm/perf_event_p4.h 
b/arch/x86/include/asm/perf_event_p4.h
index 94de1a05aeba..11afdadce9c2 100644
--- a/arch/x86/include/asm/perf_event_p4.h
+++ b/arch/x86/include/asm/perf_event_p4.h
@@ -181,7 +181,7 @@ static inline u64 p4_clear_ht_bit(u64 config)
 static inline int p4_ht_active(void)
 {
 #ifdef CONFIG_SMP
-   return smp_num_siblings > 1;
+   return topology_max_smt_threads() > 1;
 #endif
return 0;
 }
@@ -189,7 +189,7 @@ static inline int p4_ht_active(void)
 static inline int p4_ht_thread(int cpu)
 {
 #ifdef CONFIG_SMP
-   if (smp_num_siblings == 2)
+   if (topology_max_smt_threads() == 2)
return cpu != 
cpumask_first(this_cpu_cpumask_var_ptr(cpu_sibling_map));
 #endif
return 0;
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 461f53d27708..cf28a3932917 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -18,7 +18,6 @@
 #include 
 #include 
 
-extern int smp_num_siblings;
 extern unsigned int num_processors;
 
 DECLARE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_map);
@@ -170,7 +169,6 @@ static inline int wbinvd_on_all_cpus(void)
wbinvd();
return 0;
 }
-#define smp_num_siblings   1
 #endif /* CONFIG_SMP */
 
 extern unsigned disabled_cpus;
diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index c1d2a9892352..b5ff1c784eef 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -116,16 +116,16 @@ extern unsigned int __max_logical_packages;
 #define topology_max_packages()(__max_logical_packages)
 
 extern int __max_smt_threads;
-
-static inline int topology_max_smt_threads(void)
-{
-   return __max_smt_threads;
-}
+#define topology_max_smt_threads() (__max_smt_threads)
+extern int __hw_smt_threads;
+#define topology_hw_smt_threads()  (__hw_smt_threads)
 
 int topology_update_package_map(unsigned int apicid, unsigned int cpu);
 extern int topology_phys_to_logical_pkg(unsigned int pkg);
 #else
 #define topology_max_packages()(1)
+#define topology_max_smt_threads() (1)
+#define topology_hw_smt_threads()  (1)
 static inline int
 topology_

[PATCH v2 2/2] acpi, x86: Use SPCR table for earlycon on x86

2017-12-11 Thread Prarit Bhargava
The ACPI SPCR code has been used to define an earlycon console for ARM64
and can be used for x86.

Modify the ACPI SPCR parsing code to account for console behaviour
differences between ARM64 and x86.  Initialize the SPCR code from
x86 ACPI initialization code.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux...@vger.kernel.org
Cc: linux-a...@vger.kernel.org
Cc: linux-ser...@vger.kernel.org
Cc: Bhupesh Sharma <bhsha...@redhat.com>
Cc: Lv Zheng <lv.zh...@intel.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: x...@kernel.org
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Catalin Marinas <catalin.mari...@arm.com>
Cc: Will Deacon <will.dea...@arm.com>
Cc: "Rafael J. Wysocki" <r...@rjwysocki.net>
Cc: Timur Tabi <ti...@codeaurora.org>
---
 Documentation/admin-guide/kernel-parameters.txt | 3 +++
 arch/arm64/kernel/acpi.c| 2 +-
 arch/x86/kernel/acpi/boot.c | 4 
 drivers/acpi/Kconfig| 2 +-
 drivers/acpi/spcr.c | 7 +--
 include/linux/acpi.h| 7 +--
 6 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 0d173289c67e..c7cc890a0e81 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -918,6 +918,9 @@
stdout-path property in device tree's chosen node,
or determined by the ACPI SPCR table.
 
+   [X86] When used with no options the early console is
+   determined by the ACPI SPCR table.
+
cdns,[,options]
Start an early, polled-mode console on a Cadence
(xuartps) serial port at the specified address. Only
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3e33bbdf3b7..aaee4864b63e 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -355,7 +355,7 @@ void __init acpi_boot_table_init(void)
early_init_dt_scan_chosen_stdout();
} else {
/* Always enable the ACPI SPCR console */
-   acpi_parse_spcr(console_acpi_spcr_enable);
+   acpi_parse_spcr(console_acpi_spcr_enable, true);
if (IS_ENABLED(CONFIG_ACPI_BGRT))
acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
}
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index f4c463df8b08..f01a03643cff 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1626,6 +1627,9 @@ int __init acpi_boot_init(void)
if (!acpi_noirq)
x86_init.pci.init = pci_acpi_init;
 
+   /* Do not enable ACPI SPCR console by default */
+   acpi_parse_spcr(console_acpi_spcr_enable, console_acpi_spcr_enable);
+
return 0;
 }
 
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 9ae98eeada76..2d4e841e0682 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -80,7 +80,7 @@ endif
 
 config ACPI_SPCR_TABLE
bool "ACPI Serial Port Console Redirection Support"
-   default y if ARM64
+   default y if (X86 || ARM64)
help
  Enable support for Serial Port Console Redirection (SPCR) Table.
  This table provides information about the configuration of the
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index f4bb8110e404..c9469b488527 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -39,7 +39,7 @@ bool console_acpi_spcr_enable __initdata;
  * from arch initialization code as soon as the DT/ACPI decision is made.
  *
  */
-int __init acpi_parse_spcr(bool earlycon)
+int __init acpi_parse_spcr(bool earlycon, bool enable_console)
 {
static char opts[ACPI_SPCR_OPTS_SIZE];
static char uart[ACPI_SPCR_BUF_SIZE];
@@ -112,7 +112,10 @@ int __init acpi_parse_spcr(bool earlycon)
if (earlycon)
setup_earlycon(opts);
 
-   err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
+   if (enable_console)
+   err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
+   else
+   err = 0;
 done:
acpi_put_table((struct acpi_table_header *)table);
return err;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 875d7327d91c..4c2d449bab9b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1248,10 +1248,13 @@ extern bool console_acpi_spcr_enable __initdata;
 extern int acpi_arch_setup_

[PATCH v2 0/2] acpi, x86: Add SPCR table support

2017-12-11 Thread Prarit Bhargava
The SPCR (Serial Port Console Redirection) Table provides information
about the configuration of the serial port.  This information can be used to
configure the early console.

SPCR support was added for ARM64 and is made available across all arches
in this patchset.  The first patch adds a weak per-arch configuration function
and moves the SPCR code into ACPI.  The second patch adds support to x86.

The existing behaviour on ARM64 is maintained.  If the SPCR exists the
earlycon and console are automatically configured.

The existing default behaviour on x86 is also maintained.  If no console or
earlycon parameter is defined and the SPCR exists, the serial port is not
configured.  If the earlycon parameter is used both the early console
and the console are configured using the data from the SPCR.

Additional testing (requested by Timur): On Gigabyte (no quirks), Qualcomm
Amberwing systems to test Qualcomm quirks, HPE Mantis system to test xgene
quirks.  Tested several x86 systems with and without a SPCR to confirm
no changes in default behaviour.

[v2]: See 1/2 for code changes.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux...@vger.kernel.org
Cc: linux-a...@vger.kernel.org
Cc: linux-ser...@vger.kernel.org
Cc: Bhupesh Sharma <bhsha...@redhat.com>
Cc: Lv Zheng <lv.zh...@intel.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: x...@kernel.org
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Catalin Marinas <catalin.mari...@arm.com>
Cc: Will Deacon <will.dea...@arm.com>
Cc: "Rafael J. Wysocki" <r...@rjwysocki.net>
Cc: Timur Tabi <ti...@codeaurora.org>

Prarit Bhargava (2):
  acpi, spcr: Make SPCR avialable to other architectures
  acpi, x86: Use SPCR table for earlycon on x86

 Documentation/admin-guide/kernel-parameters.txt |   9 +-
 arch/arm64/kernel/acpi.c| 128 -
 arch/x86/kernel/acpi/boot.c |   4 +
 drivers/acpi/Kconfig|   7 +-
 drivers/acpi/spcr.c | 180 ++--
 drivers/tty/serial/earlycon.c   |  15 +-
 include/linux/acpi.h|  14 +-
 include/linux/serial_core.h |   2 -
 8 files changed, 198 insertions(+), 161 deletions(-)

-- 
2.15.0.rc0.39.g2f0e14e64

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

2017-12-11 Thread Prarit Bhargava
Other architectures can use SPCR to setup an early console or console
but the current code is ARM64 specific.

Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
function acpi_arch_setup_console() that can be used for arch-specific
setup.  Move flags into ACPI code.  Update the Documention on the use of
the SPCR.

[v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
Keep ACPI_SPCR_TABLE selected for ARM64.  Fix 8-bit port access width
mmio value.  Move baud rate check earlier.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux...@vger.kernel.org
Cc: linux-a...@vger.kernel.org
Cc: linux-ser...@vger.kernel.org
Cc: Bhupesh Sharma <bhsha...@redhat.com>
Cc: Lv Zheng <lv.zh...@intel.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: x...@kernel.org
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Catalin Marinas <catalin.mari...@arm.com>
Cc: Will Deacon <will.dea...@arm.com>
Cc: "Rafael J. Wysocki" <r...@rjwysocki.net>
Cc: Timur Tabi <ti...@codeaurora.org>
---
 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 arch/arm64/kernel/acpi.c| 128 -
 drivers/acpi/Kconfig|   7 +-
 drivers/acpi/spcr.c | 175 ++--
 drivers/tty/serial/earlycon.c   |  15 +-
 include/linux/acpi.h|  11 +-
 include/linux/serial_core.h |   2 -
 7 files changed, 184 insertions(+), 160 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 6571fbfdb2a1..0d173289c67e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -914,9 +914,9 @@
 
earlycon=   [KNL] Output early console device and options.
 
-   When used with no options, the early console is
-   determined by the stdout-path property in device
-   tree's chosen node.
+   [ARM64] The early console is determined by the
+   stdout-path property in device tree's chosen node,
+   or determined by the ACPI SPCR table.
 
cdns,[,options]
Start an early, polled-mode console on a Cadence
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3162715ed78..b3e33bbdf3b7 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -25,7 +25,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
return ret;
 }
 
+/*
+ * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
+ * occasionally getting stuck as 1. To avoid the potential for a hang, check
+ * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
+ * implementations, so only do so if an affected platform is detected in
+ * acpi_parse_spcr().
+ */
+bool qdf2400_e44_present;
+EXPORT_SYMBOL(qdf2400_e44_present);
+
+/*
+ * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
+ * Detect them by examining the OEM fields in the SPCR header, similar to PCI
+ * quirk detection in pci_mcfg.c.
+ */
+static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
+{
+   if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
+   return false;
+
+   if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
+   return true;
+
+   if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
+   h->oem_revision == 1)
+   return true;
+
+   return false;
+}
+
+/*
+ * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
+ * register aligned to 32-bit. In addition, the BIOS also encoded the
+ * access width to be 8 bits. This function detects this errata condition.
+ */
+static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
+{
+   bool xgene_8250 = false;
+
+   if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
+   return false;
+
+   if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
+   memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
+   return false;
+
+   if (!memcmp(tb->header.oem_table_id, "XGENESPC",
+   ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
+   xgene_8250 = true;
+
+   if (!memcmp(tb->header.oem_table_id, "ProLiant",
+   ACPI_OEM_TABLE_ID_SI

Re: [PATCH 1/2] acpi, spcr: Make SPCR avialable to other architectures

2017-12-08 Thread Prarit Bhargava


On 12/07/2017 01:43 PM, Timur Tabi wrote:
> On Thu, Dec 7, 2017 at 11:29 AM, Prarit Bhargava <pra...@redhat.com> wrote:
>> Other architectures can use SPCR to setup an early console or console but
>> the current code is ARM64 specific.
>>
>> Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
>> function acpi_arch_setup_console() that can be used for arch-specific
>> setup.  Move SPCR initialization flag into ACPI code.  Update the
>> Documention on the use of the SPCR earlycon.
>>
>> This patch does not change arm64 behaviour.
> 
> Let's hope so.  Our E44 erratum work-around has caused lots of
> problems in the past, so the code is a bit fragile.

So it does look like I broke something :(  I'll fix this and do more testing.
One thing that I just learned (relative to x86) is that you have to test much
further and wider than on x86.

P.

> 
>> +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
>> +{
>> +   if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
>> +   return false;
>> +
>> +   if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
>> +   return true;
>> +
>> +   if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
>> +   h->oem_revision == 1)
>> +   return true;
>> +
>> +   return false;
>> +}
> 
> Please give us a chance to test this patch before merging. We've had a
> lot of problems with our E44 work-around, and we need to make sure
> that qdf2400_erratum_44_present function is called before the pl011
> driver is probed.
> 
>> +
>> +/*
>> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
>> + * register aligned to 32-bit. In addition, the BIOS also encoded the
>> + * access width to be 8 bits. This function detects this errata condition.
>> + */
>> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>> +{
>> +   bool xgene_8250 = false;
>> +
>> +   if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
>> +   return false;
>> +
>> +   if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
>> +   memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
>> +   return false;
>> +
>> +   if (!memcmp(tb->header.oem_table_id, "XGENESPC",
>> +   ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
>> +   xgene_8250 = true;
>> +
>> +   if (!memcmp(tb->header.oem_table_id, "ProLiant",
>> +   ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
>> +   xgene_8250 = true;
>> +
>> +   return xgene_8250;
>> +}
> 
> I suspect that this function has the same issues as
> qdf2400_erratum_44_present().
> 
>> +config ACPI_SPCR_TABLE
>> +   bool "ACPI Serial Port Console Redirection Support"
>> +   default y if ARM64
>> +   help
>> + Enable support for Serial Port Console Redirection (SPCR) Table.
>> + This table provides information about the configuration of the
>> + earlycon console.
>> +
> 
> So ACPI without SPCR has never been tested by us.  Making it optional
> makes me a little nervous.  We'll have to evaluate this change.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/2] acpi, x86: Add SPCR table support

2017-12-08 Thread Prarit Bhargava


On 12/08/2017 10:31 AM, Jeffrey Hugo wrote:
> On 12/8/2017 7:29 AM, Prarit Bhargava wrote:
>>
>>
>> On 12/08/2017 01:29 AM, Ingo Molnar wrote:
>>>
>>> * Prarit Bhargava <pra...@redhat.com> wrote:
>>>
>>>> The SPCR (Serial Port Console Redirection) Table provides information
>>>> about the configuration of serial port.  This information can be used
>>>> to configure the early console.
>>>
>>> s/about the configuration of serial port
>>>   /about the configuration of the serial port
>>>
>>>> SPCR support was added for arm64 and is made available across all arches
>>>> in this patchset.  The first patch adds a weak per-arch configuration 
>>>> function
>>>> and moves the SPCR code into ACPI.  The second patch adds support to x86.
>>>>
>>>> The existing behaviour on arm64 is maintained.  If the SPCR exists the
>>>> earlycon and console are automatically configured.
>>>
>>> s/arm64
>>>   /ARM64
>>>
>>> which is easier to read and it's also the prevalent spelling:
>>>
>>>   triton:~/tip> for N in $(git grep -ih arm64 arch/arm64/ | sed
>>> 's/[[:punct:]]/ /g'); do echo $N | grep -iw arm64; done | sort | uniq -c
>>>   412 arm64
>>> 1 Arm64
>>>   854 ARM64
>>>
>>>> The existing default behaviour on x86 is also maintained.  If no console or
>>>> earlycon parameter is defined and the SPCR exists , the serial port is not
>>>> configured.  If the earlycon parameter is used both the early console
>>>> and the console are configured using the data from the SPCR.
>>>
>>> s/exists , the
>>>   /exists, the
>>>
>>> But, the logic to not use the SPCR looks confusing to me.
>>>
>>> The SPCR is only present if the user has explicitly configured a serial 
>>> console
>>> for that machine, either in the firmware, or remotely via IPMI, correct? 
>>> I.e.
>>> SPCR
>>> will not be spuriously present by default on systems that have a serial 
>>> console
>>> but the user never expressed any interest for them, right?
>>
>> If I disable "Serial Port Console Debug" in my BIOS I still see the SPCR
>> configured:
>>
>> [root@prarit-lab ~]# dmesg | grep SPCR
>> [0.00] ACPI: SPCR 0x69031000 50 (v01
>>   )
>>
>> AFAICT the SPCR is always enabled on some systems.
> 
> "Serial Port Console Debug" sounds like DBG2 to me, although I am unsure of 
> the
> specific platform you are referencing.
> 
>>
>>>
>>> If so then we should pick up that serial console configuration and activate 
>>> it,
>>> regardless of any kernel boot options!
>>
>> I'm worried about someone who doesn't want a console on ttyS0 suddenly 
>> ending up
>> with one.  The SPCR could contain incorrect data, etc.
>>
>> I originally wanted this on by default, but the chances of breaking someone's
>> setup seems significant doesn't it?  Maybe I'm being too cautious.  Anyone 
>> else
>> want to weigh in?  I'm not ignoring your idea Ingo, I'm just worried about 
>> being
>> yelled at by a user :) because I broke their default console setup.
>>
> 
> SPCR is always present on QDF2400 (ARM64 platform).  Firmware will 
> automatically
> update the SPCR table to point to the correct console (IE Serial-over-LAN or 
> SOL
> if configured via IPMI).  Unless a user specifically chooses to override the
> SPCR via "console=", we expect that a console will be present based on the 
> data
> in SPCR.

Hey Jeffrey -- I think Ingo's & my conversation is x86-specific.  I am *NOT*
changing the behaviour on ARM64.

P.

> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/2] acpi, x86: Add SPCR table support

2017-12-08 Thread Prarit Bhargava


On 12/08/2017 01:29 AM, Ingo Molnar wrote:
> 
> * Prarit Bhargava <pra...@redhat.com> wrote:
> 
>> The SPCR (Serial Port Console Redirection) Table provides information
>> about the configuration of serial port.  This information can be used
>> to configure the early console.
> 
> s/about the configuration of serial port
>  /about the configuration of the serial port
> 
>> SPCR support was added for arm64 and is made available across all arches
>> in this patchset.  The first patch adds a weak per-arch configuration 
>> function
>> and moves the SPCR code into ACPI.  The second patch adds support to x86.
>>
>> The existing behaviour on arm64 is maintained.  If the SPCR exists the
>> earlycon and console are automatically configured.
> 
> s/arm64
>  /ARM64
> 
> which is easier to read and it's also the prevalent spelling:
> 
>  triton:~/tip> for N in $(git grep -ih arm64 arch/arm64/ | sed 
> 's/[[:punct:]]/ /g'); do echo $N | grep -iw arm64; done | sort | uniq -c
>  412 arm64
>1 Arm64
>  854 ARM64
> 
>> The existing default behaviour on x86 is also maintained.  If no console or
>> earlycon parameter is defined and the SPCR exists , the serial port is not
>> configured.  If the earlycon parameter is used both the early console
>> and the console are configured using the data from the SPCR.
> 
> s/exists , the
>  /exists, the
> 
> But, the logic to not use the SPCR looks confusing to me.
> 
> The SPCR is only present if the user has explicitly configured a serial 
> console 
> for that machine, either in the firmware, or remotely via IPMI, correct? I.e. 
> SPCR 
> will not be spuriously present by default on systems that have a serial 
> console 
> but the user never expressed any interest for them, right?

If I disable "Serial Port Console Debug" in my BIOS I still see the SPCR 
configured:

[root@prarit-lab ~]# dmesg | grep SPCR
[0.00] ACPI: SPCR 0x69031000 50 (v01
  )

AFAICT the SPCR is always enabled on some systems.

> 
> If so then we should pick up that serial console configuration and activate 
> it, 
> regardless of any kernel boot options!

I'm worried about someone who doesn't want a console on ttyS0 suddenly ending up
with one.  The SPCR could contain incorrect data, etc.

I originally wanted this on by default, but the chances of breaking someone's
setup seems significant doesn't it?  Maybe I'm being too cautious.  Anyone else
want to weigh in?  I'm not ignoring your idea Ingo, I'm just worried about being
yelled at by a user :) because I broke their default console setup.

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] acpi, x86: Use SPCR table for earlycon on x86

2017-12-07 Thread Prarit Bhargava


On 12/07/2017 01:46 PM, Timur Tabi wrote:
> On Thu, Dec 7, 2017 at 11:29 AM, Prarit Bhargava <pra...@redhat.com> wrote:
>> -int __init acpi_parse_spcr(bool earlycon)
>> +int __init acpi_parse_spcr(bool earlycon, bool enable_console)
>>  {
>> static char opts[ACPI_SPCR_OPTS_SIZE];
>> static char uart[ACPI_SPCR_BUF_SIZE];
>> @@ -113,7 +113,8 @@ int __init acpi_parse_spcr(bool earlycon)
>> if (earlycon)
>> setup_earlycon(opts);
>>
>> -   err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
>> +   if (enable_console)
>> +   err = add_preferred_console(uart, 0, opts + strlen(uart) + 
>> 1);
> 
> So if earlycon==true but enable_console==false, then you parse SPCR
> and use it for the early console, but not the primary console?  I'm
> not sure what that gives you.
> 

On arm64 the original SPCR code provides both early and primary consoles (as
you've pointed out).  IOW, if SPRC exists on arm64 a console will be
initialized.  That is still the case after my change.

The expected behaviour on x86 is that unless someone specifies 'earlycon' or
'console=' that no console should be initialized.  There are users (some IOT
boxes, etc.) that do NOT want a console.

The above code keeps the existing x86 behaviour.

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/2] acpi, x86: Add SPCR table support

2017-12-07 Thread Prarit Bhargava
The SPCR (Serial Port Console Redirection) Table provides information
about the configuration of serial port.  This information can be used
to configure the early console.

SPCR support was added for arm64 and is made available across all arches
in this patchset.  The first patch adds a weak per-arch configuration function
and moves the SPCR code into ACPI.  The second patch adds support to x86.

The existing behaviour on arm64 is maintained.  If the SPCR exists the
earlycon and console are automatically configured.

The existing default behaviour on x86 is also maintained.  If no console or
earlycon parameter is defined and the SPCR exists , the serial port is not
configured.  If the earlycon parameter is used both the early console
and the console are configured using the data from the SPCR.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux...@vger.kernel.org
Cc: linux-a...@vger.kernel.org
Cc: linux-ser...@vger.kernel.org
Cc: Bhupesh Sharma <bhsha...@redhat.com>
Cc: Lv Zheng <lv.zh...@intel.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: x...@kernel.org
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Catalin Marinas <catalin.mari...@arm.com>
Cc: Will Deacon <will.dea...@arm.com>
Cc: "Rafael J. Wysocki" <r...@rjwysocki.net>

Prarit Bhargava (2):
  acpi, spcr: Make SPCR avialable to other architectures
  acpi, x86: Use SPCR table for earlycon on x86

 Documentation/admin-guide/kernel-parameters.txt |   9 +-
 arch/arm64/Kconfig  |   1 -
 arch/arm64/kernel/acpi.c| 128 +-
 arch/x86/kernel/acpi/boot.c |   3 +
 drivers/acpi/Kconfig|  11 +-
 drivers/acpi/spcr.c | 171 ++--
 drivers/tty/serial/earlycon.c   |  16 +--
 include/linux/acpi.h|  14 +-
 include/linux/serial_core.h |   2 -
 9 files changed, 195 insertions(+), 160 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3 v12] printk: Add monotonic, boottime, and realtime timestamps

2017-09-26 Thread Prarit Bhargava


On 09/26/2017 07:48 AM, Petr Mladek wrote:
> On Mon 2017-09-18 13:51:00, Prarit Bhargava wrote:
>> printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
>> timestamp to printk messages.  The local hardware clock loses time each
>> day making it difficult to determine exactly when an issue has occurred in
>> the kernel log, and making it difficult to determine how kernel and
>> hardware issues relate to each other in real time.
>>
>> Make printk output different timestamps by adding options for no
>> timestamp, the local hardware clock, the monotonic clock, the boottime
>> clock, and the real clock.  Allow a user to pick one of the clocks by
>> using the printk.time kernel parameter.  Output the type of clock in
>> /sys/module/printk/parameters/time so userspace programs can interpret the
>> timestamp.
>>
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index 512f7c2baedd..5e0bf2ef02f7 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -1201,14 +1204,130 @@ static inline void boot_delay_msec(int level)
>> +static int printk_time = CONFIG_PRINTK_TIME_TYPE;
>> +
>> +static int printk_set_ts_source(enum timestamp_sources ts_source)
>> +{
>> +int err = 0;
> 
> 
>> @@ -1861,6 +1980,7 @@ static size_t msg_print_text(const struct printk_log 
>> *msg,
>>   bool syslog, char *buf, size_t size) { return 0; }
>>  static bool suppress_message_printing(int level) { return false; }
>>  
>> +static int printk_time;
> 
> I worried if the variable should have got initialized. But it seems to
> be a relic from an older version. The variable is not longer used and
> needed when CONFIG_PRINTK is not defined. It is proved by gcc:
> 
>   CC  kernel/printk/printk.o
> kernel/printk/printk.c:1983:12: warning: ‘printk_time’ defined but not used 
> [-Wunused-variable]
>  static int printk_time;
> 

I didn't catch that :(.  tglx, want a v14?

P.

> 
>>  #endif /* CONFIG_PRINTK */
>>  
>>  #ifdef CONFIG_EARLY_PRINTK
> 
> Otherwise that patch looks fine. With the unused variable removed,
> feel free to use:
> 
> Reviewed-by: Petr Mladek <pmla...@suse.com>
> 
> Best Regards,
> Petr
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3 v11] printk: Add monotonic, boottime, and realtime timestamps

2017-09-19 Thread Prarit Bhargava


On 09/17/2017 06:46 AM, Sergey Senozhatsky wrote:

> I'm a bit uncomfortable with the "breaks user space" part. since this
> is a strictly debugging option, would it be sufficient to store those
> extended timestamps as prefixes of every message?
> see (sorry for "self-quoting"):
>   lkml.kernel.org/r/20170917062608.GA512@tigerII.localdomain
>

Sergey, I haven't forgotten about the above.  It's something
I'm going to look at after this initial patchset is done.

P.

> each message, thus, will be in the following format
> 
> [current header: loglevel, timestamp, etc] [extended printk data] message text
> 
> extended printk data can contain your monotonic/etc timestamps, and
> anything else.
> 
> and then it's up to you how do you grep the messages and process the
> extended data. but the point is - user space tools (journald, dmesg,
> etc.) stays intact. which is kinda nice.
> 
> so we can avoid that chicken and egg problem: we break user space
> by merging the patchset but user space people don't want to talk
> about any fixes until we break those tools.
> 
>   -ss
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/3 v12] printk: Add new timestamps

2017-09-18 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Add monotonic, boottime, and real clock timestamps in addition to the existing
local hardware clock timestamp.

Changes from v12: In 1/3 remove typecast.  In 3/3, Reword Kconfig names,
simplify timestamp logic and remove recursive code.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
[jstultz: reworked Kconfig settings to avoid defconfig noise]
Signed-off-by: John Stultz <john.stu...@linaro.org>

Prarit Bhargava (2):
  time: Make fast functions return 0 before timekeeping is initialized
  printk: Add monotonic, boottime, and realtime timestamps

Thomas Gleixner (1):
  timekeeping: Provide NMI safe access to clock realtime

 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 include/linux/timekeeper_internal.h |   6 +-
 include/linux/timekeeping.h |   1 +
 kernel/printk/printk.c  | 130 +++-
 kernel/time/timekeeping.c   |  60 +--
 lib/Kconfig.debug   |  48 -
 6 files changed, 233 insertions(+), 18 deletions(-)

-- 
1.8.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3 v11] printk: Add monotonic, boottime, and realtime timestamps

2017-09-15 Thread Prarit Bhargava


On 09/15/2017 09:28 AM, Petr Mladek wrote:
> 
> I am still slightly nervous that external tools would need updating.
> Also they might have troubles to interpret the time stamps especially
> when the source is changed at runtime via
> /sys/module/printk/parameters/time.

In earlier versions I had logic to prevent switching during runtime.  tglx
requested that it be removed and I removed it.  Personally I fall into the "I'm
going to set it once and never change it" however, as Mark will offer there are
cases where it might be advantageous to change the timestamp.  I think he has a
patch that will append a b/r/etc. to the timestamp so that userspace can
interpret the different timestamps.

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/3 v11] printk: Add new timestamps

2017-09-05 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Add monotonic, boottime, and real clock timestamps in addition to the existing
local hardware clock timestamp.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
[jstultz: reworked Kconfig settings to avoid defconfig noise]
Signed-off-by: John Stultz <john.stu...@linaro.org>

Prarit Bhargava (2):
  time: Make fast functions return 0 before timekeeping is initialized
  printk: Add monotonic, boottime, and realtime timestamps

Thomas Gleixner (1):
  timekeeping: Provide NMI safe access to clock realtime

 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 include/linux/timekeeper_internal.h |   6 +-
 include/linux/timekeeping.h |   1 +
 kernel/printk/printk.c  | 113 ++--
 kernel/time/timekeeping.c   |  72 +--
 lib/Kconfig.debug   |  48 +-
 6 files changed, 228 insertions(+), 18 deletions(-)

-- 
1.8.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3 v11] printk: Add monotonic, boottime, and realtime timestamps

2017-09-05 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Make printk output different timestamps by adding options for no
timestamp, the local hardware clock, the monotonic clock, the boottime
clock, and the real clock.  Allow a user to pick one of the clocks by
using the printk.time kernel parameter.  Output the type of clock in
/sys/module/printk/parameters/time so userspace programs can interpret the
timestamp.

v2: Use peterz's suggested Kconfig options.  Merge patchset together.
Fix i386 !CONFIG_PRINTK builds.

v3: Fixed x86_64_defconfig. Added printk_time_type enum and
printk_time_str for better output. Added BOOTTIME clock functionality.

v4: Fix messages, add additional printk.time options, and fix configs.

v5: Renaming of structures, and allow printk_time_set() to
evaluate substrings of entries (eg: allow 'r', 'real', 'realtime').  From
peterz, make fast functions return 0 until timekeeping is initialized
(removes timekeeping_active & ktime_get_boot|real_log_ts() suggested by
 tglx and adds ktime_get_real_offset()).  Switch to a function pointer
for printk_get_ts() and reference fast functions.  Make timestamp_sources enum
match choice options for CONFIG_PRINTK_TIME (adds PRINTK_TIME_UNDEFINED).

v6: Define PRINTK_TIME_UNDEFINED for !CONFIG_PRINTK builds.  Separate
timekeeping changes into separate patch.  Minor include file cleanup.

v7: Add default case to printk_set_timestamp() and add PRINTK_TIME_DEBUG
for users that want to set timestamp to different values during runtime.
Add jstultz' Kconfig to avoid defconfig churn.

v8: Add CONFIG_PRINTK_TIME_DEBUG to allow timestamp runtime switching.
Rename PRINTK_TIME_DISABLE to PRINTK_TIME_DISABLED.  Rename
printk_set_timestamp() to printk_set_ts_func().  Separate
printk_set_ts_func() and printk_get_first_ts() portions.  Rename param
functions.  Adjust configs, enum, and timestamp_sources_str to be 0-4.
Add mention realtime clock is UTC in Documentation.

v9: Fix typo.  Add __ktime_get_real_fast_ns_unsafe().

v10: Remove time parameter restrictions.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
[jstultz: reworked Kconfig settings to avoid defconfig noise]
Signed-off-by: John Stultz <john.stu...@linaro.org>
---
 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 kernel/printk/printk.c  | 113 ++--
 lib/Kconfig.debug   |  48 +-
 3 files changed, 159 insertions(+), 8 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index d9c171ce4190..8d6b194533af 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3188,8 +3188,10 @@
ratelimit - ratelimit the logging
Default: ratelimit
 
-   printk.time=Show timing data prefixed to each printk message line
-   Format:   (1/Y/y=enable, 0/N/n=disable)
+   printk.time=Show timestamp prefixed to each printk message line
+   Format: 
+   (0/N/n/disable, 1/Y/y/local,
+b/boot, m/monotonic, r/realtime (in UTC))
 
processor.max_cstate=   [HW,ACPI]
Limit processor to maximum C-state
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index fc47863f629c..5aaeb1ebd26c 100644
--- a/kernel/printk/printk.c
+++ 

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

2017-08-28 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Make printk output different timestamps by adding options for no
timestamp, the local hardware clock, the monotonic clock, the boottime
clock, and the real clock.  Allow a user to pick one of the clocks by
using the printk.time kernel parameter.  Output the type of clock in
/sys/module/printk/parameters/time so userspace programs can interpret the
timestamp.

Real clock & 32-bit systems:  Selecting the real clock printk timestamp may
lead to unlikely situations where a timestamp is wrong because the real time
is read without the protection of a sequence lock when printk_get_ts() is set
to __ktime_get_real_fast_ns_unsafe().

v2: Use peterz's suggested Kconfig options.  Merge patchset together.
Fix i386 !CONFIG_PRINTK builds.

v3: Fixed x86_64_defconfig. Added printk_time_type enum and
printk_time_str for better output. Added BOOTTIME clock functionality.

v4: Fix messages, add additional printk.time options, and fix configs.

v5: Renaming of structures, and allow printk_time_set() to
evaluate substrings of entries (eg: allow 'r', 'real', 'realtime').  From
peterz, make fast functions return 0 until timekeeping is initialized
(removes timekeeping_active & ktime_get_boot|real_log_ts() suggested by
 tglx and adds ktime_get_real_offset()).  Switch to a function pointer
for printk_get_ts() and reference fast functions.  Make timestamp_sources enum
match choice options for CONFIG_PRINTK_TIME (adds PRINTK_TIME_UNDEFINED).

v6: Define PRINTK_TIME_UNDEFINED for !CONFIG_PRINTK builds.  Separate
timekeeping changes into separate patch.  Minor include file cleanup.

v7: Add default case to printk_set_timestamp() and add PRINTK_TIME_DEBUG
for users that want to set timestamp to different values during runtime.
Add jstultz' Kconfig to avoid defconfig churn.

v8: Add CONFIG_PRINTK_TIME_DEBUG to allow timestamp runtime switching.
Rename PRINTK_TIME_DISABLE to PRINTK_TIME_DISABLED.  Rename
printk_set_timestamp() to printk_set_ts_func().  Separate
printk_set_ts_func() and printk_get_first_ts() portions.  Rename param
functions.  Adjust configs, enum, and timestamp_sources_str to be 0-4.
Add mention realtime clock is UTC in Documentation.

v9: Fix typo.  Add __ktime_get_real_fast_ns_unsafe().

v10: Remove time parameter restrictions.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
[jstultz: reworked Kconfig settings to avoid defconfig noise]
Signed-off-by: John Stultz <john.stu...@linaro.org>
---
 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 include/linux/timekeeping.h |   1 +
 kernel/printk/printk.c  | 116 +++-
 kernel/time/timekeeping.c   |  13 +++
 lib/Kconfig.debug   |  48 +-
 5 files changed, 176 insertions(+), 8 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index d9c171ce4190..8d6b194533af 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3188,8 +3188,10 @@
ratelimit - ratelimit the logging
Default: ratelimit
 
-   printk.time=Show timing data prefixed to each printk message line
-   Format:   (1/Y/y=enable, 0/N/n=disable)
+   printk.time=Show timestamp prefixed to each printk message line
+

[PATCH 0/2 v10] printk: Add new timestamps

2017-08-28 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Add monotonic, boottime, and real clock timestamps in addition to the existing
local hardware clock timestamp.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
[jstultz: reworked Kconfig settings to avoid defconfig noise]
Signed-off-by: John Stultz <john.stu...@linaro.org>

Prarit Bhargava (2):
  time: Make fast functions return 0 before timekeeping is initialized
  printk: Add monotonic, boottime, and realtime timestamps

 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 include/linux/timekeeping.h |   1 +
 kernel/printk/printk.c  | 116 +++-
 kernel/time/timekeeping.c   |  54 ---
 lib/Kconfig.debug   |  48 +-
 5 files changed, 206 insertions(+), 19 deletions(-)

-- 
1.8.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2017-08-25 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Make printk output different timestamps by adding options for no
timestamp, the local hardware clock, the monotonic clock, the boottime
clock, and the real clock.  Allow a user to pick one of the clocks by
using the printk.time kernel parameter.  Output the type of clock in
/sys/module/printk/parameters/time so userspace programs can interpret the
timestamp.

Real clock & 32-bit systems:  Selecting the real clock printk timestamp may
lead to unlikely situations where a timestamp is wrong because the real time
is read without the protection of a sequence lock when printk_get_ts() is set
to __ktime_get_real_fast_ns_unsafe().

v2: Use peterz's suggested Kconfig options.  Merge patchset together.
Fix i386 !CONFIG_PRINTK builds.

v3: Fixed x86_64_defconfig. Added printk_time_type enum and
printk_time_str for better output. Added BOOTTIME clock functionality.

v4: Fix messages, add additional printk.time options, and fix configs.

v5: Renaming of structures, and allow printk_time_set() to
evaluate substrings of entries (eg: allow 'r', 'real', 'realtime').  From
peterz, make fast functions return 0 until timekeeping is initialized
(removes timekeeping_active & ktime_get_boot|real_log_ts() suggested by
 tglx and adds ktime_get_real_offset()).  Switch to a function pointer
for printk_get_ts() and reference fast functions.  Make timestamp_sources enum
match choice options for CONFIG_PRINTK_TIME (adds PRINTK_TIME_UNDEFINED).

v6: Define PRINTK_TIME_UNDEFINED for !CONFIG_PRINTK builds.  Separate
timekeeping changes into separate patch.  Minor include file cleanup.

v7: Add default case to printk_set_timestamp() and add PRINTK_TIME_DEBUG
for users that want to set timestamp to different values during runtime.
Add jstultz' Kconfig to avoid defconfig churn.

v8: Add CONFIG_PRINTK_TIME_DEBUG to allow timestamp runtime switching.
Rename PRINTK_TIME_DISABLE to PRINTK_TIME_DISABLED.  Rename
printk_set_timestamp() to printk_set_ts_func().  Separate
printk_set_ts_func() and printk_get_first_ts() portions.  Rename param
functions.  Adjust configs, enum, and timestamp_sources_str to be 0-4.
Add mention realtime clock is UTC in Documentation.

v9: Fix typo.  Add __ktime_get_real_fast_ns_unsafe().

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
[jstultz: reworked Kconfig settings to avoid defconfig noise]
Signed-off-by: John Stultz <john.stu...@linaro.org>
---
 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 include/linux/timekeeping.h |   1 +
 kernel/printk/printk.c  | 142 +++-
 kernel/time/timekeeping.c   |  13 +++
 lib/Kconfig.debug   |  65 ++-
 5 files changed, 219 insertions(+), 8 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index d9c171ce4190..8d6b194533af 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3188,8 +3188,10 @@
ratelimit - ratelimit the logging
Default: ratelimit
 
-   printk.time=Show timing data prefixed to each printk message line
-   Format:   (1/Y/y=enable, 0/N/n=disable)
+   printk.time=Show timestamp prefixed to each printk message line
+   Format: 
+

[PATCH 0/2 v9] printk: Add new timestamps

2017-08-25 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Add monotonic, boottime, and real clock timestamps in addition to the existing
local hardware clock timestamp.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
[jstultz: reworked Kconfig settings to avoid defconfig noise]
Signed-off-by: John Stultz <john.stu...@linaro.org>

Prarit Bhargava (2):
  time: Make fast functions return 0 before timekeeping is initialized
  printk: Add monotonic, boottime, and realtime timestamps

 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 include/linux/timekeeping.h |   1 +
 kernel/printk/printk.c  | 142 +++-
 kernel/time/timekeeping.c   |  54 +++--
 lib/Kconfig.debug   |  65 ++-
 5 files changed, 249 insertions(+), 19 deletions(-)

-- 
1.8.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2017-08-25 Thread Prarit Bhargava


On 08/24/2017 02:50 PM, John Stultz wrote:
> On Thu, Aug 24, 2017 at 6:42 AM, Prarit Bhargava <pra...@redhat.com> wrote:
>> --- a/include/linux/timekeeping.h
>> +++ b/include/linux/timekeeping.h
>> @@ -239,6 +239,7 @@ static inline u64 ktime_get_raw_ns(void)
>>  extern u64 ktime_get_mono_fast_ns(void);
>>  extern u64 ktime_get_raw_fast_ns(void);
>>  extern u64 ktime_get_boot_fast_ns(void);
>> +extern u64 ktime_get_real_offset(void);
>>
>>  /*
>>   * Timespec interfaces utilizing the ktime based ones
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index fc47863f629c..dd972bc5c88b 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
> ...
>> + * printk_get_real_ns: - Return a realtime timestamp for printk messages
>> + * On 32-bit systems selecting the real clock printk timestamp may lead to
>> + * unlikely situations where a timestamp is wrong because the real time 
>> offset
>> + * is read without the protection of a sequence lock.
>> + */
>> +static u64 printk_get_real_ns(void)
>> +{
>> +   return ktime_get_mono_fast_ns() + ktime_get_real_offset();
>> +}
>> +
> ...
>> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
>> index d111039e0245..de07bb5ffef5 100644
>> --- a/kernel/time/timekeeping.c
>> +++ b/kernel/time/timekeeping.c
>> @@ -508,6 +508,11 @@ u64 notrace ktime_get_boot_fast_ns(void)
>>  }
>>  EXPORT_SYMBOL_GPL(ktime_get_boot_fast_ns);
>>
>> +u64 ktime_get_real_offset(void)
>> +{
>> +   return ktime_to_ns(tk_core.timekeeper.offs_real);
>> +}
>> +
> 
> Apologies! One last nit here.  So if we're going to export
> ktime_get_real_offset(), folks are going to use it, and there is no
> documentation about its very sketchy behavioral limits as an
> interface, except elsewhere in the printk code.
> 
> Instead of doing that, could you export a
> __ktime_get_real_fast_ns_unsafe() function, which has its limits
> (calculating the realtime w/o locks, thus may return completely bad
> values occasionally) clearly documented in the timekeeping code?

np.  I'm going to copy the code for

u64 notrace ktime_get_boot_fast_ns(void)

but I'm unsure why the function is marked "notrace", and if
__ktime_get_real_fast_ns_unsafe() must be as well?  I don't see anything in the
git log that indicates why the function is notrace.

I've added Joel to this thread ...

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/2 v8] printk: Add new timestamps

2017-08-24 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Add monotonic, boottime, and real clock timestamps in addition to the existing
local hardware clock timestamp.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
[jstultz: reworked Kconfig settings to avoid defconfig noise]
Signed-off-by: John Stultz <john.stu...@linaro.org>

Prarit Bhargava (2):
  time: Make fast functions return 0 before timekeeping is initialized
  printk: Add monotonic, boottime, and realtime timestamps

 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 include/linux/timekeeping.h |   1 +
 kernel/printk/printk.c  | 154 +++-
 kernel/time/timekeeping.c   |  46 +--
 lib/Kconfig.debug   |  65 +-
 5 files changed, 253 insertions(+), 19 deletions(-)

-- 
1.8.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2017-08-23 Thread Prarit Bhargava

On 08/23/2017 04:45 AM, Petr Mladek wrote:
> On Thu 2017-08-17 09:15:39, Prarit Bhargava wrote:
>> printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
>> timestamp to printk messages.  The local hardware clock loses time each
>> day making it difficult to determine exactly when an issue has occurred in
>> the kernel log, and making it difficult to determine how kernel and
>> hardware issues relate to each other in real time.
>>
>> Make printk output different timestamps by adding options for no
>> timestamp, the local hardware clock, the monotonic clock, the boottime
>> clock, and the real clock.  Allow a user to pick one of the clocks by
>> using the printk.time kernel parameter.  Output the type of clock in
>> /sys/module/printk/parameters/time so userspace programs can interpret the
>> timestamp.
>>
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index fc47863f629c..3c46217629fd 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -1202,14 +1205,144 @@ static inline void boot_delay_msec(int level)
>>  }
>>  #endif
>>  
>> -static bool printk_time = IS_ENABLED(CONFIG_PRINTK_TIME);
>> -module_param_named(time, printk_time, bool, S_IRUGO | S_IWUSR);
>> +static int printk_time = CONFIG_PRINTK_TIME_TYPE;
>> +static int printk_time_source;
>> +
>> +/**
>> + * enum timestamp_sources - Timestamp sources for printk() messages.
>> + * @PRINTK_TIME_UNDEFINED: Timestamp undefined.  This option is not 
>> selectable
>> + * from the configs, and is used as a reference in the code.
>> + * @PRINTK_TIME_DISABLE: No time stamp.
>> + * @PRINTK_TIME_LOCAL: Local hardware clock timestamp.
>> + * @PRINTK_TIME_BOOT: Boottime clock timestamp.
>> + * @PRINTK_TIME_MONO: Monotonic clock timestamp.
>> + * @PRINTK_TIME_REAL: Realtime clock timestamp.  On 32-bit
>> + * systems selecting the real clock printk timestamp may lead to unlikely
>> + * situations where a timestamp is wrong because the real time offset is 
>> read
>> + * without the protection of a sequence lock when printk_get_ts() is set to
>> + * printk_get_real_ns().
>> + */
>> +enum timestamp_sources {
> 
> I guess that this might be static. Also it is related to the
> PRINTK_TIME_TYPE Kconfig option. So the name should be
> enum printk_time_type or so.
> 
> 
>> +PRINTK_TIME_UNDEFINED = 0,
> 
> I would use -1 for PRINTK_TIME_UNDEFINED or I would remove it
> completely, see below. Then we could use 0 for PRINTK_TIME_DISABLED
> which is more expected.

FIXED along with adjustment of configs (see below).

> 
> 
>> +PRINTK_TIME_DISABLE = 1,
> 
> This should be PRINTK_TIME_DISABLED to be in sync with
> the string and style of the other flags.
>

FIXED.

> 
>> +PRINTK_TIME_LOCAL = 2,
>> +PRINTK_TIME_BOOT = 3,
>> +PRINTK_TIME_MONO = 4,
>> +PRINTK_TIME_REAL = 5,
>> +};
>> +
>> +static const char * const timestamp_sources_str[6] = {
>> +"undefined",
> 
> Do we really need the string "undefined"? 

It is only included to keep timestamp_sources_str & the enum in sync.
FIXED & removed with adjustment of configs & enum.



>> +/**
>> + * printk_get_real_ns: - Return a realtime timestamp for printk messages
>> + * On 32-bit systems selecting the real clock printk timestamp may lead to
>> + * unlikely situations where a timestamp is wrong because the real time 
>> offset
>> + * is read without the protection of a sequence lock.
>> + */
>> +static u64 printk_get_real_ns(void)
>> +{
>> +return ktime_get_mono_fast_ns() + ktime_get_real_offset();
>> +}
>> +
>> +static u64 printk_set_timestamp(void)
> 
> I would rename this function to printk_set_ts_func() or so.
> It will be more clear the it sets the function pointer.
> 

FIXED.

> 
>> +{
>> +switch (printk_time) {
>> +case PRINTK_TIME_LOCAL:
>> +case PRINTK_TIME_DISABLE:
>> +default:
>> +printk_get_ts = local_clock;
> 
> The PRINTK_TIME_DISABLE case is more complicated. We should set
> printk_get_ts only when it has not been set yet. Otherwise,
> we should keep storing the same type of time stamps into
> to the log buffer and keep it consistent.
> 

I think you've figured this out below but for other readers,
printk_time_set() does not call printk_set_timestamp with
printk_time = PRINTK_TIME_DISABLE.

> 
>> +break;
>> +case PRINTK_TIME_BOOT:
>> +printk_get_ts = ktime_get_boot_fast_ns;
>> +  

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

2017-08-22 Thread Prarit Bhargava


On 08/22/2017 10:23 AM, Petr Mladek wrote:
> On Tue 2017-08-22 10:09:40, Prarit Bhargava wrote:
>>
>>
>> On 08/17/2017 11:30 AM, Mark Salyzyn wrote:
>>> On 08/17/2017 06:15 AM, Prarit Bhargava wrote:
>>>> printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
>>>> timestamp to printk messages.  The local hardware clock loses time each
>>>> day making it difficult to determine exactly when an issue has occurred in
>>>> the kernel log, and making it difficult to determine how kernel and
>>>> hardware issues relate to each other in real time.
>>> Congrats, greatly eases merges into older kernels, and has eliminated the 
>>> churn
>>> this could place on all the configured systems out there.
>>>
>>> Sadly, one of my suggestions did not quite go the way I expected ;-} easy to
>>> correct, and fix (I missed a spot in my original suggestion, as code changed
>>> underfoot over the set ;-/). (see bottom)
>>
>> Added.  I will send out v8 shortly.
> 
> It might make sense to wait a bit. I am in the middle of v7 review and
> have several comments.
> 
> I would suggest to slow down this a bit anyway. You sent 7 versions
> within three weeks. It improved nicely over time. But I think that
> I am not the only one who has troubles to follow.
> 

np Petr.  I will hold off on v8.

P.

> Best Regards,
> Petr
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2017-08-22 Thread Prarit Bhargava


On 08/17/2017 11:30 AM, Mark Salyzyn wrote:
> On 08/17/2017 06:15 AM, Prarit Bhargava wrote:
>> printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
>> timestamp to printk messages.  The local hardware clock loses time each
>> day making it difficult to determine exactly when an issue has occurred in
>> the kernel log, and making it difficult to determine how kernel and
>> hardware issues relate to each other in real time.
> Congrats, greatly eases merges into older kernels, and has eliminated the 
> churn
> this could place on all the configured systems out there.
> 
> Sadly, one of my suggestions did not quite go the way I expected ;-} easy to
> correct, and fix (I missed a spot in my original suggestion, as code changed
> underfoot over the set ;-/). (see bottom)

Added.  I will send out v8 shortly.

> 
> 
> 
> I am not convinced that user space is entirely at a disadvantage with this
> 'feature' enabled. Before interpreting it can read
> /sys/module/printk/parameters/time, then sniff for the flowing content for 
> time
> breaks (watch for printk: timestamp set to ). Of course, the value in
> 'time' is current, so it would be _wrong_ during flow of previous content 
> until
> the first time break shows up if it really was being switched that often.
> 
> (echo local ; echo disabled ; echo boottime ; echo monotonic ; echo realtime ;
> echo local ) > /sys/module/printk/parameters/time
> 
> [  473.589169] printk: timestamp set to local
> printk: timestamp set to disabled
> [  473.545384] printk: timestamp set to boottime
> [  473.549924] printk: timestamp set to monotonic
> [1502957708.055265] printk: timestamp set to realtime
> [  473.612024] printk: timestamp set to local
> 
> A 'fix' would be to add a letter after the timestamp if not local. For 
> example:
> 
> [  473.589169] printk: timestamp set to local
> printk: timestamp set to disabled
> [  473.545384b] printk: timestamp set to boottime
> [  473.549924m] printk: timestamp set to monotonic
> [1502957708.055265U] printk: timestamp set to realtime
> [  473.612024] printk: timestamp set to local
> 
> (I used U instead of r, because it is actually UTC, and did not add 'l' 
> because
> it is a long standing default)
> 
> But there would be concern over a change in time format API, so maybe it 
> should
> be relegated to a CONFIG_PRINTK_TIME_DEBUG 'feature' only to add the timebase
> letters?

... then you wouldn't be able to use dmesg, etc., to get proper timestamp
conversions. :(  I'm not going to code dmesg for that because, as you point out
it is outside of the scope of this patch.  Maybe we can do something in future
for this?

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2017-08-17 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Make printk output different timestamps by adding options for no
timestamp, the local hardware clock, the monotonic clock, the boottime
clock, and the real clock.  Allow a user to pick one of the clocks by
using the printk.time kernel parameter.  Output the type of clock in
/sys/module/printk/parameters/time so userspace programs can interpret the
timestamp.

Real clock & 32-bit systems:  Selecting the real clock printk timestamp may
lead to unlikely situations where a timestamp is wrong because the real time
without the protection of a sequence lock when printk_get_ts() is set
printk_get_real_ns().

v2: Use peterz's suggested Kconfig options.  Merge patchset together.
Fix i386 !CONFIG_PRINTK builds.

v3: Fixed x86_64_defconfig. Added printk_time_type enum and
printk_time_str for better output. Added BOOTTIME clock functionality.

v4: Fix messages, add additional printk.time options, and fix configs.

v5: Renaming of structures, and allow printk_time_set() to
evaluate substrings of entries (eg: allow 'r', 'real', 'realtime').  From
peterz, make fast functions return 0 until timekeeping is initialized
(removes timekeeping_active & ktime_get_boot|real_log_ts() suggested by
 tglx and adds ktime_get_real_offset()).  Switch to a function pointer
for printk_get_ts() and reference fast functions.  Make timestamp_sources enum
match choice options for CONFIG_PRINTK_TIME (adds PRINTK_TIME_UNDEFINED).

v6: Define PRINTK_TIME_UNDEFINED for !CONFIG_PRINTK builds.  Separate
timekeeping changes into separate patch.  Minor include file cleanup.

v7: Add default case to printk_set_timestamp() and add PRINTK_TIME_DEBUG
for users that want to set timestamp to different values during runtime.
Add jstultz' Kconfig to avoid defconfig churn.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
[jstultz: reworked Kconfig settings to avoid defconfig noise]
Signed-off-by: John Stultz <john.stu...@linaro.org>
---
 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 include/linux/timekeeping.h |   1 +
 kernel/printk/printk.c  | 150 +++-
 kernel/time/timekeeping.c   |   5 +
 lib/Kconfig.debug   |  48 +++-
 5 files changed, 202 insertions(+), 8 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index d9c171ce4190..1ba277ecbf24 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3188,8 +3188,10 @@
ratelimit - ratelimit the logging
Default: ratelimit
 
-   printk.time=Show timing data prefixed to each printk message line
-   Format:   (1/Y/y=enable, 0/N/n=disable)
+   printk.time=Show timestamp prefixed to each printk message line
+   Format: 
+   (0/N/n/disable, 1/Y/y/local,
+b/boot, m/monotonic, r/realtime)
 
processor.max_cstate=   [HW,ACPI]
Limit processor to maximum C-state
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index ddc229ff6d1e..80ca2fa22b6a 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -239,6 +239,7 @@ static inline u64 ktime_get_raw_ns(void)
 extern u64 ktime_get_mono_

[PATCH 0/2 v7] printk: Add new timestamps

2017-08-17 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Add monotonic, boottime, and real clock timestamps in addition to the existing
local hardware clock timestamp.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
[jstultz: reworked Kconfig settings to avoid defconfig noise]
Signed-off-by: John Stultz <john.stu...@linaro.org>

Prarit Bhargava (2):
  time: Make fast functions return 0 before timekeeping is initialized
  printk: Add monotonic, boottime, and realtime timestamps

 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 include/linux/timekeeping.h |   1 +
 kernel/printk/printk.c  | 150 +++-
 kernel/time/timekeeping.c   |  46 ++--
 lib/Kconfig.debug   |  48 +++-
 5 files changed, 232 insertions(+), 19 deletions(-)

-- 
1.8.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2017-08-17 Thread Prarit Bhargava


On 08/17/2017 12:46 AM, John Stultz wrote:
> From: Prarit Bhargava <pra...@redhat.com>
> 
> 
> Prarit,
>   So I took my own shot at cleaning up the Kconfig settings to make
> it a bit more sane in my opinion. This should allow the legacy
> defconfigs to just work, and avoids all the defconfig churn in the
> patch.
> 
> I almost went through to try to simplify the code and make
> PRINTK_TIME_DISABLE = 0 and to get rid of PRINTK_TIME_UNDEFINED,
> as that seems like extra complexity that could probably be done
> simpler, but I didn't quite have the time to get my head around
> it all, so I left it be. But you might want to see if it can't
> be further simplified (ie: initialize it to CONFIG_PRINTK_TIME_VAL
> and then override it by the boot parameter if specified).
> 
> Anwyay, let me know what you think, and feel free to adopt this
> along with any other changes you have planned.
> 
> thanks
> -john



> index 98fe715..83683e9ad 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -8,12 +8,64 @@ config PRINTK_TIME
> messages to be added to the output of the syslog() system
> call and at the console.
>  
> +choice
> + prompt "printk default clock timestamp"
> + default PRINTK_TIME_LOCAL if PRINTK_TIME
> + default PRINTK_TIME_DISABLE if !PRINTK_TIME
> + help
> +   This option is selected by setting one of
> +   PRINTK_TIME_[DISABLE|LOCAL|BOOT|MONO|REAL] and causes time stamps of
> +   the printk() messages to be added to the output of the syslog()
> +   system call and at the console.
> +
> The timestamp is always recorded internally, and exported
> to /dev/kmsg. This flag just specifies if the timestamp should
> be included, not that the timestamp is recorded.
>  
> The behavior is also controlled by the kernel command line
> -   parameter printk.time=1. See 
> Documentation/admin-guide/kernel-parameters.rst
> +   parameter printk.time. See
> +   Documentation/admin-guide/kernel-parameters.rst
> +
> +config PRINTK_TIME_DISABLE
> + bool "Disabled" if !PRINTK_TIME
> + help
> +  Selecting this option disables the time stamps of printk().
> +
> +config PRINTK_TIME_LOCAL
> + bool "Local Clock" if PRINTK_TIME
> + help
> +   Selecting this option causes the time stamps of printk() to be
> +   stamped with the unadjusted hardware clock.
> +
> +config PRINTK_TIME_BOOT
> + bool "CLOCK_BOOTTIME" if PRINTK_TIME
> + help
> +   Selecting this option causes the time stamps of printk() to be
> +   stamped with the adjusted boottime clock.
> +
> +config PRINTK_TIME_MONO
> + bool "CLOCK_MONOTONIC" if PRINTK_TIME
> + help
> +   Selecting this option causes the time stamps of printk() to be
> +   stamped with the adjusted monotonic clock.
> +
> +config PRINTK_TIME_REAL
> + bool "CLOCK_REALTIME" if PRINTK_TIME
> + help
> +   Selecting this option causes the time stamps of printk() to be
> +   stamped with the adjusted realtime clock.
> +endchoice
> +
> +config PRINTK_TIME_TYPE
> + int
> + depends on PRINTK
> + range 1 5
> + default 1 if PRINTK_TIME_DISABLE
> + default 2 if PRINTK_TIME_LOCAL
> + default 3 if PRINTK_TIME_BOOT
> + default 4 if PRINTK_TIME_MONO
> + default 5 if PRINTK_TIME_REAL
> +

So, IIUC the result of this is, for example,

PRINT_TIME=y
PRINT_TIME_BOOT=y
PRINT_TIME_TIME_TYPE=3

In an earlier version of my patchset someone said not to introduce new config
options like this.  The reason I did introduce (IIRC I called it
CONFIG_PRINTK_CLOCK) was to avoid all the config churn but that was shot down.

If this is back on the table, then I too have a version that does essentially
what your patch does.

P.

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2017-08-16 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Make printk output different timestamps by adding options for no
timestamp, the local hardware clock, the monotonic clock, the boottime
clock, and the real clock.  Allow a user to pick one of the clocks by
using the printk.time kernel parameter.  Output the type of clock in
/sys/module/printk/parameters/time so userspace programs can interpret the
timestamp.

Real clock & 32-bit systems:  Selecting the real clock printk timestamp may
lead to unlikely situations where a timestamp is wrong because the real time
without the protection of a sequence lock when printk_get_ts() is set
printk_get_real_ns().

v2: Use peterz's suggested Kconfig options.  Merge patchset together.
Fix i386 !CONFIG_PRINTK builds.

v3: Fixed x86_64_defconfig. Added printk_time_type enum and
printk_time_str for better output. Added BOOTTIME clock functionality.

v4: Fix messages, add additional printk.time options, and fix configs.

v5: Renaming of structures, and allow printk_time_set() to
evaluate substrings of entries (eg: allow 'r', 'real', 'realtime').  From
peterz, make fast functions return 0 until timekeeping is initialized
(removes timekeeping_active & ktime_get_boot|real_log_ts() suggested by
 tglx and adds ktime_get_real_offset()).  Switch to a function pointer
for printk_get_ts() and reference fast functions.  Make timestamp_sources enum
match choice options for CONFIG_PRINTK_TIME (adds PRINTK_TIME_UNDEFINED).

v6: Define PRINTK_TIME_UNDEFINED for !CONFIG_PRINTK builds.  Separate
timekeeping changes into separate patch.  Minor include file cleanup.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
---
 Documentation/admin-guide/kernel-parameters.txt|   6 +-
 arch/arm/configs/aspeed_g4_defconfig   |   2 +-
 arch/arm/configs/aspeed_g5_defconfig   |   2 +-
 arch/arm/configs/axm55xx_defconfig |   2 +-
 arch/arm/configs/bcm2835_defconfig |   2 +-
 arch/arm/configs/colibri_pxa270_defconfig  |   2 +-
 arch/arm/configs/colibri_pxa300_defconfig  |   2 +-
 arch/arm/configs/dove_defconfig|   2 +-
 arch/arm/configs/efm32_defconfig   |   2 +-
 arch/arm/configs/exynos_defconfig  |   2 +-
 arch/arm/configs/ezx_defconfig |   2 +-
 arch/arm/configs/h5000_defconfig   |   2 +-
 arch/arm/configs/hisi_defconfig|   2 +-
 arch/arm/configs/imote2_defconfig  |   2 +-
 arch/arm/configs/imx_v6_v7_defconfig   |   2 +-
 arch/arm/configs/keystone_defconfig|   2 +-
 arch/arm/configs/lpc18xx_defconfig |   2 +-
 arch/arm/configs/magician_defconfig|   2 +-
 arch/arm/configs/mmp2_defconfig|   2 +-
 arch/arm/configs/moxart_defconfig  |   2 +-
 arch/arm/configs/mps2_defconfig|   2 +-
 arch/arm/configs/multi_v7_defconfig|   2 +-
 arch/arm/configs/mvebu_v7_defconfig|   2 +-
 arch/arm/configs/mxs_defconfig |   2 +-
 arch/arm/configs/omap2plus_defconfig   |   2 +-
 arch/arm/configs/pxa168_defconfig  |   2 +-
 arch/arm/configs/pxa3xx_defconfig  |   2 +-
 arch/arm/configs/pxa910_defconfig  |   2 +-
 arch/arm/configs/pxa_defconfig |   2 +-
 arch/arm/configs/qcom_defconf

[PATCH 0/2 v6] printk: Add new timestamps

2017-08-16 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Add monotonic, boottime, and real clock timestamps in addition to the existing
local hardware clock timestamp.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org

Prarit Bhargava (2):
  time: Make fast functions return 0 before timekeeping is initialized
  printk: Add monotonic, boottime, and realtime timestamps

 Documentation/admin-guide/kernel-parameters.txt|   6 +-
 arch/arm/configs/aspeed_g4_defconfig   |   2 +-
 arch/arm/configs/aspeed_g5_defconfig   |   2 +-
 arch/arm/configs/axm55xx_defconfig |   2 +-
 arch/arm/configs/bcm2835_defconfig |   2 +-
 arch/arm/configs/colibri_pxa270_defconfig  |   2 +-
 arch/arm/configs/colibri_pxa300_defconfig  |   2 +-
 arch/arm/configs/dove_defconfig|   2 +-
 arch/arm/configs/efm32_defconfig   |   2 +-
 arch/arm/configs/exynos_defconfig  |   2 +-
 arch/arm/configs/ezx_defconfig |   2 +-
 arch/arm/configs/h5000_defconfig   |   2 +-
 arch/arm/configs/hisi_defconfig|   2 +-
 arch/arm/configs/imote2_defconfig  |   2 +-
 arch/arm/configs/imx_v6_v7_defconfig   |   2 +-
 arch/arm/configs/keystone_defconfig|   2 +-
 arch/arm/configs/lpc18xx_defconfig |   2 +-
 arch/arm/configs/magician_defconfig|   2 +-
 arch/arm/configs/mmp2_defconfig|   2 +-
 arch/arm/configs/moxart_defconfig  |   2 +-
 arch/arm/configs/mps2_defconfig|   2 +-
 arch/arm/configs/multi_v7_defconfig|   2 +-
 arch/arm/configs/mvebu_v7_defconfig|   2 +-
 arch/arm/configs/mxs_defconfig |   2 +-
 arch/arm/configs/omap2plus_defconfig   |   2 +-
 arch/arm/configs/pxa168_defconfig  |   2 +-
 arch/arm/configs/pxa3xx_defconfig  |   2 +-
 arch/arm/configs/pxa910_defconfig  |   2 +-
 arch/arm/configs/pxa_defconfig |   2 +-
 arch/arm/configs/qcom_defconfig|   2 +-
 arch/arm/configs/raumfeld_defconfig|   2 +-
 arch/arm/configs/shmobile_defconfig|   2 +-
 arch/arm/configs/socfpga_defconfig |   2 +-
 arch/arm/configs/stm32_defconfig   |   2 +-
 arch/arm/configs/sunxi_defconfig   |   2 +-
 arch/arm/configs/tango4_defconfig  |   2 +-
 arch/arm/configs/tegra_defconfig   |   2 +-
 arch/arm/configs/u300_defconfig|   2 +-
 arch/arm/configs/u8500_defconfig   |   2 +-
 arch/arm/configs/vt8500_v6_v7_defconfig|   2 +-
 arch/arm/configs/xcep_defconfig|   2 +-
 arch/arm/configs/zx_defconfig  |   2 +-
 arch/arm64/configs/defconfig   |   2 +-
 arch/m68k/configs/amcore_defconfig |   2 +-
 arch/mips/configs/ath25_defconfig  |   2 +-
 arch/mips/configs/bcm47xx_defconfig|   2 +-
 arch/mips/configs/bmips_be_defconfig   |   2 +-
 arch/mips/configs/bmips_stb_defconfig  |   2 +-
 arch/mips/configs/ci20_defconfig   |   2 +-
 arch/mips/configs/generic_defconfig|   2 +-
 arch/mips/configs/lemote2f_defconfig   |   2 +-
 arch/mi

[PATCH v5] printk: Add monotonic, boottime, and realtime timestamps

2017-08-10 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Make printk output different timestampes by adding options for no
timestamp, the local hardware clock, the monotonic clock, the boottime
clock, and the real clock.  Allow a user to pick one of the clocks by
using the printk.time kernel parameter.  Output the type of clock in
/sys/module/printk/parameters/time so userspace programs can interpret the
timestamp.

Real clock & 32-bit systems:  Selecting the real clock printk timestamp may
lead to unlikely situations where a timestamp is wrong because the real time
offset is read without the protection of a sequence lock in the call to
ktime_get_log_ts() in printk_get_ts().

v2: Use peterz's suggested Kconfig options.  Merge patchset together.
Fix i386 !CONFIG_PRINTK builds.

v3: Fixed x86_64_defconfig. Added printk_time_type enum and
printk_time_str for better output. Added BOOTTIME clock functionality.

v4: Fix messages, add additional printk.time options, and fix configs.

v5: Renaming of structures, and allow printk_time_set() to
evaluate substrings of entries (eg: allow 'r', 'real', 'realtime').  From
peterz, make fast functinos return 0 until timekeeping is initialized
(removes timekeeping_active & ktime_get_boot|real_log_ts() suggested by
 tglx and adds ktime_get_real_offset()).  Switch to a function pointer
for printk_get_ts() and reference fast functions.  Make timestamp_sources enum
match choice options for CONFIG_PRINTK_TIME (adds PRINTK_TIME_UNDEFINED).

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
---
 Documentation/admin-guide/kernel-parameters.txt|   6 +-
 arch/arm/configs/aspeed_g4_defconfig   |   2 +-
 arch/arm/configs/aspeed_g5_defconfig   |   2 +-
 arch/arm/configs/axm55xx_defconfig |   2 +-
 arch/arm/configs/bcm2835_defconfig |   2 +-
 arch/arm/configs/colibri_pxa270_defconfig  |   2 +-
 arch/arm/configs/colibri_pxa300_defconfig  |   2 +-
 arch/arm/configs/dove_defconfig|   2 +-
 arch/arm/configs/efm32_defconfig   |   2 +-
 arch/arm/configs/exynos_defconfig  |   2 +-
 arch/arm/configs/ezx_defconfig |   2 +-
 arch/arm/configs/h5000_defconfig   |   2 +-
 arch/arm/configs/hisi_defconfig|   2 +-
 arch/arm/configs/imote2_defconfig  |   2 +-
 arch/arm/configs/imx_v6_v7_defconfig   |   2 +-
 arch/arm/configs/keystone_defconfig|   2 +-
 arch/arm/configs/lpc18xx_defconfig |   2 +-
 arch/arm/configs/magician_defconfig|   2 +-
 arch/arm/configs/mmp2_defconfig|   2 +-
 arch/arm/configs/moxart_defconfig  |   2 +-
 arch/arm/configs/mps2_defconfig|   2 +-
 arch/arm/configs/multi_v7_defconfig|   2 +-
 arch/arm/configs/mvebu_v7_defconfig|   2 +-
 arch/arm/configs/mxs_defconfig |   2 +-
 arch/arm/configs/omap2plus_defconfig   |   2 +-
 arch/arm/configs/pxa168_defconfig  |   2 +-
 arch/arm/configs/pxa3xx_defconfig  |   2 +-
 arch/arm/configs/pxa910_defconfig  |   2 +-
 arch/arm/configs/pxa_defconfig |   2 +-
 arch/arm/configs/qcom_defconfig|   2 +-
 arch/arm/configs/raumfeld_defconfig|   2 +-
 arch/arm/configs/shmobile_d

Re: [PATCH v3] printk: Add boottime and real timestamps

2017-08-09 Thread Prarit Bhargava


On 08/09/2017 02:24 PM, Luis R. Rodriguez wrote:
> On Mon, Aug 07, 2017 at 02:17:33PM -0400, Prarit Bhargava wrote:
>>
>>
>> On 08/07/2017 01:14 PM, Luis R. Rodriguez wrote:
>>
>>>
>>> Note printk_late_init() is a late_initcall(). This means if the
>>> printk_time_setting was disabled it will take a while to enable it. 
>>> Enabling it
>>> is done at the device_initcall(), so if printk setting is disabled but a 
>>> user
>>> enables it with a toggle of the module param there is a period of time 
>>> during
>>> which time resolution would be different. 
>>
>> I'm not sure I follow your comment.  Could you elaborate with an example of
>> what you think is going wrong or might be confusing?
> 
> Sure let's consider this:
> 
> +static u64 printk_get_ts(void)
> +{
> + u64 mono, offset_real;
> +
> + if (printk_time <= PRINTK_TIME_LOCAL)
> + return local_clock();
> +
> + if (printk_time == PRINTK_TIME_BOOT)
> + return ktime_get_boot_log_ts();
> +
> + mono = ktime_get_real_log_ts(_real);
> +
> + if (printk_time == PRINTK_TIME_MONO)
> + return mono;
> +
> + return mono + offset_real;
> +}
> 
> So even if printk_time was flipped in the end the backend routines used will 
> be
> local_clock(), ,ktime_get_boot_log_ts() or ktime_get_real_log_ts().
> 
> This is used here;
>  
> @@ -1643,7 +1756,7 @@ static bool cont_add(int facility, int level, enum 
> log_flags flags, const char *
>   cont.facility = facility;
>   cont.level = level;
>   cont.owner = current;
> - cont.ts_nsec = local_clock();
> + cont.ts_nsec = printk_get_ts();
>   cont.flags = flags;
>   }
> 
> 
> But lets inspect these new calls:
>  
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> @@ -477,6 +479,24 @@ u64 notrace ktime_get_boot_fast_ns(void)
>  }
>  EXPORT_SYMBOL_GPL(ktime_get_boot_fast_ns);
>  
> +u64 ktime_get_real_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();
> +}
> +
> +u64 ktime_get_boot_log_ts(void)
> +{
> + if (timekeeping_active)
> + return ktime_get_boot_fast_ns();
> + else
> + return local_clock();
> +}
> +
> 
> So they are really only effectively calling something other than
> what lock_clock() returns *iff* timekeeping_active is true. But
> this is only set later at the respective device_initcall() in this
> file:
> 
> @@ -1530,6 +1550,8 @@ void __init timekeeping_init(void)
>  
>   write_seqcount_end(_core.seq);
>   raw_spin_unlock_irqrestore(_lock, flags);
> +
> + timekeeping_active = 1;
>  }
>  
> 
> So when the boot param is processed and prints out that it has
> changed someone inspecting any time setting after that print
> may assume its using after that ktime_get_mono_fast_ns() or
> time_get_boot_fast_ns() but this is not accurate, it will use
> local_clock() until *after* device_initcall().
> 
> So in between boot and this particular device_initcall() time
> resolution can only be local_time(). Seems worth documenting
> that.

I've moved to a different model of using a fn ptr for print_get_ts() and using
peterz's suggestion of returning 0 until the timekeeping is initialized, so this
won't be a problem any more.

P.

> 
>   Luis
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] printk: Add monotonic, boottime, and realtime timestamps

2017-08-08 Thread Prarit Bhargava


On 08/08/2017 04:28 AM, Peter Zijlstra wrote:
> On Mon, Aug 07, 2017 at 01:36:39PM -0700, Paul E. McKenney wrote:
>> On Mon, Aug 07, 2017 at 04:06:09PM -0400, Prarit Bhargava wrote:
> 
>>> peterz?  Want to offer a suggestion?  The issue is that I'm changing a bool
>>> config option to an int and that impacts all the arch's defconfigs.  John 
>>> points
>>> out that this is a lot of churn and we're both wondering if there's a 
>>> better way
>>> to do the configs.
>>
>> The usual approach is to keep the old bool Kconfig option, and add another
>> int Kconfig option that depends on the original one.  The tests for
>> the int value get a bit more complex, but one way to handle this is to
>> define a cpp macro something like the following:
>>
>> #ifdef CONFIG_OLD_OPTION
>> #define CPP_NEW_OPTION 0
>> #else
>> #define CPP_NEW_OPTION CONFIG_NEW_OPTION
>> #endif
>>
>> Then use CPP_NEW_OPTION, where zero means disabled and other numbers
>> select the available options.
>>
>> Adjust to suit depending on what values mean what.
>>
>> Another approach is to make the range of the new Kconfig option
>> depend on the old option:
>>
>> config NEW_OPTION
>>  int "your description here"
>>  range 1 5 if OLD_OPTION
>>  range 0 0 if !OLD_OPTION
>>  default 0
>>  help
>>your help here
>>
>> Again, adjust to suit depending on what values mean what.
> 
> Right this. Except I don't see the !OLD_OPTION working as expected.
> A 'new' config will not include the old one, so the !OLD_OPTION thing
> will 'always' be true.
> 
> So your:
> 
>> @@ -1,8 +1,46 @@
>>  menu "printk and dmesg options"
>>
>> +choice
>> +   prompt "printk default clock"
>> +   config PRINTK_TIME_DISABLE
>> +   bool "Disabled"
>> +   help
>> +Selecting this option disables the time stamps of printk().
>> +
>> +   config PRINTK_TIME_LOCAL
>> +   bool "Local Clock"
>> +   help
>> + Selecting this option causes the time stamps of printk() to be
>> + stamped with the unadjusted hardware clock.
>> +
>> +   config PRINTK_TIME_BOOT
>> +   bool "CLOCK_BOOTTIME"
>> +   help
>> + Selecting this option causes the time stamps of printk() to be
>> + stamped with the adjusted boottime clock.
>> +
>> +   config PRINTK_TIME_MONO
>> +   bool "CLOCK_MONOTONIC"
>> +   help
>> + Selecting this option causes the time stamps of printk() to be
>> + stamped with the adjusted monotonic clock.
>> +
>> +   config PRINTK_TIME_REAL
>> +   bool "CLOCK_REALTIME"
>> +   help
>> + Selecting this option causes the time stamps of printk() to be
>> + stamped with the adjusted realtime clock.
>> +
>> +endchoice
>> +
>>  config PRINTK_TIME
> 
> Change that into something like:
> 
> config PRINTK_CLOCK
> 
> 
>> -   bool "Show timing information on printks"
>> +   int "Show time stamp information on printks"
>> depends on PRINTK
>> +   default 0 if PRINTK_TIME_DISABLE
>> +   default 1 if PRINTK_TIME_LOCAL
> 
> And that into:
> 
>   default 1 if PRINTK_TIME_LOCAL || PRINTK_TIME
> 
>> +   default 2 if PRINTK_TIME_BOOT
>> +   default 3 if PRINTK_TIME_MONO
>> +   default 4 if PRINTK_TIME_REAL
>>help
>>  Selecting this option causes time stamps of the printk()
> 
> Then the old PRINTK_TIME symbol will auto-convert into the new
> equivalent.
> 

I don't think there's an easy code way around this.  Essentially this Kconfig
code boils down to properly evaluating

config PRINTK_CLOCK
default 1 if PRINTK_TIME
default 0

where there is no Kconfig entry for PRINTK_TIME.

If undefined CONFIG_PRINTK_TIME is used in a config, it is immediately
scrubbed by the kconfig script so it doesn't "exist" when CONFIG_PRINTK_CLOCK
is evaluated.  The result of that is CONFIG_PRINT_CLOCK=0.

I tried

config PRINTK_TIME
bool "old config option"

then I end up with both a CONFIG_PRINTK_CLOCK=1 and a CONFIG_PRINTK_TIME=y in
the resulting config which is confusing.

I've debated using the other suggestion that Paul made but TBH (sorry
Paul) it seems like I'm avoiding the real but noisy solution of

s/PRINTK_TIME=y/PRINTK_TIME=1/g

I'm obviously open to other suggestions...

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] printk: Add monotonic, boottime, and realtime timestamps

2017-08-08 Thread Prarit Bhargava


On 08/07/2017 08:19 PM, Sergey Senozhatsky wrote:
> On (08/07/17 11:52), Prarit Bhargava wrote:



> [..]
>> +if (strlen(param) == 1) {
>> +/* Preserve legacy boolean settings */
>> +if (!strcmp("0", param) || !strcmp("n", param) ||
>> +!strcmp("N", param))
>> +_printk_time = PRINTK_TIME_DISABLE;
>> +if (!strcmp("1", param) || !strcmp("y", param) ||
>> +!strcmp("Y", param))
>> +_printk_time = PRINTK_TIME_LOCAL;
>> +}
>> +if (_printk_time == -1) {
>> +for (stamp = 0; stamp <= 4; stamp++) {
>> +if (!strncmp(printk_time_str[stamp], param,
>> + strlen(param))) {
>> +_printk_time = stamp;
>> +break;
>> +}
>> +}
>> +}
> 
>   you can use match_string() here.

match_string doesn't match correctly.  Others have made requests for, for
example, printk.time=r printk.time=real, etc.  Instead of butchering the code
for a bunch of strings I chose to allow partial matching and simplify the code.

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] printk: Add monotonic, boottime, and realtime timestamps

2017-08-08 Thread Prarit Bhargava


On 08/07/2017 08:19 PM, Sergey Senozhatsky wrote:
> On (08/07/17 11:52), Prarit Bhargava wrote:
> [..]
>> +/**
>> + * enum printk_time_type - Timestamp types for printk() messages.
>> + * @PRINTK_TIME_DISABLE: No time stamp.
>> + * @PRINTK_TIME_LOCAL: Local hardware clock timestamp.
>> + * @PRINTK_TIME_BOOT: Boottime clock timestamp.
>> + * @PRINTK_TIME_MONO: Monotonic clock timestamp.
>> + * @PRINTK_TIME_REAL: Realtime clock timestamp.  On 32-bit
>> + * systems selecting the real clock printk timestamp may lead to unlikely
>> + * situations where a timestamp is wrong because the real time offset is 
>> read
>> + * without the protection of a sequence lock in the call to 
>> ktime_get_log_ts()
>> + * in printk_get_ts() below.
>> + */
>> +enum printk_time_type {
>> +PRINTK_TIME_DISABLE = 0,
>> +PRINTK_TIME_LOCAL = 1,
>> +PRINTK_TIME_BOOT = 2,
>> +PRINTK_TIME_MONO = 3,
>> +PRINTK_TIME_REAL = 4,
>> +};
> 
> may be call the entire thing 'timestamp surces' or something?

Done.

> 
> [..]
>> +if (strlen(param) == 1) {
>> +/* Preserve legacy boolean settings */
>> +if (!strcmp("0", param) || !strcmp("n", param) ||
>> +!strcmp("N", param))
>> +_printk_time = PRINTK_TIME_DISABLE;
>> +if (!strcmp("1", param) || !strcmp("y", param) ||
>> +!strcmp("Y", param))
>> +_printk_time = PRINTK_TIME_LOCAL;
>> +}
>> +if (_printk_time == -1) {
>> +for (stamp = 0; stamp <= 4; stamp++) {
>> +if (!strncmp(printk_time_str[stamp], param,
>> + strlen(param))) {
>> +_printk_time = stamp;
>> +break;
>> +}
>> +}
>> +}
> 
>   you can use match_string() here.
> 
>> +if (_printk_time == -1) {
>> +pr_warn("printk: invalid timestamp value %s\n", param);
>> +return -EINVAL;
>> +}
> 
> `invalid timestamp value' is confusing.

I can't think of anything better than 'invalid timestamp option'.  If you find
that confusing, please explain why.

> 
> 
>> +} else if ((printk_time_setting != _printk_time) &&
>> +   (_printk_time != 0)) {
>> +pr_warn("printk: timestamp can only be set to 0(disabled) or 
>> %s\n",
>> +printk_time_str[printk_time_setting]);
> 
> ditto.

Changed to 'timestamp can only e set to 0, disabled, or '.

> 
> 
>> +return -EINVAL;
>> +}
>> +
>> +printk_time = _printk_time;
>> +pr_info("printk: timestamp set to %s\n", printk_time_str[printk_time]);
> 
> ditto.

I don't find this confusing _at all_.  Please offer an explanation of why you
think that message is confusing.

> 
> 
> [..]
> 
>> +static u64 printk_get_ts(void)
>> +{
>> +u64 mono, offset_real;
>> +
>> +if (printk_time <= PRINTK_TIME_LOCAL)
>> +return local_clock();
>> +
>> +if (printk_time == PRINTK_TIME_BOOT)
>> +return ktime_get_boot_log_ts();
>> +
>> +mono = ktime_get_real_log_ts(_real);
>> +
>> +if (printk_time == PRINTK_TIME_MONO)
>> +return mono;
>> +
>> +return mono + offset_real;
>> +}
> 
> this looks hard...

See previous suggestion from peterz to switch to a fn ptr.

> 
>> +static int printk_time;
>> +static int printk_time_setting;
> 
> how about s/printk_time_setting/printk_time_source/? or something similar?

Sure.

P.

> 
>   -ss
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] printk: Add monotonic, boottime, and realtime timestamps

2017-08-07 Thread Prarit Bhargava


On 08/07/2017 02:47 PM, John Stultz wrote:
> On Mon, Aug 7, 2017 at 11:04 AM, Prarit Bhargava <pra...@redhat.com> wrote:
>> On 08/07/2017 12:52 PM, John Stultz wrote:
>>> Still not quite following why you're updating all the defconfigs. I'd
>>> make sure the Kconfig default settings are right, and leave updating
>>> the defconfig to arch/device maintainers. It adds a lot of noise to
>>> the patch.
>>
>> Hmm ... I thought it was up to the patch submitter to make sure that
>> 'make defconfig' still worked?  Are you sure I can leave that broken?
>>
>> /me *really* doesn't want to get yelled at by every arch maintainer.
> 
> No. Don't break systems, but at the same time, can't you use the
> default value in Kconfig to set it properly so the old defconfig
> settings don't really matter?
> 
> Apologies if I've not followed the issue properly, but it is odd, as
> I'm not sure I can think of a patch I've seen before that had so much
> defconfig noise in it.  Again, I've not looked into it closely, so it
> may just be my own ignorance, but it makes me suspect there is a
> better way.
> 

peterz?  Want to offer a suggestion?  The issue is that I'm changing a bool
config option to an int and that impacts all the arch's defconfigs.  John points
out that this is a lot of churn and we're both wondering if there's a better way
to do the configs.

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] printk: Add boottime and real timestamps

2017-08-07 Thread Prarit Bhargava


On 08/07/2017 01:14 PM, Luis R. Rodriguez wrote:

> 
> Note printk_late_init() is a late_initcall(). This means if the
> printk_time_setting was disabled it will take a while to enable it. Enabling 
> it
> is done at the device_initcall(), so if printk setting is disabled but a user
> enables it with a toggle of the module param there is a period of time during
> which time resolution would be different. 

I'm not sure I follow your comment.  Could you elaborate with an example of
what you think is going wrong or might be confusing?

P.

> Perhaps for some this is not useful
> information but for others I think this would be very valuable.
> 
> There is also something to say about the time in between initializing access
> to ktime_get_mono_fast_ns(), how early is this reliable?
> 
> All these things could be mentioned on the documentation.
> 
>   Luis
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] printk: Add monotonic, boottime, and realtime timestamps

2017-08-07 Thread Prarit Bhargava


On 08/07/2017 12:58 PM, Mark Salyzyn wrote:
> On 08/07/2017 08:52 AM, Prarit Bhargava wrote:
>> diff --git a/arch/arm/configs/aspeed_g4_defconfig
>> b/arch/arm/configs/aspeed_g4_defconfig
>> index cfc2465e8b77..5f3c50914e92 100644
>> --- a/arch/arm/configs/aspeed_g4_defconfig
>> +++ b/arch/arm/configs/aspeed_g4_defconfig
>> @@ -162,7 +162,7 @@ CONFIG_JFFS2_FS_XATTR=y
>>   CONFIG_UBIFS_FS=y
>>   CONFIG_SQUASHFS=y
>>   CONFIG_SQUASHFS_XZ=y
>> -CONFIG_PRINTK_TIME=y
>> +CONFIG_PRINTK_TIME_LOCAL=y
>>   CONFIG_DYNAMIC_DEBUG=y
>>   CONFIG_STRIP_ASM_SYMS=y
>>   CONFIG_DEBUG_FS=y
> Many have had misgivings, let me try another pass at this.
> 
> We (royal we) should really look into adjusting configuration parsing to allow
> an easy transition from boolean to selection ... I am sure this is not the 
> first
> time bistate/tristate was moved to a number.
> 
> An idea? Maybe look into a way to deal with this to use something _other_ than
> CONFIG_PRINTK_TIME to hold the selection, and keep a (hidden/legacy?)
> CONFIG_PRINTK_TIME that when selected sets CONFIG_PRINTK_TIME_LOCAL, and 
> switch
> to _not_ CONFIG_PRINTK_TIME_DISABLE as the internal mechanical replacement for
> it. I do not know how disruptive this will be, but is worth it if the codebase
> supports it, and legacy config retained?

I looked for one but couldn't find one.  The kernel is a big place, though, and
perhaps it already exists :/.

>> +
>> +static int printk_time_set(const char *val, const struct kernel_param *kp)
>> +{
>> +char *param = strstrip((char *)val);
>> +int _printk_time = -1;
>> +int stamp;
>> +
>> +if (strlen(param) == 1) {
>> +/* Preserve legacy boolean settings */
>> +if (!strcmp("0", param) || !strcmp("n", param) ||
> if strlen(param) == 1, then param[0] == '0' etc works fine and is KISS.
>> +/*
>> + * Only allow enabling and disabling of the current printk_time
>> + * setting.  Changing it from one setting to another confuses
>> + * userspace.
>> + */
>> +if (printk_time_setting == PRINTK_TIME_DISABLE) {
>> +printk_time_setting = _printk_time;
>> +} else if ((printk_time_setting != _printk_time) &&
>> +   (_printk_time != 0)) {
>> +pr_warn("printk: timestamp can only be set to 0(disabled) or %s\n",
>> +printk_time_str[printk_time_setting]);
>> +return -EINVAL;
>> +}
> I agree with the restriction in the general case.  However (as hinted at
> before() #ifdef CONFIG_PRINTK_TIME_RESTRICT (default y, or #ifndef
> CONFIG_PRINTK_TIME_DEBUG default n) around this will allow us users to choose 
> if
> we are confused or not. I can see being able to change it on the fly as an
> option. Especially since we have /sys/module/printk/parameters/time.

Yeah, but I think that should be a later enhancement.

P.

> 
> -- Mark
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] printk: Add monotonic, boottime, and realtime timestamps

2017-08-07 Thread Prarit Bhargava


On 08/07/2017 12:52 PM, John Stultz wrote:
> On Mon, Aug 7, 2017 at 8:52 AM, Prarit Bhargava <pra...@redhat.com> wrote:
>> printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
>> timestamp to printk messages.  The local hardware clock loses time each
>> day making it difficult to determine exactly when an issue has occurred in
>> the kernel log, and making it difficult to determine how kernel and
>> hardware issues relate to each other in real time.
>>
>> Make printk output different timestampes by adding options for no
>> timestamp, the local hardware clock, the monotonic clock, the boottime
>> clock, and the real clock.  Allow a user to pick one of the clocks by
>> using the printk.time kernel parameter.  Output the type of clock in
>> /sys/module/printk/parameters/time so userspace programs can interpret the
>> timestamp.
>>
>> Real clock & 32-bit systems:  Selecting the real clock printk timestamp may
>> lead to unlikely situations where a timestamp is wrong because the real time
>> offset is read without the protection of a sequence lock in the call to
>> ktime_get_log_ts() in printk_get_ts().
>>
>> v2: Use peterz's suggested Kconfig options.  Merge patchset together.
>> Fix i386 !CONFIG_PRINTK builds.
>>
>> v3: Fixed x86_64_defconfig. Added printk_time_type enum and
>> printk_time_str for better output. Added BOOTTIME clock functionality.
>>
>> v4: Fix messages, add additional printk.time options, and fix configs.
>>
>> Signed-off-by: Prarit Bhargava <pra...@redhat.com>
>> Cc: Mark Salyzyn <saly...@android.com>
>> Cc: Jonathan Corbet <cor...@lwn.net>
>> Cc: Petr Mladek <pmla...@suse.com>
>> Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
>> Cc: Steven Rostedt <rost...@goodmis.org>
>> Cc: John Stultz <john.stu...@linaro.org>
>> Cc: Thomas Gleixner <t...@linutronix.de>
>> Cc: Stephen Boyd <sb...@codeaurora.org>
>> Cc: Andrew Morton <a...@linux-foundation.org>
>> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
>> Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
>> Cc: Christoffer Dall <cd...@linaro.org>
>> Cc: Deepa Dinamani <deepa.ker...@gmail.com>
>> Cc: Ingo Molnar <mi...@kernel.org>
>> Cc: Joel Fernandes <joe...@google.com>
>> Cc: Prarit Bhargava <pra...@redhat.com>
>> Cc: Kees Cook <keesc...@chromium.org>
>> Cc: Peter Zijlstra <pet...@infradead.org>
>> Cc: Geert Uytterhoeven <geert+rene...@glider.be>
>> Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
>> Cc: Nicholas Piggin <npig...@gmail.com>
>> Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
>> Cc: Olof Johansson <o...@lixom.net>
>> Cc: Josh Poimboeuf <jpoim...@redhat.com>
>> Cc: linux-doc@vger.kernel.org
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt|   6 +-
>>  arch/arm/configs/aspeed_g4_defconfig   |   2 +-
>>  arch/arm/configs/aspeed_g5_defconfig   |   2 +-
>>  arch/arm/configs/axm55xx_defconfig |   2 +-
>>  arch/arm/configs/bcm2835_defconfig |   2 +-
>>  arch/arm/configs/colibri_pxa270_defconfig  |   2 +-
>>  arch/arm/configs/colibri_pxa300_defconfig  |   2 +-
>>  arch/arm/configs/dove_defconfig|   2 +-
>>  arch/arm/configs/efm32_defconfig   |   2 +-
>>  arch/arm/configs/exynos_defconfig  |   2 +-
>>  arch/arm/configs/ezx_defconfig |   2 +-
>>  arch/arm/configs/h5000_defconfig   |   2 +-
>>  arch/arm/configs/hisi_defconfig|   2 +-
>>  arch/arm/configs/imote2_defconfig  |   2 +-
>>  arch/arm/configs/imx_v6_v7_defconfig   |   2 +-
>>  arch/arm/configs/keystone_defconfig|   2 +-
>>  arch/arm/configs/lpc18xx_defconfig |   2 +-
>>  arch/arm/configs/magician_defconfig|   2 +-
>>  arch/arm/configs/mmp2_defconfig|   2 +-
>>  arch/arm/configs/moxart_defconfig  |   2 +-
>>  arch/arm/configs/mps2_defconfig|   2 +-
>>  arch/arm/configs/multi_v7_defconfig|   2 +-
>>  arch/arm/configs/mvebu_v7_defconfig|   2 +-
>>  arch/arm/configs/mxs_defconfig |   2 +-
>>  arch/arm/configs/omap2plus_defconfig   |   2 +-
>>  arch/arm/configs/pxa168_defconfig  |   2 +-
>> 

Re: [PATCH v3] printk: Add boottime and real timestamps

2017-08-07 Thread Prarit Bhargava


On 08/04/2017 11:36 AM, Mark Salyzyn wrote:
> On 08/03/2017 06:18 PM, Prarit Bhargava wrote:
> 
>> diff --git a/arch/arm/configs/aspeed_g4_defconfig
>> b/arch/arm/configs/aspeed_g4_defconfig
>> index cfc2465e8b77..6c73c305ad17 100644
>> --- a/arch/arm/configs/aspeed_g4_defconfig
>> +++ b/arch/arm/configs/aspeed_g4_defconfig
>> @@ -162,7 +162,9 @@ CONFIG_JFFS2_FS_XATTR=y
>>  CONFIG_UBIFS_FS=y
>>  CONFIG_SQUASHFS=y
>>  CONFIG_SQUASHFS_XZ=y
>> -CONFIG_PRINTK_TIME=y
>> +# CONFIG_PRINTK_TIME_DISABLE is not set
>> +CONFIG_PRINTK_TIME_LOCAL=Y
>> +CONFIG_PRINTK_TIME=1
>>  CONFIG_DYNAMIC_DEBUG=y
>>  CONFIG_STRIP_ASM_SYMS=y
>>  CONFIG_DEBUG_FS=y
> 
> savedefconfig tells me otherwise:
> 
> -CONFIG_PRINTK_TIME=y
> +CONFIG_PRINTK_TIME_LOCAL=y
> 
> is all that is needed, the rest is fluff and will cause a savedefconfig 
> mismatch
> error.
>> +static int printk_time_set(const char *val, const struct kernel_param *kp)
>> +{
>> +char *param = strstrip((char *)val);
>> +int _printk_time;
>> +
>> +if (strlen(param) != 1)
>> +return -EINVAL;
> (see below) strlen is so restrictive, wouldn't it be nice(tm) to allow "boot",
> "monotonic" or "realtime" strings? Certainly KISS can handle the first letters
> for next to zero cost. (switch statement optimization)

Thanks Mark,

I had asked this question before and didn't get an answer: Do I need to
worry about the ABI of /sys/modules/printk/parameters/time.  Since it hasn't
been explicitly added to the stable ABI files, I'm going to assume it wasn't
important enough to add it.  Looking at google, however, leads to a lot
of hits for 'printk.time' kernel parameters settings so I think I better
maintain the legacy boolean settings.

I'm going to do: 0/n/N/1/y/Y to maintain the existing ABI, and add (as you
suggest above 'disable|local|boottime|monotonic|realtime' as valid
parameters.

>> +
>> +switch (param[0]) {
>> +case '0':
>> +case 'n':
>> +case 'N':
> I would wish for a few more 'convenience' cases:
> 
> case 'd': case 'D': /* Disable */

It would be trivial then to add only the lower case options (I dislike upper
case settings ...) by adding

!strcmp(printk_time_str[0], param, 1)

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3] printk: Add boottime and real timestamps

2017-08-03 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Make printk output different timestampes by adding options for no
timestamp, the local hardware clock, the monotonic clock, and the real
clock.  Allow a user to pick one of the clocks by using the printk.time
kernel parameter.  Output the type of clock in
/sys/module/printk/parameters/time so userspace programs can interpret the
timestamp.

Real clock & 32-bit systems:  Selecting the real clock printk timestamp may
lead to unlikely situations where a timestamp is wrong because the real time
offset is read without the protection of a sequence lock in the call to
ktime_get_log_ts() in printk_get_ts().

v2: Use peterz's suggested Kconfig options.  Merge patchset together.
Fix i386 !CONFIG_PRINTK builds.

v3: Fixed x86_64_defconfig. Added & kdoc'ifyied printk_time_type enum and
printk_time_str for better output. Added BOOTTIME clock functionality.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
---
 Documentation/admin-guide/kernel-parameters.txt|   6 +-
 arch/arm/configs/aspeed_g4_defconfig   |   4 +-
 arch/arm/configs/aspeed_g5_defconfig   |   4 +-
 arch/arm/configs/axm55xx_defconfig |   4 +-
 arch/arm/configs/bcm2835_defconfig |   4 +-
 arch/arm/configs/colibri_pxa270_defconfig  |   4 +-
 arch/arm/configs/colibri_pxa300_defconfig  |   4 +-
 arch/arm/configs/dove_defconfig|   4 +-
 arch/arm/configs/efm32_defconfig   |   4 +-
 arch/arm/configs/exynos_defconfig  |   4 +-
 arch/arm/configs/ezx_defconfig |   4 +-
 arch/arm/configs/h5000_defconfig   |   4 +-
 arch/arm/configs/hisi_defconfig|   4 +-
 arch/arm/configs/imote2_defconfig  |   4 +-
 arch/arm/configs/imx_v6_v7_defconfig   |   4 +-
 arch/arm/configs/keystone_defconfig|   4 +-
 arch/arm/configs/lpc18xx_defconfig |   4 +-
 arch/arm/configs/magician_defconfig|   4 +-
 arch/arm/configs/mmp2_defconfig|   4 +-
 arch/arm/configs/moxart_defconfig  |   4 +-
 arch/arm/configs/mps2_defconfig|   4 +-
 arch/arm/configs/multi_v7_defconfig|   4 +-
 arch/arm/configs/mvebu_v7_defconfig|   4 +-
 arch/arm/configs/mxs_defconfig |   4 +-
 arch/arm/configs/omap2plus_defconfig   |   4 +-
 arch/arm/configs/pxa168_defconfig  |   4 +-
 arch/arm/configs/pxa3xx_defconfig  |   4 +-
 arch/arm/configs/pxa910_defconfig  |   4 +-
 arch/arm/configs/pxa_defconfig |   4 +-
 arch/arm/configs/qcom_defconfig|   4 +-
 arch/arm/configs/raumfeld_defconfig|   4 +-
 arch/arm/configs/shmobile_defconfig|   4 +-
 arch/arm/configs/socfpga_defconfig |   4 +-
 arch/arm/configs/stm32_defconfig   |   4 +-
 arch/arm/configs/sunxi_defconfig   |   4 +-
 arch/arm/configs/tango4_defconfig  |   4 +-
 arch/arm/configs/tegra_defconfig   |   4 +-
 arch/arm/configs/u300_defconfig|   4 +-
 arch/arm/configs/u8500_defconfig   |   4 +-
 arch/arm/configs/vt8500_v6_v7_defconfig|   4 +-
 arch/arm/configs/xcep_defconfig|   4 +-
 arch/arm/configs/zx_defconfig  |   4 

Re: [PATCH v2] printk: Add boottime and real timestamps

2017-08-01 Thread Prarit Bhargava


On 08/01/2017 01:00 PM, John Stultz wrote:
> On Tue, Aug 1, 2017 at 5:55 AM, Prarit Bhargava <pra...@redhat.com> wrote:
>> printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
>> timestamp to printk messages.  The local hardware clock loses time each
>> day making it difficult to determine exactly when an issue has occurred in
>> the kernel log, and making it difficult to determine how kernel and
>> hardware issues relate to each other in real time.
>>
>> Make printk output different timestampes by adding options for no
>> timestamp, the local hardware clock, the monotonic clock, and the real
>> clock.  Allow a user to pick one of the clocks by using the printk.time
>> kernel parameter.  Output the type of clock in
>> /sys/module/printk/parameters/time so userspace programs can interpret the
>> timestamp.
>>
>> Real clock & 32-bit systems:  Selecting the real clock printk timestamp may
>> lead to unlikely situations where a timestamp is wrong because the real time
>> offset is read without the protection of a sequence lock in the call to
>> ktime_get_log_ts() in printk_get_ts().
>>
>> v2: Use peterz's suggested Kconfig options.  Merge patchset together.  Fix
>> i386 !CONFIG_PRINTK builds.
>>
>> Signed-off-by: Prarit Bhargava <pra...@redhat.com>
> ...
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 98fe715522e8..7a8870b4ddbb 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -1,8 +1,39 @@
>>  menu "printk and dmesg options"
>>
>> +choice
>> +   prompt "printk default clock"
>> +   config PRINTK_TIME_DISABLE
>> +   bool "Disabled"
>> +   help
>> +Selecting this option disables the time stamps of printk().
>> +
>> +   config PRINTK_TIME_LOCAL
>> +   bool "Local Clock"
>> +   help
>> + Selecting this option causes the time stamps of printk() to be
>> + stamped with the unadjusted hardware clock.
>> +
>> +   config PRINTK_TIME_MONO
>> +   bool "CLOCK_MONOTONIC"
>> +   help
>> + Selecting this option causes the time stamps of printk() to be
>> + stamped with the adjusted monotonic clock.
>> +
>> +   config PRINTK_TIME_REAL
>> +   bool "CLOCK_REALTIME"
>> +   help
>> + Selecting this option causes the time stamps of printk() to be
>> + stamped with the adjusted realtime clock.
> 
> Its been asked already, but I've not yet seen an answer.

Sorry for missing this.

> Is there a reason your not also adding PRINTK_TIME_BOOT here (which to
> me would be more generally useful then REAL or MONO)?

REAL has been useful to me in debug cases where events on the system were timed
to the wall clock (ex cron job running at 3AM).  I hadn't really thought much
about using BOOT TBH because MONO seemed to work just fine.

Mark Salyzyn, did you want BOOT or MONO?

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] printk: Add boottime and real timestamps

2017-08-01 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Make printk output different timestampes by adding options for no
timestamp, the local hardware clock, the monotonic clock, and the real
clock.  Allow a user to pick one of the clocks by using the printk.time
kernel parameter.  Output the type of clock in
/sys/module/printk/parameters/time so userspace programs can interpret the
timestamp.

Real clock & 32-bit systems:  Selecting the real clock printk timestamp may
lead to unlikely situations where a timestamp is wrong because the real time
offset is read without the protection of a sequence lock in the call to
ktime_get_log_ts() in printk_get_ts().

v2: Use peterz's suggested Kconfig options.  Merge patchset together.  Fix
i386 !CONFIG_PRINTK builds.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Prarit Bhargava <pra...@redhat.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org
---
 Documentation/admin-guide/kernel-parameters.txt|  6 +-
 arch/arm/configs/aspeed_g4_defconfig   |  4 +-
 arch/arm/configs/aspeed_g5_defconfig   |  4 +-
 arch/arm/configs/axm55xx_defconfig |  4 +-
 arch/arm/configs/bcm2835_defconfig |  4 +-
 arch/arm/configs/colibri_pxa270_defconfig  |  4 +-
 arch/arm/configs/colibri_pxa300_defconfig  |  4 +-
 arch/arm/configs/dove_defconfig|  4 +-
 arch/arm/configs/efm32_defconfig   |  4 +-
 arch/arm/configs/exynos_defconfig  |  4 +-
 arch/arm/configs/ezx_defconfig |  4 +-
 arch/arm/configs/h5000_defconfig   |  4 +-
 arch/arm/configs/hisi_defconfig|  4 +-
 arch/arm/configs/imote2_defconfig  |  4 +-
 arch/arm/configs/imx_v6_v7_defconfig   |  4 +-
 arch/arm/configs/keystone_defconfig|  4 +-
 arch/arm/configs/lpc18xx_defconfig |  4 +-
 arch/arm/configs/magician_defconfig|  4 +-
 arch/arm/configs/mmp2_defconfig|  4 +-
 arch/arm/configs/moxart_defconfig  |  4 +-
 arch/arm/configs/mps2_defconfig|  4 +-
 arch/arm/configs/multi_v7_defconfig|  4 +-
 arch/arm/configs/mvebu_v7_defconfig|  4 +-
 arch/arm/configs/mxs_defconfig |  4 +-
 arch/arm/configs/omap2plus_defconfig   |  4 +-
 arch/arm/configs/pxa168_defconfig  |  4 +-
 arch/arm/configs/pxa3xx_defconfig  |  4 +-
 arch/arm/configs/pxa910_defconfig  |  4 +-
 arch/arm/configs/pxa_defconfig |  4 +-
 arch/arm/configs/qcom_defconfig|  4 +-
 arch/arm/configs/raumfeld_defconfig|  4 +-
 arch/arm/configs/shmobile_defconfig|  4 +-
 arch/arm/configs/socfpga_defconfig |  4 +-
 arch/arm/configs/stm32_defconfig   |  4 +-
 arch/arm/configs/sunxi_defconfig   |  4 +-
 arch/arm/configs/tango4_defconfig  |  4 +-
 arch/arm/configs/tegra_defconfig   |  4 +-
 arch/arm/configs/u300_defconfig|  4 +-
 arch/arm/configs/u8500_defconfig   |  4 +-
 arch/arm/configs/vt8500_v6_v7_defconfig|  4 +-
 arch/arm/configs/xcep_defconfig|  4 +-
 arch/arm/configs/zx_defconfig  |  4 +-
 arch/arm64/configs/defconfig   |  4 +-
 arch/m68k/configs/amcore_defconfig |  4 +-
 arch/mips/con

Re: [PATCH 1/2] printk: Make CONFIG_PRINTK_TIME an int

2017-07-31 Thread Prarit Bhargava


On 07/25/2017 08:55 AM, Luis R. Rodriguez wrote:
> On Tue, Jul 25, 2017 at 08:17:26AM -0400, Prarit Bhargava wrote:
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index fc47863f629c..26cf6cadd267 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -1202,8 +1202,40 @@ static inline void boot_delay_msec(int level)
>>  }
>>  #endif
>>  
>> -static bool printk_time = IS_ENABLED(CONFIG_PRINTK_TIME);
>> -module_param_named(time, printk_time, bool, S_IRUGO | S_IWUSR);
>> +static int printk_time = CONFIG_PRINTK_TIME;
> 
> You could just use unsigned int but is the reason you went with int to
> enable backward compatibility with the old bool =y or =n?

Yes, I wanted to preserve backwards compatibility.

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] printk: Add boottime and real timestamps

2017-07-28 Thread Prarit Bhargava


On 07/25/2017 09:00 AM, Peter Zijlstra wrote:
> On Tue, Jul 25, 2017 at 08:17:27AM -0400, Prarit Bhargava wrote:
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 5b1662ec546f..6cd38a25f8ea 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -1,8 +1,8 @@
>>  menu "printk and dmesg options"
>>  
>>  config PRINTK_TIME
>> -int "Show timing information on printks (0-1)"
>> -range 0 1
>> +int "Show timing information on printks (0-3)"
>> +range 0 3
>>  default "0"
>>  depends on PRINTK
>>  help
>> @@ -13,7 +13,8 @@ config PRINTK_TIME
>>The timestamp is always recorded internally, and exported
>>to /dev/kmsg. This flag just specifies if the timestamp should
>>be included, not that the timestamp is recorded. 0 disables the
>> -  timestamp and 1 uses the local clock.
>> +  timestamp and 1 uses the local clock, 2 uses the monotonic clock, and
>> +  3 uses real clock.
>>  
>>The behavior is also controlled by the kernel command line
>>parameter printk.time=1. See 
>> Documentation/admin-guide/kernel-parameters.rst
> 
> 
> choice
>   prompt "printk default clock"
>   default PRIMTK_TIME_DISABLE
>   help
>goes here
> 
>   config PRINTK_TIME_DISABLE
>   bool "Disabled"
>   help
>goes here
> 
>   config PRINTK_TIME_LOCAL
>   bool "local clock"
>   help
>goes here
> 
>   config PRINTK_TIME_MONO
>   bool "CLOCK_MONOTONIC"
>   help
>goes here
> 
>   config PRINTK_TIME_REAL
>   bool "CLOCK_REALTIME"
>   help
>goes here
> 
> endchoice
> 
> config PRINTK_TIME
>   int
>   default 0 if PRINTK_TIME_DISABLE
>   default 1 if PRINTK_TIME_LOCAL
>   default 2 if PRINTK_TIME_MONO
>   default 3 if PRINTK_TIME_REAL
> 
> 

Thanks for the above change.  I can see that makes the code simpler.

> Although I must strongly discourage using REALTIME, DST will make
> untangling your logs an absolute nightmare. I would simply not provide
> it.

I understand your concern, however, I've been in situations where REALTIME
stamping has pointed me in the direction of where a bug was.  Even with the
complicated logs I think it is worthwhile.

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] printk: Add boottime and real timestamps

2017-07-25 Thread Prarit Bhargava
printk.time=1/CONFIG_PRINTK_TIME=Y timestamps printks with an unmodified
hardware clock timestamp.  This clock loses time each day making it
difficult to determine when an issue has occurred in the kernel log.

Modify printk.time to output local, monotonic, or a real timestamp.
Modify the output of /sys/module/printk/parameters/time to output the type
of clock so userspace programs can interpret the timestamp.

Real clock & 32-bit systems:  Selecting the real clock printk timestamp
may lead to unlikely situations where a timestamp is wrong because the
real time offset is read without the protection of a sequence lock in the
call to ktime_get_log_ts() in printk_get_ts().

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: "Theodore Ts'o" <ty...@mit.edu>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org


---
 Documentation/admin-guide/kernel-parameters.txt |  6 +-
 include/linux/timekeeping.h |  1 +
 kernel/printk/printk.c  | 77 +
 kernel/time/timekeeping.c   | 14 +
 lib/Kconfig.debug   |  7 ++-
 5 files changed, 89 insertions(+), 16 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index c3b14abf9da4..c03240d057b1 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3188,8 +3188,10 @@
ratelimit - ratelimit the logging
Default: ratelimit
 
-   printk.time=Show timing data prefixed to each printk message line
-   Format:   (1/Y/y=enable, 0/N/n=disable)
+   printk.time=Show timestamp prefixed to each printk message line
+   Format: 
+   (0/N/n = disable, 1/Y/y = local/unadjusted HW,
+2 = monotonic, 3 = real)
 
processor.max_cstate=   [HW,ACPI]
Limit processor to maximum C-state
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index ddc229ff6d1e..adb84af42deb 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -239,6 +239,7 @@ static inline u64 ktime_get_raw_ns(void)
 extern u64 ktime_get_mono_fast_ns(void);
 extern u64 ktime_get_raw_fast_ns(void);
 extern u64 ktime_get_boot_fast_ns(void);
+extern u64 ktime_get_log_ts(u64 *offset_real);
 
 /*
  * Timespec interfaces utilizing the ktime based ones
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 26cf6cadd267..35536369a56d 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -576,6 +576,8 @@ static u32 truncate_msg(u16 *text_len, u16 *trunc_msg_len,
return msg_used_size(*text_len + *trunc_msg_len, 0, pad_len);
 }
 
+static u64 printk_get_ts(void);
+
 /* insert record into the buffer, discard old ones, update heads */
 static int log_store(int facility, int level,
 enum log_flags flags, u64 ts_nsec,
@@ -624,7 +626,7 @@ static int log_store(int facility, int level,
if (ts_nsec > 0)
msg->ts_nsec = ts_nsec;
else
-   msg->ts_nsec = local_clock();
+   msg->ts_nsec = printk_get_ts();
memset(log_dict(msg) + dict_len, 0, pad_len);
msg->len = size;
 
@@ -1203,26 +1205,60 @@ static inline void boot_delay_msec(int level)
 #endif
 
 static int printk_time = CONFIG_PRINTK_TIME;
+static int printk_time_setting; /* initial setting */
 
+/*
+ * Real clock & 32-bit systems:  Selecting the real clock printk timestamp may
+ * lead to unlikely situations where a timestamp is wrong because the real time
+ * offset is read without the protection of a sequence lock in the call to
+ * ktime_get_log_ts() 

[PATCH 0/2] printk: allow different timestamps for printk.time

2017-07-25 Thread Prarit Bhargava
Over the past years I've seen many reports of bugs that include
time-stamped kernel logs (enabled when CONFIG_PRINTK_TIME=y or
print.time=1 is specified as a kernel parameter) that do not align
with either external time stamped logs or /var/log/messages.  This
also makes determining the time of a failure difficult in cases where
/var/log/messages is unavailable.

For example,

[root@intel-wildcatpass-06 ~]# date; echo "Hello!" > /dev/kmsg ; date
Thu Jul 20 11:38:22 EST 2017
Thu Jul 20 11:38:22 EST 2017

which displays

[83973.768912] Hello!

on the serial console.

Running a script to convert this to the stamped time,

[root@intel-wildcatpass-06 ~]# ./human.sh  | tail -1
[Thu July 17 11:39:45 2017] Hello!

which is already off by 1 minute and 23 seconds off after ~24 hours of
uptime.

This occurs because the printk time stamp is obtained from a call to
local_clock() which (on x86) is a direct call to the hardware.  These
hardware clock reads are not modified by the standard ntp or ptp protocol
The other timestamps are and that results in situations external
time sources are further and further offset from the kernel log
timestamps.

Implement printk.time settings to allow a user to specify the monotonic
or real clocks.  The default is the local clock (hardware clock).

Real clock & 32-bit systems:  Selecting the real clock printk timestamp may
lead to unlikely situations where a timestamp is wrong because the real time
offset is read without the protection of a sequence lock in the call to
ktime_get_log_ts() in printk_get_ts().

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: "Theodore Ts'o" <ty...@mit.edu>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org

Prarit Bhargava (2):
  printk: Make CONFIG_PRINTK_TIME an int
  printk: Add boottime and real timestamps

 Documentation/admin-guide/kernel-parameters.txt|  6 +-
 arch/arm/configs/aspeed_g4_defconfig   |  2 +-
 arch/arm/configs/aspeed_g5_defconfig   |  2 +-
 arch/arm/configs/axm55xx_defconfig |  2 +-
 arch/arm/configs/bcm2835_defconfig |  2 +-
 arch/arm/configs/colibri_pxa270_defconfig  |  2 +-
 arch/arm/configs/colibri_pxa300_defconfig  |  2 +-
 arch/arm/configs/dove_defconfig|  2 +-
 arch/arm/configs/efm32_defconfig   |  2 +-
 arch/arm/configs/exynos_defconfig  |  2 +-
 arch/arm/configs/ezx_defconfig |  2 +-
 arch/arm/configs/h5000_defconfig   |  2 +-
 arch/arm/configs/hisi_defconfig|  2 +-
 arch/arm/configs/imote2_defconfig  |  2 +-
 arch/arm/configs/imx_v6_v7_defconfig   |  2 +-
 arch/arm/configs/keystone_defconfig|  2 +-
 arch/arm/configs/lpc18xx_defconfig |  2 +-
 arch/arm/configs/magician_defconfig|  2 +-
 arch/arm/configs/mmp2_defconfig|  2 +-
 arch/arm/configs/moxart_defconfig  |  2 +-
 arch/arm/configs/mps2_defconfig|  2 +-
 arch/arm/configs/multi_v7_defconfig|  2 +-
 arch/arm/configs/mvebu_v7_defconfig|  2 +-
 arch/arm/configs/mxs_defconfig |  2 +-
 arch/arm/configs/omap2plus_defconfig   |  2 +-
 arch/arm/configs/pxa168_defconfig  |  2 +-
 arch/arm/configs/pxa3xx_defconfig  |  2 +-
 arch/arm/configs/pxa910_defconfig  |  2 +-
 arch/arm/configs/pxa_defconfig |  2 +-
 arch/arm/configs/qcom_defconfig|  2 +-
 arch/arm/configs/raumfeld_defconfig|  2 +-
 arch/arm/configs/shmobile_defconfig|  2 +-
 arch/arm/configs/socfpga_defconfig |  2 +-
 arch/arm/configs/stm32_defconfig 

[PATCH 1/2] printk: Make CONFIG_PRINTK_TIME an int

2017-07-25 Thread Prarit Bhargava
CONFIG_PRINTK_TIME is a bool and in order to add timestamp options for
the monotonic and real time clock it must be expanded to an int.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Mark Salyzyn <saly...@android.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: John Stultz <john.stu...@linaro.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
Cc: Christoffer Dall <cd...@linaro.org>
Cc: Deepa Dinamani <deepa.ker...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Joel Fernandes <joe...@google.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: "Luis R. Rodriguez" <mcg...@kernel.org>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: "Jason A. Donenfeld" <ja...@zx2c4.com>
Cc: Olof Johansson <o...@lixom.net>
Cc: "Theodore Ts'o" <ty...@mit.edu>
Cc: Josh Poimboeuf <jpoim...@redhat.com>
Cc: linux-doc@vger.kernel.org


---
 Documentation/admin-guide/kernel-parameters.txt|  2 +-
 arch/arm/configs/aspeed_g4_defconfig   |  2 +-
 arch/arm/configs/aspeed_g5_defconfig   |  2 +-
 arch/arm/configs/axm55xx_defconfig |  2 +-
 arch/arm/configs/bcm2835_defconfig |  2 +-
 arch/arm/configs/colibri_pxa270_defconfig  |  2 +-
 arch/arm/configs/colibri_pxa300_defconfig  |  2 +-
 arch/arm/configs/dove_defconfig|  2 +-
 arch/arm/configs/efm32_defconfig   |  2 +-
 arch/arm/configs/exynos_defconfig  |  2 +-
 arch/arm/configs/ezx_defconfig |  2 +-
 arch/arm/configs/h5000_defconfig   |  2 +-
 arch/arm/configs/hisi_defconfig|  2 +-
 arch/arm/configs/imote2_defconfig  |  2 +-
 arch/arm/configs/imx_v6_v7_defconfig   |  2 +-
 arch/arm/configs/keystone_defconfig|  2 +-
 arch/arm/configs/lpc18xx_defconfig |  2 +-
 arch/arm/configs/magician_defconfig|  2 +-
 arch/arm/configs/mmp2_defconfig|  2 +-
 arch/arm/configs/moxart_defconfig  |  2 +-
 arch/arm/configs/mps2_defconfig|  2 +-
 arch/arm/configs/multi_v7_defconfig|  2 +-
 arch/arm/configs/mvebu_v7_defconfig|  2 +-
 arch/arm/configs/mxs_defconfig |  2 +-
 arch/arm/configs/omap2plus_defconfig   |  2 +-
 arch/arm/configs/pxa168_defconfig  |  2 +-
 arch/arm/configs/pxa3xx_defconfig  |  2 +-
 arch/arm/configs/pxa910_defconfig  |  2 +-
 arch/arm/configs/pxa_defconfig |  2 +-
 arch/arm/configs/qcom_defconfig|  2 +-
 arch/arm/configs/raumfeld_defconfig|  2 +-
 arch/arm/configs/shmobile_defconfig|  2 +-
 arch/arm/configs/socfpga_defconfig |  2 +-
 arch/arm/configs/stm32_defconfig   |  2 +-
 arch/arm/configs/sunxi_defconfig   |  2 +-
 arch/arm/configs/tango4_defconfig  |  2 +-
 arch/arm/configs/tegra_defconfig   |  2 +-
 arch/arm/configs/u300_defconfig|  2 +-
 arch/arm/configs/u8500_defconfig   |  2 +-
 arch/arm/configs/vt8500_v6_v7_defconfig|  2 +-
 arch/arm/configs/xcep_defconfig|  2 +-
 arch/arm/configs/zx_defconfig  |  2 +-
 arch/arm64/configs/defconfig   |  2 +-
 arch/m68k/configs/amcore_defconfig |  2 +-
 arch/mips/configs/ath25_defconfig  |  2 +-
 arch/mips/configs/bcm47xx_defconfig|  2 +-
 arch/mips/configs/bmips_be_defconfig   |  2 +-
 arch/mips/configs/bmips_stb_defconfig  |  2 +-
 arch/mips/configs/ci20_defconfig   |  2 +-
 arch/mips/configs/generic_defconfig|  2 +-
 arch/mips/configs/lemote2f_defconfig   |  2 +-
 arch/mips/configs/loongson3_defconfig  |  2 +-
 arch/mips/configs/nlm_xlp_defconfig|  2 +-
 arch/mips/configs/nlm_xlr_defconfig|  2 +-
 arch/mips/configs/pistachio_defconfig  |  2 +-
 arch/mips/configs/qi_lb60_defconfig|  2 +-
 arch/mips/configs/rt305x_defconfig |  2 +-
 arch/mips/configs/xway_defconfig   |  2 +-
 arch/parisc/configs/generic-64bit_defconfig|  2 +-
 arch/powerpc/configs/40x/virt

[PATCH v5] Add kernel parameter to blacklist modules

2016-07-19 Thread Prarit Bhargava
Rusty, this looks like it will work.  I tested this by adding

module_blacklist=acpi_cpufreq,dummy_module

as a kernel parameter, and verified that acpi_cpufreq was not loaded
during the initrd and systemd phases of the boot.  I then tried to
manually load a simple module called dummy_module.ko which fails with

[root@dell-per330-01 ~]# insmod dummy-module.ko
insmod: ERROR: could not insert module dummy-module.ko: Operation not
permitted

P.

---8<

Blacklisting a module in linux has long been a problem.  The current
procedure is to use rd.blacklist=module_name, however, that doesn't
cover the case after the initramfs and before a boot prompt (where one
is supposed to use /etc/modprobe.d/blacklist.conf to blacklist
runtime loading). Using rd.shell to get an early prompt is hit-or-miss,
and doesn't cover all situations AFAICT.

This patch adds this functionality of permanently blacklisting a module
by its name via the kernel parameter module_blacklist=module_name.

[v2]: Rusty, use core_param() instead of __setup() which simplifies
things.

[v3]: Rusty, undo wreckage from strsep()

[v4]: Rusty, simpler version of blacklisted()

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Rusty Russell <ru...@rustcorp.com.au>
Cc: linux-doc@vger.kernel.org
---
 Documentation/kernel-parameters.txt |3 +++
 kernel/module.c |   24 
 2 files changed, 27 insertions(+)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 82b42c958d1c..c720b96f2efc 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2295,6 +2295,9 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
Note that if CONFIG_MODULE_SIG_FORCE is set, that
is always true, so this option does nothing.
 
+   module_blacklist=  [KNL] Do not load a comma-separated list of
+   modules.  Useful for debugging problem modules.
+
mousedev.tap_time=
[MOUSE] Maximum time between finger touching and
leaving touchpad surface for touch to be considered
diff --git a/kernel/module.c b/kernel/module.c
index 5f71aa63ed2a..33b85824d30b 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3155,6 +3155,27 @@ int __weak module_frob_arch_sections(Elf_Ehdr *hdr,
return 0;
 }
 
+/* module_blacklist is a comma-separated list of module names */
+static char *module_blacklist;
+static bool blacklisted(char *module_name)
+{
+   const char *p;
+   size_t len;
+
+   if (!module_blacklist)
+   return false;
+
+   for (p = module_blacklist; *p; p += len) {
+   len = strcspn(p, ",");
+   if (strlen(module_name) == len && !memcmp(module_name, p, len))
+   return true;
+   if (p[len] == ',')
+   len++;
+   }
+   return false;
+}
+core_param(module_blacklist, module_blacklist, charp, 0400);
+
 static struct module *layout_and_allocate(struct load_info *info, int flags)
 {
/* Module within temporary copy. */
@@ -3165,6 +3186,9 @@ static struct module *layout_and_allocate(struct 
load_info *info, int flags)
if (IS_ERR(mod))
return mod;
 
+   if (blacklisted(mod->name))
+   return ERR_PTR(-EPERM);
+
err = check_modinfo(mod, info, flags);
if (err)
return ERR_PTR(err);
-- 
1.7.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3] Add kernel parameter to blacklist modules

2016-07-06 Thread Prarit Bhargava
Rusty?  Didn't see anything from you on this ... resending.

P.


8<

Blacklisting a module in linux has long been a problem.  The current
procedure is to use rd.blacklist=module_name, however, that doesn't
cover the case after the initramfs and before a boot prompt (where one
is supposed to use /etc/modprobe.d/blacklist.conf to blacklist
runtime loading). Using rd.shell to get an early prompt is hit-or-miss,
and doesn't cover all situations AFAICT.

This patch adds this functionality of permanently blacklisting a module
by its name via the kernel parameter module_blacklist=module_name.

[v2]: Rusty, use core_param() instead of __setup() which simplifies
things.

[v3]: Rusty, undo wreckage from strsep()

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Rusty Russell <ru...@rustcorp.com.au>
Cc: linux-doc@vger.kernel.org
---
 Documentation/kernel-parameters.txt |3 +++
 kernel/module.c |   29 +
 2 files changed, 32 insertions(+)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 82b42c958d1c..c720b96f2efc 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2295,6 +2295,9 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
Note that if CONFIG_MODULE_SIG_FORCE is set, that
is always true, so this option does nothing.
 
+   module_blacklist=  [KNL] Do not load a comma-separated list of
+   modules.  Useful for debugging problem modules.
+
mousedev.tap_time=
[MOUSE] Maximum time between finger touching and
leaving touchpad surface for touch to be considered
diff --git a/kernel/module.c b/kernel/module.c
index 5f71aa63ed2a..5240da88af79 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3155,6 +3155,32 @@ int __weak module_frob_arch_sections(Elf_Ehdr *hdr,
return 0;
 }
 
+/* module_blacklist is a comma-separated list of module names */
+static char *module_blacklist;
+static bool blacklisted(char *module_name)
+{
+   char *str, *entry;
+
+   if (!module_blacklist)
+   return false;
+
+   str = module_blacklist;
+   do {
+   if (str != module_blacklist)
+   module_blacklist[strlen(str) - 1] = ',';
+   entry = strsep(, ",");
+   if (!strcmp(module_name, entry)) {
+   pr_info("module %s is blacklisted\n", module_name);
+   if (str != module_blacklist)
+   module_blacklist[strlen(str) - 1] = ',';
+   return true;
+   }
+   } while (str);
+
+   return false;
+}
+core_param(module_blacklist, module_blacklist, charp, 0400);
+
 static struct module *layout_and_allocate(struct load_info *info, int flags)
 {
/* Module within temporary copy. */
@@ -3165,6 +3191,9 @@ static struct module *layout_and_allocate(struct 
load_info *info, int flags)
if (IS_ERR(mod))
return mod;
 
+   if (blacklisted(mod->name))
+   return ERR_PTR(-EPERM);
+
err = check_modinfo(mod, info, flags);
if (err)
return ERR_PTR(err);
-- 
1.7.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3] Add kernel parameter to blacklist modules

2016-06-23 Thread Prarit Bhargava
Blacklisting a module in linux has long been a problem.  The current
procedure is to use rd.blacklist=module_name, however, that doesn't
cover the case after the initramfs and before a boot prompt (where one
is supposed to use /etc/modprobe.d/blacklist.conf to blacklist
runtime loading). Using rd.shell to get an early prompt is hit-or-miss,
and doesn't cover all situations AFAICT.

This patch adds this functionality of permanently blacklisting a module
by its name via the kernel parameter module_blacklist=module_name.

[v2]: Rusty, use core_param() instead of __setup() which simplifies
things.

[v3]: Rusty, undo wreckage from strsep()

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Rusty Russell <ru...@rustcorp.com.au>
Cc: linux-doc@vger.kernel.org
---
 Documentation/kernel-parameters.txt |3 +++
 kernel/module.c |   29 +
 2 files changed, 32 insertions(+)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 82b42c958d1c..c720b96f2efc 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2295,6 +2295,9 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
Note that if CONFIG_MODULE_SIG_FORCE is set, that
is always true, so this option does nothing.
 
+   module_blacklist=  [KNL] Do not load a comma-separated list of
+   modules.  Useful for debugging problem modules.
+
mousedev.tap_time=
[MOUSE] Maximum time between finger touching and
leaving touchpad surface for touch to be considered
diff --git a/kernel/module.c b/kernel/module.c
index 5f71aa63ed2a..5240da88af79 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3155,6 +3155,32 @@ int __weak module_frob_arch_sections(Elf_Ehdr *hdr,
return 0;
 }
 
+/* module_blacklist is a comma-separated list of module names */
+static char *module_blacklist;
+static bool blacklisted(char *module_name)
+{
+   char *str, *entry;
+
+   if (!module_blacklist)
+   return false;
+
+   str = module_blacklist;
+   do {
+   if (str != module_blacklist)
+   module_blacklist[strlen(str) - 1] = ',';
+   entry = strsep(, ",");
+   if (!strcmp(module_name, entry)) {
+   pr_info("module %s is blacklisted\n", module_name);
+   if (str != module_blacklist)
+   module_blacklist[strlen(str) - 1] = ',';
+   return true;
+   }
+   } while (str);
+
+   return false;
+}
+core_param(module_blacklist, module_blacklist, charp, 0400);
+
 static struct module *layout_and_allocate(struct load_info *info, int flags)
 {
/* Module within temporary copy. */
@@ -3165,6 +3191,9 @@ static struct module *layout_and_allocate(struct 
load_info *info, int flags)
if (IS_ERR(mod))
return mod;
 
+   if (blacklisted(mod->name))
+   return ERR_PTR(-EPERM);
+
err = check_modinfo(mod, info, flags);
if (err)
return ERR_PTR(err);
-- 
1.7.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] Add kernel parameter to blacklist modules

2016-06-15 Thread Prarit Bhargava


On 06/14/2016 05:20 PM, Rusty Russell wrote:
> Prarit Bhargava <pra...@redhat.com> writes:
>> Blacklisting a module in linux has long been a problem.  The current
>> procedure is to use rd.blacklist=module_name, however, that doesn't
>> cover the case after the initramfs and before a boot prompt (where one
>> is supposed to use /etc/modprobe.d/blacklist.conf to blacklist
>> runtime loading). Using rd.shell to get an early prompt is hit-or-miss,
>> and doesn't cover all situations AFAICT.
>>
>> This patch adds this functionality of permanently blacklisting a module
>> by its name via the kernel parameter module_blacklist=module_name.
>>
>> [v2]: Rusty, use core_param() instead of __setup(), and drop struct which
>> simplifies things.
>>
>> Signed-off-by: Prarit Bhargava <pra...@redhat.com>
>> Cc: Jonathan Corbet <cor...@lwn.net>
>> Cc: Rusty Russell <ru...@rustcorp.com.au>
>> Cc: linux-doc@vger.kernel.org
>> ---
>>  Documentation/kernel-parameters.txt |3 +++
>>  kernel/module.c |   25 +
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/Documentation/kernel-parameters.txt 
>> b/Documentation/kernel-parameters.txt
>> index 82b42c958d1c..c720b96f2efc 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -2295,6 +2295,9 @@ bytes respectively. Such letter suffixes can also be 
>> entirely omitted.
>>  Note that if CONFIG_MODULE_SIG_FORCE is set, that
>>  is always true, so this option does nothing.
>>  
>> +module_blacklist=  [KNL] Do not load a comma-separated list of
>> +modules.  Useful for debugging problem modules.
>> +
>>  mousedev.tap_time=
>>  [MOUSE] Maximum time between finger touching and
>>  leaving touchpad surface for touch to be considered
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 5f71aa63ed2a..5ff5287b19a8 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -3155,6 +3155,28 @@ int __weak module_frob_arch_sections(Elf_Ehdr *hdr,
>>  return 0;
>>  }
>>  
>> +/* module_blacklist is a comma-separated list of module names */
>> +static char *module_blacklist;
>> +static bool blacklisted(char *module_name)
>> +{
>> +char *str, *entry;
>> +
>> +if (!module_blacklist)
>> +return false;
>> +
>> +str = module_blacklist;
>> +do {
>> +entry = strsep(, ",");
>> +if (!strcmp(module_name, entry)) {
>> +pr_info("module %s is blacklisted\n", module_name);
>> +return true;
>> +}
> 
> strsep mangles the string; this will only work once :)
> 
> This is untested, and a little ugly:
> 
>len = strlen(module_name);
>
>while ((p = strstr(p, module_name)) != NULL) {
>   if ((p == module_blacklist || p[-1] == ',') &&
>   (p[len] == ',' || p[len] == '\0'))
> return true;
>   p += len;
>}
>return false;

Hmm ... yeah, a bit ugly.  I could also easily do:

str = module_blacklist;
do {
if (str != module_blacklist)
module_blacklist[strlen(str) - 1] = ',';
entry = strsep(, ",");
if (!strcmp(module_name, entry)) {
pr_info("module %s is blacklisted\n", module_name);
if (str != module_blacklist)
module_blacklist[strlen(str) - 1] = ',';
return true;
}
} while (str);

which results in the correct behavior AFAICT with
"module_blacklist=dummy_module,prarit_module":

On boot:

[   23.737613] blacklist: comparing |dns_resolver| to |dummy_module|
[   23.743713] blacklist: comparing |dns_resolver| to |prarit_module|

when attempting to load dummy_module:

[   43.938798] blacklist: comparing |dummy_module| to |dummy_module|
[   43.944915] module dummy_module is blacklisted

and attempting to load another module after that (in this case ixgbe):

[   47.572557] blacklist: comparing |mdio| to |dummy_module|
[   47.577961] blacklist: comparing |mdio| to |prarit_module|
[   47.684713] blacklist: comparing |ixgbe| to |dummy_module|
[   47.690202] blacklist: comparing |ixgbe| to |prarit_module|

Any objection to that version?  Admittedly yours is shorter but I feel like mine
might be "easier" to read...

P.

> 
> Cheers,
> Rusty.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add kernel parameter to blacklist modules

2016-06-14 Thread Prarit Bhargava


On 06/14/2016 01:17 PM, Christoph Hellwig wrote:
> On Mon, Jun 13, 2016 at 08:32:41AM -0400, Prarit Bhargava wrote:
>> Blacklisting a module in linux has long been a problem.  The process of
>> blacklisting a module has changed over time, and it seems that every OS
>> does it slightly differently and depends on the age of the init system
>> used on that OS.
> 
> And why would we care about blacklisting a module?


I have a new system that I want to install that a customer could not install any
Linux OS on.  It's sitting on my desk right now because I finally had to get
them to send me the system.  And I *cannot* get the bloody thing to install
anything recent because the damned nouveau driver keeps blowing up in the latest
Fedora rawhide (which is pretty close to upstream), or install anyting older
because the storage isn't detected on older releases.  I have *no* way of
stopping the driver from loading so that I can at least start debugging, or
provide valuable debug information in a kernel.org bugzilla (or appropriate
mailing list).

So what I'm stuck with now is a very expensive (albeit pretty) paperweight.


Sorry about the rant ... but I had to let it all out Christoph ... :)

Now, I am smart enough to install to a USB stick, remove the nouveau driver from
both the install image and the initramfs, play around with the myriad and random
BIOS settings on this laptop (I will note that these are different for EVERY
piece of hardware) to figure out how to get the BIOS or EFI and Secure Boot
settings to detect the USB stick, cross my fingers and stand on one leg while I
pray to $deity that the system boots.

I do NOT expect random user to follow those steps in order to install Fedora or
another other upstream-following Linux OS.  They won't, and users turn to other
OSes that "just work".  That does nothing for the community.  It does nothing to
improve bug reporting.  It does nothing to improve the kernel.

I'd much rather tell those users to add "module_blacklist=nouveau" and send me
some debug information so I can find out what the issue might be, and find
hardware similar to theirs to help them out.  I still might have to tell that
user to send me their hardware but that's always a last resort.

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] Add kernel parameter to blacklist modules

2016-06-14 Thread Prarit Bhargava
Blacklisting a module in linux has long been a problem.  The current
procedure is to use rd.blacklist=module_name, however, that doesn't
cover the case after the initramfs and before a boot prompt (where one
is supposed to use /etc/modprobe.d/blacklist.conf to blacklist
runtime loading). Using rd.shell to get an early prompt is hit-or-miss,
and doesn't cover all situations AFAICT.

This patch adds this functionality of permanently blacklisting a module
by its name via the kernel parameter module_blacklist=module_name.

[v2]: Rusty, use core_param() instead of __setup(), and drop struct which
simplifies things.

Signed-off-by: Prarit Bhargava <pra...@redhat.com>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Rusty Russell <ru...@rustcorp.com.au>
Cc: linux-doc@vger.kernel.org
---
 Documentation/kernel-parameters.txt |3 +++
 kernel/module.c |   25 +
 2 files changed, 28 insertions(+)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 82b42c958d1c..c720b96f2efc 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2295,6 +2295,9 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
Note that if CONFIG_MODULE_SIG_FORCE is set, that
is always true, so this option does nothing.
 
+   module_blacklist=  [KNL] Do not load a comma-separated list of
+   modules.  Useful for debugging problem modules.
+
mousedev.tap_time=
[MOUSE] Maximum time between finger touching and
leaving touchpad surface for touch to be considered
diff --git a/kernel/module.c b/kernel/module.c
index 5f71aa63ed2a..5ff5287b19a8 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3155,6 +3155,28 @@ int __weak module_frob_arch_sections(Elf_Ehdr *hdr,
return 0;
 }
 
+/* module_blacklist is a comma-separated list of module names */
+static char *module_blacklist;
+static bool blacklisted(char *module_name)
+{
+   char *str, *entry;
+
+   if (!module_blacklist)
+   return false;
+
+   str = module_blacklist;
+   do {
+   entry = strsep(, ",");
+   if (!strcmp(module_name, entry)) {
+   pr_info("module %s is blacklisted\n", module_name);
+   return true;
+   }
+   } while (str);
+
+   return false;
+}
+core_param(module_blacklist, module_blacklist, charp, 0400);
+
 static struct module *layout_and_allocate(struct load_info *info, int flags)
 {
/* Module within temporary copy. */
@@ -3165,6 +3187,9 @@ static struct module *layout_and_allocate(struct 
load_info *info, int flags)
if (IS_ERR(mod))
return mod;
 
+   if (blacklisted(mod->name))
+   return ERR_PTR(-EPERM);
+
err = check_modinfo(mod, info, flags);
if (err)
return ERR_PTR(err);
-- 
1.7.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html