From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752756AbdHHXIJ (ORCPT ); Tue, 8 Aug 2017 19:08:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45594 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752513AbdHHXIE (ORCPT ); Tue, 8 Aug 2017 19:08:04 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com B04F913A43 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=prarit@redhat.com Subject: Re: [PATCH v4] printk: Add monotonic, boottime, and realtime timestamps To: Peter Zijlstra , "Paul E. McKenney" 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> From: Prarit Bhargava Cc: 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 Message-ID: <3a1d78e8-744f-8668-fd20-e56d125c5089@redhat.com> Date: Tue, 8 Aug 2017 19:08:00 -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: <20170808082803.opp33rd3g64pe7bw@hirez.programming.kicks-ass.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Tue, 08 Aug 2017 23:08:04 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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.