Re: [PATCH v4 3/3] media: atomisp: Only use trace_printk if allowed

From: Steven Rostedt <rostedt@goodmis.org>
To: Nicolas Boichat <drinkcat@chromium.org>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	devel@driverdev.osuosl.org, lkml <linux-kernel@vger.kernel.org>,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Douglas Anderson <dianders@chromium.org>,
	Guenter Roeck <groeck@chromium.org>,
	Joe Perches <joe@perches.com>
Subject: Re: [PATCH v4 3/3] media: atomisp: Only use trace_printk if allowed
Date: Thu, 20 Aug 2020 23:01:05 -0400	[thread overview]
Message-ID: <20200820230105.1f9651b7@oasis.local.home> (raw)
In-Reply-To: <CANMq1KCpV+RKCqHsK-=Xeb+Vq28qdnBVF51rcga7m9-xjqNSCg@mail.gmail.com>

On Fri, 21 Aug 2020 10:39:02 +0800
Nicolas Boichat <drinkcat@chromium.org> wrote:

> I'm not sure how that helps? I mean, the use case you have in mind is
> somebody reusing a distro/random config and not being able to use
> trace_printk, right? If that config has CONFIG_DISABLE_TRACE_PRINTK=y,
> then the developer will still need to flip that back.
> 
> Note that the option I'm added has default=y (_allow_ trace_printk),
> so I don't think default y or default n really matters?

Ideally, the production system doesn't have it set. It only sets it to
make sure that it's clean before sending out. But then it can add it
back before production. Yeah, it's pretty much cutting hairs between
the two. I don't like either one.

Really, if you are worried about this, just add your patch to your
local tree. I'm not sure this is something that can be fixed upstream.

Another idea is to add something like below, and build with:

 make CHECK_TRACE_PRINT=1

This way it is a build command line option that causes the build to
fail if trace_printk() is added.

This way production systems can add this to make sure their kernels are
free of trace_printk() but it doesn't affect the config that is used.

-- Steve

[ Not even compiled tested! ]

diff --git a/Makefile b/Makefile
index 2057c92a6205..5714a738879d 100644
--- a/Makefile
+++ b/Makefile
@@ -91,6 +91,13 @@ else
   Q = @
 endif
 
+ifeq ("$(origin CHECK_TRACE_PRINTK)", "command line")
+  KBUILD_NO_TRACE_PRINTK = $(NO_TRACE_PRINTK)
+endif
+ifndef KBUILD_NO_TRACE_PRINTK
+  KBUILD_NO_TRACE_PRINTK = 0
+endif
+
 # If the user is running make -s (silent mode), suppress echoing of
 # commands
 
@@ -839,6 +846,10 @@ KBUILD_AFLAGS	+= -gz=zlib
 KBUILD_LDFLAGS	+= --compress-debug-sections=zlib
 endif
 
+ifeq ($(KBUILD_NO_TRACE_PRINTK),1)
+KBUILD_CFLAGS += -DNO_TRACE_PRINTK
+endif
+
 KBUILD_CFLAGS += $(DEBUG_CFLAGS)
 export DEBUG_CFLAGS
 
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 500def620d8f..bee432547d26 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -680,11 +680,14 @@ extern void tracing_stop(void);
 static inline __printf(1, 2)
 void ____trace_printk_check_format(const char *fmt, ...)
 {
+#ifdef NO_TRACE_PRINTK
+	extern void __no_trace_printk_on_build(void);
+	__no_trace_printk_on_build();
+#endif
 }
 #define __trace_printk_check_format(fmt, args...)			\
 do {									\
-	if (0)								\
-		____trace_printk_check_format(fmt, ##args);		\
+	____trace_printk_check_format(fmt, ##args);			\
 } while (0)
 
 /**
