Re: [PATCH v5] kernel/trace: Add DISALLOW_TRACE_PRINTK make option

From: Steven Rostedt <rostedt@goodmis.org>
To: Nicolas Boichat <drinkcat@chromium.org>
Cc: dianders@chromium.org, groeck@chromium.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	"Guilherme G. Piccoli" <gpiccoli@canonical.com>,
	Kars Mulder <kerneldev@karsmulder.nl>,
	Kees Cook <keescook@chromium.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	Tiezhu Yang <yangtiezhu@loongson.cn>,
	Will Deacon <will@kernel.org>, Yue Hu <huyue2@yulong.com>,
	linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5] kernel/trace: Add DISALLOW_TRACE_PRINTK make option
Date: Mon, 24 Aug 2020 09:30:20 -0400	[thread overview]
Message-ID: <20200824093020.073ae8d8@oasis.local.home> (raw)
In-Reply-To: <20200824105852.v5.1.I4feb11d34ce7a0dd5ee2c3327fb5a1a9a646be30@changeid>

On Mon, 24 Aug 2020 10:59:13 +0800
Nicolas Boichat <drinkcat@chromium.org> wrote:

> ---
> 
> Changes since v4:
>  - Turns this into a make option, instead of a config
>    option, as suggested by Steven Rostedt <rostedt@goodmis.org>.
> 
> Changes since v2/v3:
>  - Rebase only, v3 didn't exist as I just split out the other
>    necessary patches.
>  - Added patch 3/3 to fix atomisp_compat_css20.c
> 
> Changes since v1:
>  - Use static_assert instead of __static_assert (Jason Gunthorpe)
>  - Fix issues that can be detected by this patch (running some
>    randconfig in a loop, kernel test robot, or manual inspection),
>    by:
>    - Making some debug config options that use trace_printk depend
>      on the new config option.
>    - Adding 3 patches before this one.
> 
>  Makefile               | 14 ++++++++++++++
>  include/linux/kernel.h | 17 ++++++++++++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index c4470a4e131f2ce..fb8b0d7fb4b2df7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -100,6 +100,16 @@ endif
>  
>  export quiet Q KBUILD_VERBOSE
>  
> +# Disallow use of trace_printk. Can be used by production kernels.
> +ifeq ("$(origin DISALLOW_TRACE_PRINTK)", "command line")
> +  KBUILD_DISALLOW_TRACE_PRINTK = $(DISALLOW_TRACE_PRINTK)
> +endif
> +ifndef KBUILD_DISALLOW_TRACE_PRINTK
> +  KBUILD_DISALLOW_TRACE_PRINTK = 0
> +endif
> +
> +export KBUILD_DISALLOW_TRACE_PRINTK
> +
>  # Kbuild will save output files in the current working directory.
>  # This does not need to match to the root of the kernel source tree.
>  #
> @@ -838,6 +848,10 @@ KBUILD_AFLAGS	+= -gz=zlib
>  KBUILD_LDFLAGS	+= --compress-debug-sections=zlib
>  endif
>  
> +ifeq ($(KBUILD_DISALLOW_TRACE_PRINTK),1)
> +KBUILD_CFLAGS += -DDISALLOW_TRACE_PRINTK
> +endif
> +
>  KBUILD_CFLAGS += $(DEBUG_CFLAGS)
>  export DEBUG_CFLAGS
>  

There's one more thing we need to do, is if you build without this option
then build with it, you should trigger a full kernel rebuild.
Otherwise, if you build without the option, then build with it, and it
doesn't rebuild the tree, it wont catch anything.

-- Steve