[PATCH 0/4] printk_index: Fix false positives

[PATCH 0/4] printk_index: Fix false positives

[Geert Uytterhoeven]

	Hi all,

When printk-indexing is enabled, each printk() invocation emits a
pi_entry structure, containing the format string and other information
related to its location in the kernel sources.  This is even true when
the 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.
Hence when debugging is disabled, this leads to the inclusion in the
index of lots of printk formats that cannot be emitted by the current
kernel.

This series fixes that for the common debug helpers under include/.
It reduces the size of an arm64 defconfig kernel with
CONFIG_PRINTK_INDEX=y by ca. 1.5 MiB, or 28% of the overhead of
enabling CONFIG_PRINTK_INDEX=y.

Notes:
  - netdev_(v)dbg() and netif_(v)dbg() are not affected, as
    net{dev,if}_printk() do not implement printk-indexing, except
    for the single global internal instance of __netdev_printk().
  - This series fixes only debug code in global header files under
    include/.  There are more cases to fix in subsystem-specific header
    files and in sources files.

Thanks for your comments!

Geert Uytterhoeven (4):
  printk: Let no_printk() use _printk()
  dev_printk: Add and use dev_no_printk()
  dyndbg: Use *no_printk() helpers
  ceph: Use no_printk() helper

 include/linux/ceph/ceph_debug.h | 18 +++++++-----------
 include/linux/dev_printk.h      | 25 +++++++++++++------------
 include/linux/dynamic_debug.h   |  4 ++--
 include/linux/printk.h          |  2 +-
 4 files changed, 23 insertions(+), 26 deletions(-)

-- 
2.34.1

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

[PATCH 1/4] printk: Let no_printk() use _printk()

[Geert Uytterhoeven]

When printk-indexing is enabled, each printk() invocation emits a
pi_entry structure, containing the format string and other information
related to its location in the kernel sources.  This is even true for
no_printk(): while the actual code to print the message is optimized out
by the compiler due to the always-false check, the pi_entry structure is
still emitted.

As the main purpose of no_printk() is to provide a helper to maintain
printf()-style format checking when debugging is disabled, this leads to
the inclusion in the index of lots of printk formats that cannot be
emitted by the current kernel.

Fix this by switching no_printk() from printk() to _printk().

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

Fixes: 337015573718b161 ("printk: Userspace format indexing support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 include/linux/printk.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 8ef499ab3c1ed2ec..e4878bb58f663370 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -126,7 +126,7 @@ struct va_format {
 #define no_printk(fmt, ...)				\
 ({							\
 	if (0)						\
-		printk(fmt, ##__VA_ARGS__);		\
+		_printk(fmt, ##__VA_ARGS__);		\
 	0;						\
 })
 
-- 
2.34.1

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

[Geert Uytterhoeven]

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

[PATCH 3/4] dyndbg: Use *no_printk() helpers

[Geert Uytterhoeven]

When printk-indexing is enabled, each printk() or dev_printk()
invocation emits a pi_entry structure.  This is even true when the call
is protected by an always-false check: while the actual code to print
the message is optimized out by the compiler, the pi_entry structure is
still emitted.

Fix this by replacing "if (0) *printk(...)" constructs by calls to the
corresponding *no_printk() helpers.

Note that this has minimal impact, as most (all?) callers of
dynamic_{pr,dev}_debug() are protected by checks for DYNAMIC_DEBUG
anyway.  Still, using the helpers serves as a good example to follow.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 include/linux/dynamic_debug.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 4fcbf4d4fd0a29d1..ff44ec346162a164 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -305,9 +305,9 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 #define DYNAMIC_DEBUG_BRANCH(descriptor) false
 
 #define dynamic_pr_debug(fmt, ...)					\
-	do { if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); } while (0)
+	no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
 #define dynamic_dev_dbg(dev, fmt, ...)					\
-	do { if (0) dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); } while (0)
+	dev_no_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__)
 #define dynamic_hex_dump(prefix_str, prefix_type, rowsize,		\
 			 groupsize, buf, len, ascii)			\
 	do { if (0)							\
-- 
2.34.1

[PATCH 4/4] ceph: Use no_printk() helper

[Geert Uytterhoeven]

When printk-indexing is enabled, each printk() invocation emits a
pi_entry structure.  This is even true when the call is protected by an
always-false check: while the actual code to print the message is
optimized out by the compiler, the pi_entry structure is still emitted.

Fix this by replacing "if (0) printk(...)" constructs by calls to the
no_printk() helper.

This reduces the size of an arm64 kernel with CONFIG_PRINTK_INDEX=y and
CONFIG_CEPH_FS=y by ca. 4 KiB.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 include/linux/ceph/ceph_debug.h | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/include/linux/ceph/ceph_debug.h b/include/linux/ceph/ceph_debug.h
index 11a92a946016eab5..5f904591fa5f9e57 100644
--- a/include/linux/ceph/ceph_debug.h
+++ b/include/linux/ceph/ceph_debug.h
@@ -27,17 +27,13 @@
 		 ##__VA_ARGS__)
 # else
 /* faux printk call just to see any compiler warnings. */
-#  define dout(fmt, ...)	do {				\
-		if (0)						\
-			printk(KERN_DEBUG fmt, ##__VA_ARGS__);	\
-	} while (0)
-#  define doutc(client, fmt, ...)	do {			\
-		if (0)						\
-			printk(KERN_DEBUG "[%pU %llu] " fmt,	\
-			&client->fsid,				\
-			client->monc.auth->global_id,		\
-			##__VA_ARGS__);				\
-		} while (0)
+#  define dout(fmt, ...)					\
+		no_printk(KERN_DEBUG fmt, ##__VA_ARGS__)
+#  define doutc(client, fmt, ...)				\
+		no_printk(KERN_DEBUG "[%pU %llu] " fmt,		\
+			  &client->fsid,			\
+			  client->monc.auth->global_id,		\
+			  ##__VA_ARGS__)
 # endif
 
 #else
-- 
2.34.1

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

[Andy Shevchenko]

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 0/4] printk_index: Fix false positives

[Andy Shevchenko]

On Wed, Feb 28, 2024 at 03:00:01PM +0100, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> When printk-indexing is enabled, each printk() invocation emits a
> pi_entry structure, containing the format string and other information
> related to its location in the kernel sources.  This is even true when
> the 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.
> Hence when debugging is disabled, this leads to the inclusion in the
> index of lots of printk formats that cannot be emitted by the current
> kernel.
> 
> This series fixes that for the common debug helpers under include/.
> It reduces the size of an arm64 defconfig kernel with
> CONFIG_PRINTK_INDEX=y by ca. 1.5 MiB, or 28% of the overhead of
> enabling CONFIG_PRINTK_INDEX=y.
> 
> Notes:
>   - netdev_(v)dbg() and netif_(v)dbg() are not affected, as
>     net{dev,if}_printk() do not implement printk-indexing, except
>     for the single global internal instance of __netdev_printk().
>   - This series fixes only debug code in global header files under
>     include/.  There are more cases to fix in subsystem-specific header
>     files and in sources files.


The whole series makes a lot of sense and gives a good examples for above
mentioned subsystem specific code on how to do it in a better way.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

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

[Geert Uytterhoeven]

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()

[Andy Shevchenko]

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

Re: [PATCH 0/4] printk_index: Fix false positives

[Xiubo Li]

On 2/28/24 22:00, Geert Uytterhoeven wrote:
> 	Hi all,
>
> When printk-indexing is enabled, each printk() invocation emits a
> pi_entry structure, containing the format string and other information
> related to its location in the kernel sources.  This is even true when
> the 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.
> Hence when debugging is disabled, this leads to the inclusion in the
> index of lots of printk formats that cannot be emitted by the current
> kernel.
>
> This series fixes that for the common debug helpers under include/.
> It reduces the size of an arm64 defconfig kernel with
> CONFIG_PRINTK_INDEX=y by ca. 1.5 MiB, or 28% of the overhead of
> enabling CONFIG_PRINTK_INDEX=y.
>
> Notes:
>    - netdev_(v)dbg() and netif_(v)dbg() are not affected, as
>      net{dev,if}_printk() do not implement printk-indexing, except
>      for the single global internal instance of __netdev_printk().
>    - This series fixes only debug code in global header files under
>      include/.  There are more cases to fix in subsystem-specific header
>      files and in sources files.
>
> Thanks for your comments!
>
> Geert Uytterhoeven (4):
>    printk: Let no_printk() use _printk()
>    dev_printk: Add and use dev_no_printk()
>    dyndbg: Use *no_printk() helpers
>    ceph: Use no_printk() helper
>
>   include/linux/ceph/ceph_debug.h | 18 +++++++-----------
>   include/linux/dev_printk.h      | 25 +++++++++++++------------
>   include/linux/dynamic_debug.h   |  4 ++--
>   include/linux/printk.h          |  2 +-
>   4 files changed, 23 insertions(+), 26 deletions(-)
>
This series LGTM.

Reviewed-by: Xiubo Li <xiubli@redhat.com>

Re: [PATCH 0/4] printk_index: Fix false positives

[Chris Down]

Thanks for working on this! This whole patchset looks good to me.

Reviewed-by: Chris Down <chris@chrisdown.name>

Re: [PATCH 0/4] printk_index: Fix false positives

[Petr Mladek]

On Wed 2024-02-28 15:00:01, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> When printk-indexing is enabled, each printk() invocation emits a
> pi_entry structure, containing the format string and other information
> related to its location in the kernel sources.  This is even true when
> the 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.
> Hence when debugging is disabled, this leads to the inclusion in the
> index of lots of printk formats that cannot be emitted by the current
> kernel.
> 
> This series fixes that for the common debug helpers under include/.
> It reduces the size of an arm64 defconfig kernel with
> CONFIG_PRINTK_INDEX=y by ca. 1.5 MiB, or 28% of the overhead of
> enabling CONFIG_PRINTK_INDEX=y.
> 
> Notes:
>   - netdev_(v)dbg() and netif_(v)dbg() are not affected, as
>     net{dev,if}_printk() do not implement printk-indexing, except
>     for the single global internal instance of __netdev_printk().
>   - This series fixes only debug code in global header files under
>     include/.  There are more cases to fix in subsystem-specific header
>     files and in sources files.
> 
> Thanks for your comments!
> 
> Geert Uytterhoeven (4):
>   printk: Let no_printk() use _printk()
>   dev_printk: Add and use dev_no_printk()
>   dyndbg: Use *no_printk() helpers
>   ceph: Use no_printk() helper
> 
>  include/linux/ceph/ceph_debug.h | 18 +++++++-----------
>  include/linux/dev_printk.h      | 25 +++++++++++++------------
>  include/linux/dynamic_debug.h   |  4 ++--
>  include/linux/printk.h          |  2 +-
>  4 files changed, 23 insertions(+), 26 deletions(-)

The whole series looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

I am going take it via printk tree for 6.10.

I am sorry that I haven't looked at it in time before the merge
window for 6.9. I have been snowed under various tasks. The changes
are not complicated. But they also are not critical to be pushed
an expedite way.

Best Regards,
Petr

Re: [PATCH 0/4] printk_index: Fix false positives

[Petr Mladek]

On Tue 2024-03-19 16:09:01, Petr Mladek wrote:
> On Wed 2024-02-28 15:00:01, Geert Uytterhoeven wrote:
> > 	Hi all,
> > 
> > When printk-indexing is enabled, each printk() invocation emits a
> > pi_entry structure, containing the format string and other information
> > related to its location in the kernel sources.  This is even true when
> > the 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.
> > Hence when debugging is disabled, this leads to the inclusion in the
> > index of lots of printk formats that cannot be emitted by the current
> > kernel.
> > 
> > This series fixes that for the common debug helpers under include/.
> > It reduces the size of an arm64 defconfig kernel with
> > CONFIG_PRINTK_INDEX=y by ca. 1.5 MiB, or 28% of the overhead of
> > enabling CONFIG_PRINTK_INDEX=y.
> > 
> > Notes:
> >   - netdev_(v)dbg() and netif_(v)dbg() are not affected, as
> >     net{dev,if}_printk() do not implement printk-indexing, except
> >     for the single global internal instance of __netdev_printk().
> >   - This series fixes only debug code in global header files under
> >     include/.  There are more cases to fix in subsystem-specific header
> >     files and in sources files.
> > 
> > Thanks for your comments!
> > 
> > Geert Uytterhoeven (4):
> >   printk: Let no_printk() use _printk()
> >   dev_printk: Add and use dev_no_printk()
> >   dyndbg: Use *no_printk() helpers
> >   ceph: Use no_printk() helper
> > 
> >  include/linux/ceph/ceph_debug.h | 18 +++++++-----------
> >  include/linux/dev_printk.h      | 25 +++++++++++++------------
> >  include/linux/dynamic_debug.h   |  4 ++--
> >  include/linux/printk.h          |  2 +-
> >  4 files changed, 23 insertions(+), 26 deletions(-)
> 
> The whole series looks good to me:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> 
> I am going take it via printk tree for 6.10.

JFYI, the patchset has been committed into printk/linux.git, branch
for-6.10.

Best Regards,
Petr