Re: [PATCH] PM: fix dynamic debug within pm_pr_debug()

Re: [PATCH] PM: fix dynamic debug within pm_pr_debug()

[David Cohen]

Hi,

On 3/10/22 12:48, Rafael J. Wysocki wrote:

> On Sun, Mar 6, 2022 at 6:52 AM David Cohen <dacohen@pm.me> wrote:
>
> The patch has been sent as an attachment, so I cannot comment it inline.

Ouch. I used git send-email, but I'm using protonmail with
protonmail-bridge and it somehow mangled it. I'll fix it in
future.

>
> However, printk(KERN_DEBIG, ...) is not the same as pr_debig() AFAICS.
A bit of history I found on kernel git history:

kernel pm debug messages used to use pr_debug() until it was
changed by this commit to prevent printing pm debug messages
unless the sysfs debug flag was set:
8d8b2441db96 PM / sleep: Do not print debug messages by default

This commit implemented their own debugging print function. And
It was this commit that broke dynamic debug on kernel pm functions
when changed pr_debug() to printk(KERN_DEBUG, ...).

The patch I'm sending is keeping the same logic of checking the
sysfs flag, but instead of implementing its own debugging print function,
it will just use pr_debug again which is dynamic debug friendly.
As per my tests I'm able to individually control them again through
dynamic debug control interface.

FWIW I found a typo on my commit message, so I'll fix it and resend,
avoiding the protonmail-bridge (then inline comment will be possible).

Regards, David

[PATCH] PM: fix dynamic debug within pm_pr_debug()

[David Cohen]

Currently, pm_pr_debug() and pm_deferred_pr_debug() use __pm_pr_debug()
to filter pm debug messages based on pm_debug_messages_on flag.
According to __pm_pr_debug() implementation, pm_deferred_pr_debug()
indirectly calls printk_deferred() within __pm_pr_debug() which doesn't
support dynamic debug, but pm_pr_debug() indirectly calls pr_debug()
which does support dynamic debug.

The problem is if we enable/disable dynamic debug inside __pm_pr_debug()
it will affect all pm_pr_debug() calls at once, so we can't individually
control them.

This patch changes __pm_pr_debug() implementation into macros to make
pr_debug() to be directly called by all pr_pm_debug() cases. As a direct
side effect all pr_pm_debug() can be individually controlled by dynamic
debug feature.

Signed-off-by: David Cohen <dacohen@pm.me>
---
 include/linux/suspend.h | 19 +++++++++++++++----
 kernel/power/main.c     | 29 -----------------------------
 2 files changed, 15 insertions(+), 33 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 300273ff40cc..d727d3c867e3 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -542,22 +542,33 @@ static inline void unlock_system_sleep(void) {}
 #ifdef CONFIG_PM_SLEEP_DEBUG
 extern bool pm_print_times_enabled;
 extern bool pm_debug_messages_on;
-extern __printf(2, 3) void __pm_pr_dbg(bool defer, const char *fmt, ...);
+#define __pm_pr_dbg(fmt, ...)					\
+	do {							\
+		if (pm_debug_messages_on)			\
+			pr_debug("PM: " fmt, ##__VA_ARGS__);	\
+	} while(0)
+#define __pm_deferred_pr_dbg(fmt, ...)				\
+	do {							\
+		if (pm_debug_messages_on)			\
+			printk_deferred(KERN_DEBUG "PM: " fmt, ##__VA_ARGS__);	\
+	} while(0)
 #else
 #define pm_print_times_enabled	(false)
 #define pm_debug_messages_on	(false)

 #include <linux/printk.h>

-#define __pm_pr_dbg(defer, fmt, ...) \
+#define __pm_pr_dbg(fmt, ...) \
+	no_printk(KERN_DEBUG fmt, ##__VA_ARGS__)
+#define __pm_deferred_pr_dbg(fmt, ...) \
 	no_printk(KERN_DEBUG fmt, ##__VA_ARGS__)
 #endif

 #define pm_pr_dbg(fmt, ...) \
-	__pm_pr_dbg(false, fmt, ##__VA_ARGS__)
+	__pm_pr_dbg(fmt, ##__VA_ARGS__)

 #define pm_deferred_pr_dbg(fmt, ...) \
-	__pm_pr_dbg(true, fmt, ##__VA_ARGS__)
+	__pm_deferred_pr_dbg(fmt, ##__VA_ARGS__)

 #ifdef CONFIG_PM_AUTOSLEEP

diff --git a/kernel/power/main.c b/kernel/power/main.c
index 7e646079fbeb..5242bf2ee469 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -545,35 +545,6 @@ static int __init pm_debug_messages_setup(char *str)
 }
 __setup("pm_debug_messages", pm_debug_messages_setup);

-/**
- * __pm_pr_dbg - Print a suspend debug message to the kernel log.
- * @defer: Whether or not to use printk_deferred() to print the message.
- * @fmt: Message format.
- *
- * The message will be emitted if enabled through the pm_debug_messages
- * sysfs attribute.
- */
-void __pm_pr_dbg(bool defer, const char *fmt, ...)
-{
-	struct va_format vaf;
-	va_list args;
-
-	if (!pm_debug_messages_on)
-		return;
-
-	va_start(args, fmt);
-
-	vaf.fmt = fmt;
-	vaf.va = &args;
-
-	if (defer)
-		printk_deferred(KERN_DEBUG "PM: %pV", &vaf);
-	else
-		printk(KERN_DEBUG "PM: %pV", &vaf);
-
-	va_end(args);
-}
-
 #else /* !CONFIG_PM_SLEEP_DEBUG */
 static inline void pm_print_times_init(void) {}
 #endif /* CONFIG_PM_SLEEP_DEBUG */
--
2.35.1

Re: [PATCH] PM: fix dynamic debug within pm_pr_debug()

[Rafael J. Wysocki]

On Sun, Mar 6, 2022 at 6:52 AM David Cohen <dacohen@pm.me> wrote:

The patch has been sent as an attachment, so I cannot comment it inline.

However, printk(KERN_DEBIG, ...) is not the same as pr_debig() AFAICS.