[PATCH 2/4] dev_printk: Add and use dev_no_printk()

Geert Uytterhoeven posted 4 patches 1 year, 11 months ago
[PATCH 2/4] dev_printk: Add and use dev_no_printk()
Posted by Geert Uytterhoeven 1 year, 11 months ago
When printk-indexing is enabled, each dev_printk() invocation emits a
pi_entry structure.  This is even true when the dev_printk() is
protected by an always-false check, as is typically the case for debug
messages: while the actual code to print the message is optimized out by
the compiler, the pi_entry structure is still emitted.

Avoid emitting pi_entry structures for unavailable dev_printk() kernel
messages by:
  1. Introducing a dev_no_printk() helper, mimicked after the existing
     no_printk() helper, which calls _dev_printk() instead of
     dev_printk(),
  2. Replacing all "if (0) dev_printk(...)" constructs by calls to the
     new helper.

This reduces the size of an arm64 defconfig kernel with
CONFIG_PRINTK_INDEX=y by 957 KiB.

Fixes: ad7d61f159db7397 ("printk: index: Add indexing support to dev_printk")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 include/linux/dev_printk.h | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/include/linux/dev_printk.h b/include/linux/dev_printk.h
index 6bfe70decc9fb3bc..ae80a303c216be55 100644
--- a/include/linux/dev_printk.h
+++ b/include/linux/dev_printk.h
@@ -129,6 +129,16 @@ void _dev_info(const struct device *dev, const char *fmt, ...)
 		_dev_printk(level, dev, fmt, ##__VA_ARGS__);		\
 	})
 
+/*
+ * Dummy dev_printk for disabled debugging statements to use whilst maintaining
+ * gcc's format checking.
+ */
+#define dev_no_printk(level, dev, fmt, ...)				\
+	({								\
+		if (0)							\
+			_dev_printk(level, dev, fmt, ##__VA_ARGS__);	\
+	})
+
 /*
  * #defines for all the dev_<level> macros to prefix with whatever
  * possible use of #define dev_fmt(fmt) ...
@@ -158,10 +168,7 @@ void _dev_info(const struct device *dev, const char *fmt, ...)
 	dev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
 #else
 #define dev_dbg(dev, fmt, ...)						\
-({									\
-	if (0)								\
-		dev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__); \
-})
+	dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
 #endif
 
 #ifdef CONFIG_PRINTK
@@ -247,20 +254,14 @@ do {									\
 } while (0)
 #else
 #define dev_dbg_ratelimited(dev, fmt, ...)				\
-do {									\
-	if (0)								\
-		dev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__); \
-} while (0)
+	dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
 #endif
 
 #ifdef VERBOSE_DEBUG
 #define dev_vdbg	dev_dbg
 #else
 #define dev_vdbg(dev, fmt, ...)						\
-({									\
-	if (0)								\
-		dev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__); \
-})
+	dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
 #endif
 
 /*
-- 
2.34.1
Re: [PATCH 2/4] dev_printk: Add and use dev_no_printk()
Posted by Andy Shevchenko 1 year, 11 months ago
On Wed, Feb 28, 2024 at 03:00:03PM +0100, Geert Uytterhoeven wrote:
> When printk-indexing is enabled, each dev_printk() invocation emits a
> pi_entry structure.  This is even true when the dev_printk() is
> protected by an always-false check, as is typically the case for debug
> messages: while the actual code to print the message is optimized out by
> the compiler, the pi_entry structure is still emitted.
> 
> Avoid emitting pi_entry structures for unavailable dev_printk() kernel
> messages by:
>   1. Introducing a dev_no_printk() helper, mimicked after the existing
>      no_printk() helper, which calls _dev_printk() instead of
>      dev_printk(),
>   2. Replacing all "if (0) dev_printk(...)" constructs by calls to the
>      new helper.
> 
> This reduces the size of an arm64 defconfig kernel with
> CONFIG_PRINTK_INDEX=y by 957 KiB.

...

> +/*
> + * Dummy dev_printk for disabled debugging statements to use whilst maintaining

dev_printk()

> + * gcc's format checking.
> + */
> +#define dev_no_printk(level, dev, fmt, ...)				\
> +	({								\
> +		if (0)							\
> +			_dev_printk(level, dev, fmt, ##__VA_ARGS__);	\
> +	})

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 2/4] dev_printk: Add and use dev_no_printk()
Posted by Geert Uytterhoeven 1 year, 11 months ago
Hi Andy,

On Wed, Feb 28, 2024 at 6:39 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Wed, Feb 28, 2024 at 03:00:03PM +0100, Geert Uytterhoeven wrote:
> > When printk-indexing is enabled, each dev_printk() invocation emits a
> > pi_entry structure.  This is even true when the dev_printk() is
> > protected by an always-false check, as is typically the case for debug
> > messages: while the actual code to print the message is optimized out by
> > the compiler, the pi_entry structure is still emitted.
> >
> > Avoid emitting pi_entry structures for unavailable dev_printk() kernel
> > messages by:
> >   1. Introducing a dev_no_printk() helper, mimicked after the existing
> >      no_printk() helper, which calls _dev_printk() instead of
> >      dev_printk(),
> >   2. Replacing all "if (0) dev_printk(...)" constructs by calls to the
> >      new helper.
> >
> > This reduces the size of an arm64 defconfig kernel with
> > CONFIG_PRINTK_INDEX=y by 957 KiB.
>
> ...
>
> > +/*
> > + * Dummy dev_printk for disabled debugging statements to use whilst maintaining
>
> dev_printk()

I fully agree.  But the surrounding comments don't, so I gave in.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH 2/4] dev_printk: Add and use dev_no_printk()
Posted by Andy Shevchenko 1 year, 11 months ago
On Wed, Feb 28, 2024 at 08:33:19PM +0100, Geert Uytterhoeven wrote:
> On Wed, Feb 28, 2024 at 6:39 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, Feb 28, 2024 at 03:00:03PM +0100, Geert Uytterhoeven wrote:

...

> > dev_printk()
> 
> I fully agree.  But the surrounding comments don't, so I gave in.

Is there any better justification to keep a technical debt?
I mean, the comment here can be better than existed ones, if you feel
uncomfortable with it, you can fix the rest in a separate patch.
Would it be a big deal? :-)

-- 
With Best Regards,
Andy Shevchenko