From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753443AbdHGMlR (ORCPT ); Mon, 7 Aug 2017 08:41:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33628 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752310AbdHGMlN (ORCPT ); Mon, 7 Aug 2017 08:41:13 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6CD075F7B4 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=prarit@redhat.com Subject: Re: [PATCH v3] printk: Add boottime and real timestamps To: Mark Salyzyn , linux-kernel@vger.kernel.org References: <1501809524-496-1-git-send-email-prarit@redhat.com> <3e0a0734-60ae-59b9-30bf-25cc966013da@android.com> 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 From: Prarit Bhargava Message-ID: <8aa07623-22ca-bf49-e81d-7cb9ec4e67fb@redhat.com> Date: Mon, 7 Aug 2017 08:41:08 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <3e0a0734-60ae-59b9-30bf-25cc966013da@android.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 07 Aug 2017 12:41:13 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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.