From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752768AbdHIR25 (ORCPT ); Wed, 9 Aug 2017 13:28:57 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:38826 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752718AbdHIR2z (ORCPT ); Wed, 9 Aug 2017 13:28:55 -0400 Date: Wed, 9 Aug 2017 10:28:48 -0700 From: "Paul E. McKenney" To: Prarit Bhargava Cc: Peter Zijlstra , John Stultz , lkml , Mark Salyzyn , Jonathan Corbet , Petr Mladek , Sergey Senozhatsky , Steven Rostedt , Thomas Gleixner , Stephen Boyd , Andrew Morton , Greg Kroah-Hartman , Christoffer Dall , Deepa Dinamani , Ingo Molnar , Joel Fernandes , Kees Cook , Geert Uytterhoeven , "Luis R. Rodriguez" , Nicholas Piggin , "Jason A. Donenfeld" , Olof Johansson , Josh Poimboeuf , linux-doc@vger.kernel.org Subject: Re: [PATCH v4] printk: Add monotonic, boottime, and realtime timestamps Reply-To: paulmck@linux.vnet.ibm.com References: <1502121162-27981-1-git-send-email-prarit@redhat.com> <9452a611-aefb-543e-e2f7-32301b29a2cc@redhat.com> <20170807203639.GY3730@linux.vnet.ibm.com> <20170808082803.opp33rd3g64pe7bw@hirez.programming.kicks-ass.net> <3a1d78e8-744f-8668-fd20-e56d125c5089@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3a1d78e8-744f-8668-fd20-e56d125c5089@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 x-cbid: 17080917-0044-0000-0000-0000037AA127 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007514; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000219; SDB=6.00899903; UDB=6.00450477; IPR=6.00680108; BA=6.00005520; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00016614; XFM=3.00000015; UTC=2017-08-09 17:28:53 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17080917-0045-0000-0000-000007A8B62D Message-Id: <20170809172848.GZ3730@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-08-09_05:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1706020000 definitions=main-1708090269 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Aug 08, 2017 at 07:08:00PM -0400, Prarit Bhargava wrote: > > > 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... It is someone else's turn to provide a suggestion. ;-) Thanx, Paul