Re: [PATCH 6/7] dynamic debug v2 - convert cpufreq

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

From: Jason Baron <jbaron@redhat.com>
To: Dominik Brodowski <linux@dominikbrodowski.net>,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	joe@perches.com, greg@kroah.com, nick@nick-andrew.net,
	randy.dunlap@oracle.com
Subject: Re: [PATCH 6/7] dynamic debug v2 - convert cpufreq
Date: Thu, 17 Jul 2008 17:05:32 -0400	[thread overview]
Message-ID: <20080717210531.GA13252@redhat.com> (raw)
In-Reply-To: <20080715230731.GA15208@isilmar.linta.de>

On Wed, Jul 16, 2008 at 01:07:31AM +0200, Dominik Brodowski wrote:
> 
> Hi,
> 
> On Tue, Jul 15, 2008 at 05:36:13PM -0400, Jason Baron wrote:
> > +#include <linux/dynamic_debug_cpufreq.h>
> 
> what's contained in this file (couldn't find it in this or one of the other
> diffs, but may have missed it).
> 

i forgot to include it. A full patch is found below.

> > -#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "cpufreq-nforce2", msg)
> > +#define dprintk(msg...) do { \
> > +	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
> > +		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "cpufreq-nforce2", msg); \
> > +	} while (0)
> 
> Hm.... What about leaving this as it is, renaming the
> drivers/cpufreq/cpufreq.c function to __cpufreq_debug_printk(), and then
> adding to include/linux/cpufreq.h
> 
> #define cpufreq_debug_printk(type, prefix, msg...) do { \
> 	if (dynamic_dbg_enabled(TYPE_FLAG, type, cpufreq_debug))
> 		cpufreq_debug_printk(type, prefix, msg); \
> 	} while (0)
> 

i agree that would be a bit cleaner. the new patch below includes this suggestion.


> > +#if defined(CONFIG_CPU_FREQ_DEBUG) || defined (CONFIG_DYNAMIC_PRINTK_DEBUG)
> ...
> > +#if defined(CONFIG_CPU_FREQ_DEBUG) || defined (CONFIG_DYNAMIC_PRINTK_DEBUG)
> 
> can't we just depend on thing on another?
> 

We could make CONFIG_CPU_FREQ_DEBUG force CONFIG_DYNAMIC_PRINTK_DEBUG to be on.
However, i'm trying to allow CONFIG_CPU_FREQ_DEBUG to be turned on without
enabling CONFIG_DYNAMIC_PRINTK_DEBUG. That's consistent with how i'm trying to
do this patch series. That is, individual subsystems can turn their respective
debugging on without forcing on CONFIG_DYNAMIC_PRINTK_DEBUG.

> > -module_param(debug, uint, 0644);
> > -MODULE_PARM_DESC(debug, "CPUfreq debugging: add 1 to debug core,"
> > +module_param(cpufreq_debug, uint, 0644);
> > +MODULE_PARM_DESC(cpufreq_debug, "CPUfreq debugging: add 1 to debug core,"
> >  			" 2 to debug drivers, and 4 to debug governors.");
> 
> cpufreq.cpufreq_debug is ugly and not backwards compatible... what about 
> 
> module_param_named(debug, cpufreq_debug, uint, 0644) [or the other way
> around, I always forget...])
> 

makes sense. 

thanks,

-Jason


diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
index e2d870d..549edbd 100644
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -25,6 +25,7 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
diff --git a/arch/x86/kernel/cpu/cpufreq/cpufreq-nforce2.c b/arch/x86/kernel/cpu/cpufreq/cpufreq-nforce2.c
index f03e915..3eb343d 100644
--- a/arch/x86/kernel/cpu/cpufreq/cpufreq-nforce2.c
+++ b/arch/x86/kernel/cpu/cpufreq/cpufreq-nforce2.c
@@ -7,6 +7,7 @@
  *  BIG FAT DISCLAIMER: Work in progress code. Possibly *dangerous*
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
diff --git a/arch/x86/kernel/cpu/cpufreq/gx-suspmod.c b/arch/x86/kernel/cpu/cpufreq/gx-suspmod.c
index 9d9eae8..212204f 100644
--- a/arch/x86/kernel/cpu/cpufreq/gx-suspmod.c
+++ b/arch/x86/kernel/cpu/cpufreq/gx-suspmod.c
@@ -73,6 +73,7 @@
  *			Suspend Modulation - Definitions		*
  ************************************************************************/
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
diff --git a/arch/x86/kernel/cpu/cpufreq/longhaul.c b/arch/x86/kernel/cpu/cpufreq/longhaul.c
index 06fcce5..b910935 100644
--- a/arch/x86/kernel/cpu/cpufreq/longhaul.c
+++ b/arch/x86/kernel/cpu/cpufreq/longhaul.c
@@ -21,6 +21,7 @@
  *  BIG FAT DISCLAIMER: Work in progress code. Possibly *dangerous*
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
diff --git a/arch/x86/kernel/cpu/cpufreq/longrun.c b/arch/x86/kernel/cpu/cpufreq/longrun.c
index af4a867..11ce4fa 100644
--- a/arch/x86/kernel/cpu/cpufreq/longrun.c
+++ b/arch/x86/kernel/cpu/cpufreq/longrun.c
@@ -6,6 +6,7 @@
  *  BIG FAT DISCLAIMER: Work in progress code. Possibly *dangerous*
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
diff --git a/arch/x86/kernel/cpu/cpufreq/p4-clockmod.c b/arch/x86/kernel/cpu/cpufreq/p4-clockmod.c
index 199e4e0..868cda4 100644
--- a/arch/x86/kernel/cpu/cpufreq/p4-clockmod.c
+++ b/arch/x86/kernel/cpu/cpufreq/p4-clockmod.c
@@ -20,6 +20,7 @@
  *
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k7.c b/arch/x86/kernel/cpu/cpufreq/powernow-k7.c
index 0a61159..1305afb 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k7.c
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k7.c
@@ -12,6 +12,7 @@
  * - We disable half multipliers if ACPI is used on A0 stepping CPUs.
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
index 46d4034..d18f9bd 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
@@ -24,6 +24,7 @@
  *     http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/30430.pdf
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/smp.h>
 #include <linux/module.h>
diff --git a/arch/x86/kernel/cpu/cpufreq/sc520_freq.c b/arch/x86/kernel/cpu/cpufreq/sc520_freq.c
index 42da9bd..7da87dd 100644
--- a/arch/x86/kernel/cpu/cpufreq/sc520_freq.c
+++ b/arch/x86/kernel/cpu/cpufreq/sc520_freq.c
@@ -13,6 +13,7 @@
  *	2005-03-30: - initial revision
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
diff --git a/arch/x86/kernel/cpu/cpufreq/speedstep-centrino.c b/arch/x86/kernel/cpu/cpufreq/speedstep-centrino.c
index 908dd34..6fcd23f 100644
--- a/arch/x86/kernel/cpu/cpufreq/speedstep-centrino.c
+++ b/arch/x86/kernel/cpu/cpufreq/speedstep-centrino.c
@@ -13,6 +13,7 @@
  * Copyright (C) 2003 Jeremy Fitzhardinge <jeremy@goop.org>
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
diff --git a/arch/x86/kernel/cpu/cpufreq/speedstep-ich.c b/arch/x86/kernel/cpu/cpufreq/speedstep-ich.c
index 1b50244..43dd80e 100644
--- a/arch/x86/kernel/cpu/cpufreq/speedstep-ich.c
+++ b/arch/x86/kernel/cpu/cpufreq/speedstep-ich.c
@@ -18,6 +18,7 @@
  *                        SPEEDSTEP - DEFINITIONS                    *
  *********************************************************************/
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
diff --git a/arch/x86/kernel/cpu/cpufreq/speedstep-smi.c b/arch/x86/kernel/cpu/cpufreq/speedstep-smi.c
index 8a85c93..8aef4bf 100644
--- a/arch/x86/kernel/cpu/cpufreq/speedstep-smi.c
+++ b/arch/x86/kernel/cpu/cpufreq/speedstep-smi.c
@@ -12,6 +12,7 @@
  *                        SPEEDSTEP - DEFINITIONS                    *
  *********************************************************************/
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 35a26a3..c4b0666 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -15,6 +15,7 @@
  *
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -179,10 +180,7 @@ EXPORT_SYMBOL_GPL(cpufreq_cpu_put);
 /*********************************************************************
  *                     UNIFIED DEBUG HELPERS                         *
  *********************************************************************/
-#ifdef CONFIG_CPU_FREQ_DEBUG
-
-/* what part(s) of the CPUfreq subsystem are debugged? */
-static unsigned int debug;
+#if defined(CONFIG_CPU_FREQ_DEBUG) || defined (CONFIG_DYNAMIC_PRINTK_DEBUG)
 
 /* is the debug output ratelimit'ed using printk_ratelimit? User can
  * set or modify this value.
@@ -215,7 +213,7 @@ static void cpufreq_debug_disable_ratelimit(void)
 	spin_unlock_irqrestore(&disable_ratelimit_lock, flags);
 }
 
-void cpufreq_debug_printk(unsigned int type, const char *prefix,
+void __cpufreq_debug_printk(unsigned int type, const char *prefix,
 							const char *fmt, ...)
 {
 	char s[256];
@@ -224,32 +222,32 @@ void cpufreq_debug_printk(unsigned int type, const char *prefix,
 	unsigned long flags;
 
 	WARN_ON(!prefix);
-	if (type & debug) {
-		spin_lock_irqsave(&disable_ratelimit_lock, flags);
-		if (!disable_ratelimit && debug_ratelimit
-					&& !printk_ratelimit()) {
-			spin_unlock_irqrestore(&disable_ratelimit_lock, flags);
-			return;
-		}
+	spin_lock_irqsave(&disable_ratelimit_lock, flags);
+	if (!disable_ratelimit && debug_ratelimit
+				&& !printk_ratelimit()) {
 		spin_unlock_irqrestore(&disable_ratelimit_lock, flags);
+		return;
+	}	
+	spin_unlock_irqrestore(&disable_ratelimit_lock, flags);
 
-		len = snprintf(s, 256, KERN_DEBUG "%s: ", prefix);
+	len = snprintf(s, 256, KERN_DEBUG "%s: ", prefix);
 
-		va_start(args, fmt);
-		len += vsnprintf(&s[len], (256 - len), fmt, args);
-		va_end(args);
+	va_start(args, fmt);
+	len += vsnprintf(&s[len], (256 - len), fmt, args);
+	va_end(args);
 
-		printk(s);
+	printk(s);
 
-		WARN_ON(len < 5);
-	}
+	WARN_ON(len < 5);
 }
-EXPORT_SYMBOL(cpufreq_debug_printk);
+EXPORT_SYMBOL(__cpufreq_debug_printk);
 
 
-module_param(debug, uint, 0644);
+unsigned int cpufreq_debug;
+EXPORT_SYMBOL_GPL(cpufreq_debug);
+module_param_named(debug, cpufreq_debug, uint, 0644);
 MODULE_PARM_DESC(debug, "CPUfreq debugging: add 1 to debug core,"
-			" 2 to debug drivers, and 4 to debug governors.");
+				" 2 to debug drivers, and 4 to debug governors.");
 
 module_param(debug_ratelimit, uint, 0644);
 MODULE_PARM_DESC(debug_ratelimit, "CPUfreq debugging:"
diff --git a/drivers/cpufreq/cpufreq_performance.c b/drivers/cpufreq/cpufreq_performance.c
index e8e1451..cee0e66 100644
--- a/drivers/cpufreq/cpufreq_performance.c
+++ b/drivers/cpufreq/cpufreq_performance.c
@@ -10,6 +10,7 @@
  *
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/cpufreq.h>
diff --git a/drivers/cpufreq/cpufreq_powersave.c b/drivers/cpufreq/cpufreq_powersave.c
index 13fe06b..4680ccc 100644
--- a/drivers/cpufreq/cpufreq_powersave.c
+++ b/drivers/cpufreq/cpufreq_powersave.c
@@ -10,6 +10,7 @@
  *
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/cpufreq.h>
diff --git a/drivers/cpufreq/cpufreq_userspace.c b/drivers/cpufreq/cpufreq_userspace.c
index cb2ac01..8a33636 100644
--- a/drivers/cpufreq/cpufreq_userspace.c
+++ b/drivers/cpufreq/cpufreq_userspace.c
@@ -11,6 +11,7 @@
  *
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/smp.h>
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index ae6cd60..506cc33 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2002 - 2003 Dominik Brodowski
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index ddd8652..74ce70f 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -366,14 +366,19 @@ void cpufreq_frequency_table_put_attr(unsigned int cpu);
 #define CPUFREQ_DEBUG_DRIVER	2
 #define CPUFREQ_DEBUG_GOVERNOR	4
 
-#ifdef CONFIG_CPU_FREQ_DEBUG
+#define cpufreq_debug_printk(flag, name, msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, flag, cpufreq_debug)) \
+		__cpufreq_debug_printk(flag, name, msg); \
+	} while (0)
 
-extern void cpufreq_debug_printk(unsigned int type, const char *prefix, 
-				 const char *fmt, ...);
+#if defined(CONFIG_CPU_FREQ_DEBUG) || defined (CONFIG_DYNAMIC_PRINTK_DEBUG)
 
+extern unsigned int cpufreq_debug;
+extern void __cpufreq_debug_printk(unsigned int type, const char *prefix, 
+				 const char *fmt, ...);
 #else
 
-#define cpufreq_debug_printk(msg...) do { } while(0)
+#define __cpufreq_debug_printk(msg...) do { } while(0)
 
 #endif /* CONFIG_CPU_FREQ_DEBUG */
 
diff --git a/include/linux/dynamic_debug_cpufreq.h b/include/linux/dynamic_debug_cpufreq.h
new file mode 100644
index 0000000..b0cff6d
--- /dev/null
+++ b/include/linux/dynamic_debug_cpufreq.h
@@ -0,0 +1,8 @@
+#define DYNAMIC_DEBUG_NUM_FLAGS "3"
+#define DYNAMIC_DEBUG_FLAG_NAMES "CPUFREQ_DEBUG_CORE,CPUFREQ_DEBUG_DRIVER,CPUFREQ_DEBUG_GOVERNOR"
+#define DYNAMIC_DEBUG_TYPE "2"
+#define DYNAMIC_DEBUG_MODNAME "cpufreq_shared"
+
+#ifdef CONFIG_CPU_FREQ_DEBUG
+#define DEBUG 1
+#endif

next prev parent reply	other threads:[~2008-07-17 21:06 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-15 21:36 [PATCH 6/7] dynamic debug v2 - convert cpufreq Jason Baron
2008-07-15 23:07 ` Dominik Brodowski
2008-07-17 21:05   ` Jason Baron [this message]
2008-07-17 21:15     ` Greg KH
2008-07-17 21:46       ` Jason Baron
2008-07-17 22:20         ` Dominik Brodowski
2008-07-17 21:27     ` Dominik Brodowski

find likely ancestor, descendant, or conflicting patches for this message:
( dfblob:e2d870d dfblob:549edbd dfblob:f03e915 dfblob:3eb343d
dfblob:9d9eae8 dfblob:212204f dfblob:06fcce5 dfblob:b910935
dfblob:af4a867 dfblob:11ce4fa dfblob:199e4e0 dfblob:868cda4
dfblob:0a61159 dfblob:1305afb dfblob:46d4034 dfblob:d18f9bd
dfblob:42da9bd dfblob:7da87dd dfblob:908dd34 dfblob:6fcd23f
dfblob:1b50244 dfblob:43dd80e dfblob:8a85c93 dfblob:8aef4bf
dfblob:35a26a3 dfblob:c4b0666 dfblob:e8e1451 dfblob:cee0e66
dfblob:13fe06b dfblob:4680ccc dfblob:cb2ac01 dfblob:8a33636
dfblob:ae6cd60 dfblob:506cc33 dfblob:ddd8652 dfblob:74ce70f
dfblob:b0cff6d )
 OR (
bs:"Re: [PATCH 6/7] dynamic debug v2 - convert cpufreq" )
	(help)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080717210531.GA13252@redhat.com \
    --to=jbaron@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=greg@kroah.com \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@dominikbrodowski.net \
    --cc=nick@nick-andrew.net \
    --cc=randy.dunlap@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Be sure your reply has a Subject: header at the top and a blank line before the message body.

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox