From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752009AbdHGQ6f (ORCPT ); Mon, 7 Aug 2017 12:58:35 -0400 Received: from mail-pf0-f176.google.com ([209.85.192.176]:35113 "EHLO mail-pf0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751589AbdHGQ6d (ORCPT ); Mon, 7 Aug 2017 12:58:33 -0400 Subject: Re: [PATCH v4] printk: Add monotonic, boottime, and realtime timestamps To: Prarit Bhargava , linux-kernel@vger.kernel.org Cc: Jonathan Corbet , Petr Mladek , Sergey Senozhatsky , Steven Rostedt , John Stultz , Thomas Gleixner , Stephen Boyd , Andrew Morton , Greg Kroah-Hartman , "Paul E. McKenney" , Christoffer Dall , Deepa Dinamani , Ingo Molnar , Joel Fernandes , Kees Cook , Peter Zijlstra , Geert Uytterhoeven , "Luis R. Rodriguez" , Nicholas Piggin , "Jason A. Donenfeld" , Olof Johansson , Josh Poimboeuf , linux-doc@vger.kernel.org References: <1502121162-27981-1-git-send-email-prarit@redhat.com> From: Mark Salyzyn Message-ID: Date: Mon, 7 Aug 2017 09:58:31 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <1502121162-27981-1-git-send-email-prarit@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? > + > +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. -- Mark