Messages in this thread Patch in this message |  | | Subject | Re: [PATCH] printk: fix pr_debug and pr_devel to elide function calls | | From | Jason Baron <> | | Date | Fri, 4 Dec 2015 11:31:42 -0500 |
| |
On 12/03/2015 05:45 PM, Aaron Conole wrote: > Currently, pr_debug and pr_devel will not elide function call arguments > appearing in calls to no_printk for these macros. This is because all > side effects must be honored before proceeding to the 0-value assignment > in no_printk. > > The behavior is contrary to documentation found in the CodingStyle and > header file where these functions are declared. > > This patch corrects that behavior by shunting out the call to no_printk > completely. The format string is still checked by gcc for correctness, but > no code seems to be emitted in common cases. > > fixes commit 5264f2f75d86 ("include/linux/printk.h: use and neaten > no_printk") > > Signed-off-by: Aaron Conole <aconole@redhat.com> > Reported-by: Dmitry Vyukov <dvyukov@google.com> > Cc: Joe Perches <joe@perches.com>
I think we should just convert no_printk() to not emit anything. This will avoid us adding unwrapped calls to 'no_printk()' in the future, and I think makes the code more readable. Based on Joe's previous 'eliminated_printk()' thing. IE:
diff --git a/include/linux/printk.h b/include/linux/printk.h index 9729565..58632bf 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -108,11 +108,11 @@ struct va_format { * Dummy printk for disabled debugging statements to use whilst maintaining * gcc's format and side-effect checking. */ -static inline __printf(1, 2) -int no_printk(const char *fmt, ...) -{ - return 0; -} +#define no_printk(fmt, ...) \ +do { \ + if (0) \ + printk(fmt, ##__VA_ARGS__); \ +} while (0)
#ifdef CONFIG_EARLY_PRINTK extern asmlinkage __printf(1, 2) Thanks,
-Jason
> > --- > include/linux/printk.h | 42 +++++++++++++++++++++++++++++++++--------- > 1 file changed, 33 insertions(+), 9 deletions(-) > > diff --git a/include/linux/printk.h b/include/linux/printk.h > index 9729565..87dfdd2 100644 > --- a/include/linux/printk.h > +++ b/include/linux/printk.h > @@ -270,8 +270,12 @@ extern asmlinkage void dump_stack(void) __cold; > #define pr_devel(fmt, ...) \ > printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > #else > -#define pr_devel(fmt, ...) \ > - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > +#define pr_devel(fmt, ...) \ > +do { \ > + if (0) { \ > + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\ > + } \ > +} while (0) > #endif > > #include <linux/dynamic_debug.h> > @@ -286,7 +290,11 @@ extern asmlinkage void dump_stack(void) __cold; > printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > #else > #define pr_debug(fmt, ...) \ > - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > +do { \ > + if (0) { \ > + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\ > + } \ > +} while (0) > #endif > > /* > @@ -341,7 +349,11 @@ extern asmlinkage void dump_stack(void) __cold; > printk_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > #else > #define pr_devel_once(fmt, ...) \ > - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > +do { \ > + if (0) { \ > + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\ > + } \ > +} while (0) > #endif > > /* If you are writing a driver, please use dev_dbg instead */ > @@ -350,7 +362,11 @@ extern asmlinkage void dump_stack(void) __cold; > printk_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > #else > #define pr_debug_once(fmt, ...) \ > - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > +do { \ > + if (0) { \ > + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\ > + } \ > +} while (0) > #endif > > /* > @@ -392,8 +408,12 @@ extern asmlinkage void dump_stack(void) __cold; > #define pr_devel_ratelimited(fmt, ...) \ > printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > #else > -#define pr_devel_ratelimited(fmt, ...) \ > - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > +#define pr_devel_ratelimited(fmt, ...) \ > +do { \ > + if (0) { \ > + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\ > + } \ > +} while (0) > #endif > > /* If you are writing a driver, please use dev_dbg instead */ > @@ -413,8 +433,12 @@ do { \ > #define pr_debug_ratelimited(fmt, ...) \ > printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > #else > -#define pr_debug_ratelimited(fmt, ...) \ > - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > +#define pr_debug_ratelimited(fmt, ...) \ > +do { \ > + if (0) { \ > + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\ > + } \ > +} while (0) > #endif > > extern const struct file_operations kmsg_fops; >
|  |