Message ID | 8583d54f1687c801c6cda8edddf2cf0344c6e883.1709127473.git.geert+renesas@glider.be (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | printk_index: Fix false positives | expand |
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__); \ > + })
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
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? :-)
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 /*
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(-)