diff mbox

ACPI: add dynamic_debug support

Message ID 1400755667-5377-1-git-send-email-bjorn@mork.no (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Bjørn Mork May 22, 2014, 10:47 a.m. UTC
Commit 1a699476e258 ("ACPI / hotplug / PCI: Hotplug notifications
from acpi_bus_notify()") added debug messages for a few common
events. These debug messages are unconditionally enabled if
CONFIG_DYNAMIC_DEBUG is defined, contrary to the documented
meaning, making the ACPI system spew lots of unwanted noise on
any kernel with dynamic debugging.

The bug was introduced by commit fbfddae69657 ("ACPI: Add
acpi_handle_<level>() interfaces"), which added the
CONFIG_DYNAMIC_DEBUG dependency without respecting its meaning.

Fix by adding real support for dynamic_debug.

Fixes: fbfddae69657 ("ACPI: Add acpi_handle_<level>() interfaces")
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
Background:  I suddenly got lots of these messages in v3.15-rcX
without having enabled any ACPI debugging AFAIK:

 ACPI: \_SB_.PCI0.HDEF: ACPI_NOTIFY_DEVICE_WAKE event
 ACPI: \_SB_.PCI0.EHC1: ACPI_NOTIFY_DEVICE_WAKE event
 ACPI: \_SB_.PCI0.EHC0: ACPI_NOTIFY_DEVICE_WAKE event
 ACPI: \_SB_.PCI0.IGBE: ACPI_NOTIFY_DEVICE_WAKE event
 ACPI: \_SB_.PCI0.HDEF: ACPI_NOTIFY_DEVICE_WAKE event

The reason turned out to be that I have CONFIG_DYNAMIC_DEBUG
enabled.  Which I must say I have primarily to debug other
stuff, and *not* ACPI.

An alternative way to fix this would of course be just removing
the bogus CONFIG_DYNAMIC_DEBUG dependency. But I guess it's better
to actually support dynamic debugging?


Bjørn


 drivers/acpi/utils.c |   64 ++++++++++++++++++++++++++++++++++++++++----------
 include/linux/acpi.h |   22 +++++++++++++++--
 2 files changed, 72 insertions(+), 14 deletions(-)

Comments

Rafael J. Wysocki May 26, 2014, 1:09 p.m. UTC | #1
On Thursday, May 22, 2014 12:47:47 PM Bjørn Mork wrote:
> Commit 1a699476e258 ("ACPI / hotplug / PCI: Hotplug notifications
> from acpi_bus_notify()") added debug messages for a few common
> events. These debug messages are unconditionally enabled if
> CONFIG_DYNAMIC_DEBUG is defined, contrary to the documented
> meaning, making the ACPI system spew lots of unwanted noise on
> any kernel with dynamic debugging.
> 
> The bug was introduced by commit fbfddae69657 ("ACPI: Add
> acpi_handle_<level>() interfaces"), which added the
> CONFIG_DYNAMIC_DEBUG dependency without respecting its meaning.
> 
> Fix by adding real support for dynamic_debug.
> 
> Fixes: fbfddae69657 ("ACPI: Add acpi_handle_<level>() interfaces")
> Signed-off-by: Bjørn Mork <bjorn@mork.no>

Queued up for 3.16, thanks!

> ---
> Background:  I suddenly got lots of these messages in v3.15-rcX
> without having enabled any ACPI debugging AFAIK:
> 
>  ACPI: \_SB_.PCI0.HDEF: ACPI_NOTIFY_DEVICE_WAKE event
>  ACPI: \_SB_.PCI0.EHC1: ACPI_NOTIFY_DEVICE_WAKE event
>  ACPI: \_SB_.PCI0.EHC0: ACPI_NOTIFY_DEVICE_WAKE event
>  ACPI: \_SB_.PCI0.IGBE: ACPI_NOTIFY_DEVICE_WAKE event
>  ACPI: \_SB_.PCI0.HDEF: ACPI_NOTIFY_DEVICE_WAKE event
> 
> The reason turned out to be that I have CONFIG_DYNAMIC_DEBUG
> enabled.  Which I must say I have primarily to debug other
> stuff, and *not* ACPI.
> 
> An alternative way to fix this would of course be just removing
> the bogus CONFIG_DYNAMIC_DEBUG dependency. But I guess it's better
> to actually support dynamic debugging?
> 
> 
> Bjørn
> 
> 
>  drivers/acpi/utils.c |   64 ++++++++++++++++++++++++++++++++++++++++----------
>  include/linux/acpi.h |   22 +++++++++++++++--
>  2 files changed, 72 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> index bba526148583..07c8c5a5ee95 100644
> --- a/drivers/acpi/utils.c
> +++ b/drivers/acpi/utils.c
> @@ -30,6 +30,7 @@
>  #include <linux/types.h>
>  #include <linux/hardirq.h>
>  #include <linux/acpi.h>
> +#include <linux/dynamic_debug.h>
>  
>  #include "internal.h"
>  
> @@ -457,6 +458,24 @@ acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code,
>  EXPORT_SYMBOL(acpi_evaluate_ost);
>  
>  /**
> + * acpi_handle_path: Return the object path of handle
> + *
> + * Caller must free the returned buffer
> + */
> +static char *acpi_handle_path(acpi_handle handle)
> +{
> +	struct acpi_buffer buffer = {
> +		.length = ACPI_ALLOCATE_BUFFER,
> +		.pointer = NULL
> +	};
> +
> +	if (in_interrupt() ||
> +	    acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer) != AE_OK)
> +		return NULL;
> +	return buffer.pointer;
> +}
> +
> +/**
>   * acpi_handle_printk: Print message with ACPI prefix and object path
>   *
>   * This function is called through acpi_handle_<level> macros and prints
> @@ -469,29 +488,50 @@ acpi_handle_printk(const char *level, acpi_handle handle, const char *fmt, ...)
>  {
>  	struct va_format vaf;
>  	va_list args;
> -	struct acpi_buffer buffer = {
> -		.length = ACPI_ALLOCATE_BUFFER,
> -		.pointer = NULL
> -	};
>  	const char *path;
>  
>  	va_start(args, fmt);
>  	vaf.fmt = fmt;
>  	vaf.va = &args;
>  
> -	if (in_interrupt() ||
> -	    acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer) != AE_OK)
> -		path = "<n/a>";
> -	else
> -		path = buffer.pointer;
> -
> -	printk("%sACPI: %s: %pV", level, path, &vaf);
> +	path = acpi_handle_path(handle);
> +	printk("%sACPI: %s: %pV", level, path ? path : "<n/a>" , &vaf);
>  
>  	va_end(args);
> -	kfree(buffer.pointer);
> +	kfree(path);
>  }
>  EXPORT_SYMBOL(acpi_handle_printk);
>  
> +#if defined(CONFIG_DYNAMIC_DEBUG)
> +/**
> + * __acpi_handle_debug: pr_debug with ACPI prefix and object path
> + *
> + * This function is called through acpi_handle_debug macro and debug
> + * prints a message with ACPI prefix and object path. This function
> + * acquires the global namespace mutex to obtain an object path.  In
> + * interrupt context, it shows the object path as <n/a>.
> + */
> +void
> +__acpi_handle_debug(struct _ddebug *descriptor, acpi_handle handle,
> +		    const char *fmt, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +	const char *path;
> +
> +	va_start(args, fmt);
> +	vaf.fmt = fmt;
> +	vaf.va = &args;
> +
> +	path = acpi_handle_path(handle);
> +	__dynamic_pr_debug(descriptor, "ACPI: %s: %pV", path ? path : "<n/a>", &vaf);
> +
> +	va_end(args);
> +	kfree(path);
> +}
> +EXPORT_SYMBOL(__acpi_handle_debug);
> +#endif
> +
>  /**
>   * acpi_has_method: Check whether @handle has a method named @name
>   * @handle: ACPI device handle
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 7a8f2cd66c8b..0e2569031a6f 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -37,6 +37,7 @@
>  
>  #include <linux/list.h>
>  #include <linux/mod_devicetable.h>
> +#include <linux/dynamic_debug.h>
>  
>  #include <acpi/acpi.h>
>  #include <acpi/acpi_bus.h>
> @@ -589,6 +590,14 @@ static inline __printf(3, 4) void
>  acpi_handle_printk(const char *level, void *handle, const char *fmt, ...) {}
>  #endif	/* !CONFIG_ACPI */
>  
> +#if defined(CONFIG_ACPI) && defined(CONFIG_DYNAMIC_DEBUG)
> +__printf(3, 4)
> +void __acpi_handle_debug(struct _ddebug *descriptor, acpi_handle handle, const char *fmt, ...);
> +#else
> +#define __acpi_handle_debug(descriptor, handle, fmt, ...)		\
> +	acpi_handle_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__);
> +#endif
> +
>  /*
>   * acpi_handle_<level>: Print message with ACPI prefix and object path
>   *
> @@ -610,11 +619,19 @@ acpi_handle_printk(const char *level, void *handle, const char *fmt, ...) {}
>  #define acpi_handle_info(handle, fmt, ...)				\
>  	acpi_handle_printk(KERN_INFO, handle, fmt, ##__VA_ARGS__)
>  
> -/* REVISIT: Support CONFIG_DYNAMIC_DEBUG when necessary */
> -#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
> +#if defined(DEBUG)
>  #define acpi_handle_debug(handle, fmt, ...)				\
>  	acpi_handle_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__)
>  #else
> +#if defined(CONFIG_DYNAMIC_DEBUG)
> +#define acpi_handle_debug(handle, fmt, ...)				\
> +do {									\
> +	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);			\
> +	if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT))		\
> +		__acpi_handle_debug(&descriptor, handle, pr_fmt(fmt),	\
> +				##__VA_ARGS__);				\
> +} while (0)
> +#else
>  #define acpi_handle_debug(handle, fmt, ...)				\
>  ({									\
>  	if (0)								\
> @@ -622,5 +639,6 @@ acpi_handle_printk(const char *level, void *handle, const char *fmt, ...) {}
>  	0;								\
>  })
>  #endif
> +#endif
>  
>  #endif	/*_LINUX_ACPI_H*/
>
diff mbox

Patch

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index bba526148583..07c8c5a5ee95 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -30,6 +30,7 @@ 
 #include <linux/types.h>
 #include <linux/hardirq.h>
 #include <linux/acpi.h>
+#include <linux/dynamic_debug.h>
 
 #include "internal.h"
 
@@ -457,6 +458,24 @@  acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code,
 EXPORT_SYMBOL(acpi_evaluate_ost);
 
 /**
+ * acpi_handle_path: Return the object path of handle
+ *
+ * Caller must free the returned buffer
+ */
+static char *acpi_handle_path(acpi_handle handle)
+{
+	struct acpi_buffer buffer = {
+		.length = ACPI_ALLOCATE_BUFFER,
+		.pointer = NULL
+	};
+
+	if (in_interrupt() ||
+	    acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer) != AE_OK)
+		return NULL;
+	return buffer.pointer;
+}
+
+/**
  * acpi_handle_printk: Print message with ACPI prefix and object path
  *
  * This function is called through acpi_handle_<level> macros and prints
@@ -469,29 +488,50 @@  acpi_handle_printk(const char *level, acpi_handle handle, const char *fmt, ...)
 {
 	struct va_format vaf;
 	va_list args;
-	struct acpi_buffer buffer = {
-		.length = ACPI_ALLOCATE_BUFFER,
-		.pointer = NULL
-	};
 	const char *path;
 
 	va_start(args, fmt);
 	vaf.fmt = fmt;
 	vaf.va = &args;
 
-	if (in_interrupt() ||
-	    acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer) != AE_OK)
-		path = "<n/a>";
-	else
-		path = buffer.pointer;
-
-	printk("%sACPI: %s: %pV", level, path, &vaf);
+	path = acpi_handle_path(handle);
+	printk("%sACPI: %s: %pV", level, path ? path : "<n/a>" , &vaf);
 
 	va_end(args);
-	kfree(buffer.pointer);
+	kfree(path);
 }
 EXPORT_SYMBOL(acpi_handle_printk);
 
+#if defined(CONFIG_DYNAMIC_DEBUG)
+/**
+ * __acpi_handle_debug: pr_debug with ACPI prefix and object path
+ *
+ * This function is called through acpi_handle_debug macro and debug
+ * prints a message with ACPI prefix and object path. This function
+ * acquires the global namespace mutex to obtain an object path.  In
+ * interrupt context, it shows the object path as <n/a>.
+ */
+void
+__acpi_handle_debug(struct _ddebug *descriptor, acpi_handle handle,
+		    const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+	const char *path;
+
+	va_start(args, fmt);
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	path = acpi_handle_path(handle);
+	__dynamic_pr_debug(descriptor, "ACPI: %s: %pV", path ? path : "<n/a>", &vaf);
+
+	va_end(args);
+	kfree(path);
+}
+EXPORT_SYMBOL(__acpi_handle_debug);
+#endif
+
 /**
  * acpi_has_method: Check whether @handle has a method named @name
  * @handle: ACPI device handle
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 7a8f2cd66c8b..0e2569031a6f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -37,6 +37,7 @@ 
 
 #include <linux/list.h>
 #include <linux/mod_devicetable.h>
+#include <linux/dynamic_debug.h>
 
 #include <acpi/acpi.h>
 #include <acpi/acpi_bus.h>
@@ -589,6 +590,14 @@  static inline __printf(3, 4) void
 acpi_handle_printk(const char *level, void *handle, const char *fmt, ...) {}
 #endif	/* !CONFIG_ACPI */
 
+#if defined(CONFIG_ACPI) && defined(CONFIG_DYNAMIC_DEBUG)
+__printf(3, 4)
+void __acpi_handle_debug(struct _ddebug *descriptor, acpi_handle handle, const char *fmt, ...);
+#else
+#define __acpi_handle_debug(descriptor, handle, fmt, ...)		\
+	acpi_handle_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__);
+#endif
+
 /*
  * acpi_handle_<level>: Print message with ACPI prefix and object path
  *
@@ -610,11 +619,19 @@  acpi_handle_printk(const char *level, void *handle, const char *fmt, ...) {}
 #define acpi_handle_info(handle, fmt, ...)				\
 	acpi_handle_printk(KERN_INFO, handle, fmt, ##__VA_ARGS__)
 
-/* REVISIT: Support CONFIG_DYNAMIC_DEBUG when necessary */
-#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(DEBUG)
 #define acpi_handle_debug(handle, fmt, ...)				\
 	acpi_handle_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__)
 #else
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define acpi_handle_debug(handle, fmt, ...)				\
+do {									\
+	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);			\
+	if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT))		\
+		__acpi_handle_debug(&descriptor, handle, pr_fmt(fmt),	\
+				##__VA_ARGS__);				\
+} while (0)
+#else
 #define acpi_handle_debug(handle, fmt, ...)				\
 ({									\
 	if (0)								\
@@ -622,5 +639,6 @@  acpi_handle_printk(const char *level, void *handle, const char *fmt, ...) {}
 	0;								\
 })
 #endif
+#endif
 
 #endif	/*_LINUX_ACPI_H*/