lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20080717210531.GA13252@redhat.com>
Date:	Thu, 17 Jul 2008 17:05:32 -0400
From:	Jason Baron <jbaron@...hat.com>
To:	Dominik Brodowski <linux@...inikbrodowski.net>,
	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	joe@...ches.com, greg@...ah.com, nick@...k-andrew.net,
	randy.dunlap@...cle.com
Subject: Re: [PATCH 6/7] dynamic debug v2 - convert cpufreq

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@...p.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



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ