2012-10-17 05:21:32

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH 1/2] cpufreq: Improve debug prints

With debug options on, it is difficult to locate cpufreq core's debug prints.
Fix this by prefixing debug prints with:

"cpufreq: "

Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/cpufreq/cpufreq.c | 2 ++
drivers/cpufreq/cpufreq_performance.c | 2 ++
drivers/cpufreq/cpufreq_powersave.c | 2 ++
drivers/cpufreq/cpufreq_userspace.c | 2 ++
drivers/cpufreq/freq_table.c | 2 ++
5 files changed, 10 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index db6e337..bcbc99d 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -15,6 +15,8 @@
*
*/

+#define pr_fmt(fmt) "cpufreq: " fmt
+
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>
diff --git a/drivers/cpufreq/cpufreq_performance.c b/drivers/cpufreq/cpufreq_performance.c
index f13a8a9..67e3232 100644
--- a/drivers/cpufreq/cpufreq_performance.c
+++ b/drivers/cpufreq/cpufreq_performance.c
@@ -10,6 +10,8 @@
*
*/

+#define pr_fmt(fmt) "cpufreq: " fmt
+
#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 4c2eb51..eb974b6 100644
--- a/drivers/cpufreq/cpufreq_powersave.c
+++ b/drivers/cpufreq/cpufreq_powersave.c
@@ -10,6 +10,8 @@
*
*/

+#define pr_fmt(fmt) "cpufreq: " fmt
+
#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 bedac1a..6d7ccdb 100644
--- a/drivers/cpufreq/cpufreq_userspace.c
+++ b/drivers/cpufreq/cpufreq_userspace.c
@@ -11,6 +11,8 @@
*
*/

+#define pr_fmt(fmt) "cpufreq: " fmt
+
#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 90431cb..246949c 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -9,6 +9,8 @@
*
*/

+#define pr_fmt(fmt) "cpufreq: " fmt
+
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>
--
1.7.12.rc2.18.g61b472e


2012-10-17 05:21:43

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH 2/2] cpufreq: Debugging options for the cpufreq subsystem

This adds Kconfig options for DEBUG and VERBOSE_DEBUG to the cpufreq subsystem,
This is pretty useful for developers who want to debug cpufreq subsystem and
don't want to editing the Makefile manually each time they want to debug.

Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/cpufreq/Kconfig | 14 ++++++++++++++
drivers/cpufreq/Makefile | 4 ++++
2 files changed, 18 insertions(+)

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index e24a2a1..b9ee95c 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -17,6 +17,20 @@ config CPU_FREQ

if CPU_FREQ

+config CPUFREQ_DEBUG
+ bool "cpufreq debugging"
+ help
+ This is an option for use by developers; most people should say N
+ here. This enables cpufreq core and debugging.
+
+config CPUFREQ_VDEBUG
+ bool "cpufreq verbose debugging"
+ depends on CPUFREQ_DEBUG != n
+ help
+ This is an option for use by developers; most people should say N
+ here. This enables deeper (more verbose) debugging of the cpufreq
+ core and drivers.
+
config CPU_FREQ_TABLE
tristate

diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index fe4cd26..df4cfc8 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -1,3 +1,7 @@
+# CPUfreq core & drivers debugging
+ccflags-$(CONFIG_CPUFREQ_DEBUG) := -DDEBUG
+ccflags-$(CONFIG_CPUFREQ_VDEBUG) += -DVERBOSE_DEBUG
+
# CPUfreq core
obj-$(CONFIG_CPU_FREQ) += cpufreq.o
# CPUfreq stats
--
1.7.12.rc2.18.g61b472e

2012-10-17 05:39:37

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Improve debug prints

On Wed, 2012-10-17 at 10:50 +0530, Viresh Kumar wrote:
> With debug options on, it is difficult to locate cpufreq core's debug prints.
[]
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
[]
> @@ -15,6 +15,8 @@
> +#define pr_fmt(fmt) "cpufreq: " fmt

I'd prefer that

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

be used for all of these.

2012-10-17 05:55:21

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Improve debug prints

On 17 October 2012 11:09, Joe Perches <[email protected]> wrote:
> On Wed, 2012-10-17 at 10:50 +0530, Viresh Kumar wrote:
>> With debug options on, it is difficult to locate cpufreq core's debug prints.
> []
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> []
>> @@ -15,6 +15,8 @@
>> +#define pr_fmt(fmt) "cpufreq: " fmt
>
> I'd prefer that
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> be used for all of these.

Hi Joe,

Sorry, i am not sure how KBUILD_MODNAME is compiled by kernel.

If i am not wrong KBUILD_MODNAME would work only for drivers that can
be added as module? If so, then it may not apply here, as most of the stuff
i have updated doesn't call module_init().

So, if i try KBUILD_MODNAME with my patch i get prints as:

KBUILD_MODNAME: <print - text>

--
viresh

2012-10-17 06:04:39

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Improve debug prints

On Wed, 2012-10-17 at 11:25 +0530, Viresh Kumar wrote:
> On 17 October 2012 11:09, Joe Perches <[email protected]> wrote:
> > On Wed, 2012-10-17 at 10:50 +0530, Viresh Kumar wrote:
> >> With debug options on, it is difficult to locate cpufreq core's debug prints.
> > []
> >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > []
> >> @@ -15,6 +15,8 @@
> >> +#define pr_fmt(fmt) "cpufreq: " fmt
[]
> > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

> Sorry, i am not sure how KBUILD_MODNAME is compiled by kernel.

KBUILD_MODNAME is a #define that is set by scripts/Makefile.lib.

> So, if i try KBUILD_MODNAME with my patch i get prints as:
>
> KBUILD_MODNAME: <print - text>

I believe you are quoting KBUILD_MODNAME

try

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

This should create prefixes like
"cpufreq: "
and
"cpufreq_performance: "

2012-10-17 06:17:10

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Improve debug prints

On 17 October 2012 11:34, Joe Perches <[email protected]> wrote:
> I believe you are quoting KBUILD_MODNAME

Yes. :(
Far better output with this. Thanks.

@Rafael: Please consider below patch instead:

From: Viresh Kumar <[email protected]>
Date: Wed, 17 Oct 2012 10:38:31 +0530
Subject: [PATCH] cpufreq: Improve debug prints

With debug options on, it is difficult to locate cpufreq core's debug prints.
Fix this by prefixing debug prints with KBUILD_MODNAME.

Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/cpufreq/cpufreq.c | 2 ++
drivers/cpufreq/cpufreq_performance.c | 2 ++
drivers/cpufreq/cpufreq_powersave.c | 2 ++
drivers/cpufreq/cpufreq_userspace.c | 2 ++
drivers/cpufreq/freq_table.c | 2 ++
5 files changed, 10 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index db6e337..0504b8b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -15,6 +15,8 @@
*
*/

+#define pr_fmt(fmt) KBUILD_MODNAME": " fmt
+
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>
diff --git a/drivers/cpufreq/cpufreq_performance.c
b/drivers/cpufreq/cpufreq_performance.c
index f13a8a9..57951dc 100644
--- a/drivers/cpufreq/cpufreq_performance.c
+++ b/drivers/cpufreq/cpufreq_performance.c
@@ -10,6 +10,8 @@
*
*/

+#define pr_fmt(fmt) KBUILD_MODNAME": " fmt
+
#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 4c2eb51..c4540cf 100644
--- a/drivers/cpufreq/cpufreq_powersave.c
+++ b/drivers/cpufreq/cpufreq_powersave.c
@@ -10,6 +10,8 @@
*
*/

+#define pr_fmt(fmt) KBUILD_MODNAME": " fmt
+
#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 bedac1a..c0eb546 100644
--- a/drivers/cpufreq/cpufreq_userspace.c
+++ b/drivers/cpufreq/cpufreq_userspace.c
@@ -11,6 +11,8 @@
*
*/

+#define pr_fmt(fmt) KBUILD_MODNAME": " fmt
+
#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 90431cb..8bcc930 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -9,6 +9,8 @@
*
*/

+#define pr_fmt(fmt) KBUILD_MODNAME": " fmt
+
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>

2012-10-17 06:26:56

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Improve debug prints

On Wed, 2012-10-17 at 11:47 +0530, Viresh Kumar wrote:
> On 17 October 2012 11:34, Joe Perches <[email protected]> wrote:
> > I believe you are quoting KBUILD_MODNAME
>
> Yes. :(
> Far better output with this. Thanks.

Oh good, but please use a space between KBUILD_MODNAME
and the quoted ": ".

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

look at:
$ git grep -E -h "pr_fmt.*KBUILD_MODNAME" *|sort|uniq -c|sort -rn

2012-10-17 06:33:34

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Improve debug prints

On 17 October 2012 11:56, Joe Perches <[email protected]> wrote:
> Oh good, but please use a space between KBUILD_MODNAME
> and the quoted ": ".

Anything technical behind it or just for code formatting? As output is
same in both cases :)

------------------------8<----------------------8<-------------------

From: Viresh Kumar <[email protected]>
Date: Wed, 17 Oct 2012 10:38:31 +0530
Subject: [PATCH] cpufreq: Improve debug prints

With debug options on, it is difficult to locate cpufreq core's debug prints.
Fix this by prefixing debug prints with KBUILD_MODNAME.

Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/cpufreq/cpufreq.c | 2 ++
drivers/cpufreq/cpufreq_performance.c | 2 ++
drivers/cpufreq/cpufreq_powersave.c | 2 ++
drivers/cpufreq/cpufreq_userspace.c | 2 ++
drivers/cpufreq/freq_table.c | 2 ++
5 files changed, 10 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index db6e337..85df538 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -15,6 +15,8 @@
*
*/

+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>
diff --git a/drivers/cpufreq/cpufreq_performance.c
b/drivers/cpufreq/cpufreq_performance.c
index f13a8a9..ceee068 100644
--- a/drivers/cpufreq/cpufreq_performance.c
+++ b/drivers/cpufreq/cpufreq_performance.c
@@ -10,6 +10,8 @@
*
*/

+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#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 4c2eb51..2d948a1 100644
--- a/drivers/cpufreq/cpufreq_powersave.c
+++ b/drivers/cpufreq/cpufreq_powersave.c
@@ -10,6 +10,8 @@
*
*/

+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#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 bedac1a..c8c3d29 100644
--- a/drivers/cpufreq/cpufreq_userspace.c
+++ b/drivers/cpufreq/cpufreq_userspace.c
@@ -11,6 +11,8 @@
*
*/

+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#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 90431cb..49cda25 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -9,6 +9,8 @@
*
*/

+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>

2012-10-17 06:39:16

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Improve debug prints

On Wed, 2012-10-17 at 12:03 +0530, Viresh Kumar wrote:
> On 17 October 2012 11:56, Joe Perches <[email protected]> wrote:
> > Oh good, but please use a space between KBUILD_MODNAME
> > and the quoted ": ".
>
> Anything technical behind it or just for code formatting?

Just code formatting though it makes it easier to remove all
these unnecessary defines when it becomes the default prefix
some year soon.

cheers, Joe

2012-10-17 09:21:05

by Sudeep KarkadaNagesha

[permalink] [raw]
Subject: Re: [PATCH 2/2] cpufreq: Debugging options for the cpufreq subsystem

On 17/10/12 06:20, Viresh Kumar wrote:
> This adds Kconfig options for DEBUG and VERBOSE_DEBUG to the cpufreq subsystem,
> This is pretty useful for developers who want to debug cpufreq subsystem and
> don't want to editing the Makefile manually each time they want to debug.
>
You can easily overcome this issue using dynamic debugging.
You need not recompile the code enabling debug if CONFIG_DYNAMIC_DEBUG
is enabled.

Regards,
Sudeep

2012-10-17 09:29:47

by Sudeep KarkadaNagesha

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Improve debug prints

On 17/10/12 06:20, Viresh Kumar wrote:
> With debug options on, it is difficult to locate cpufreq core's debug prints.
> Fix this by prefixing debug prints with:
>
> "cpufreq:"

With CONFIG_DYNAMIC_DEBUG, you can control(enable/disable) debug
prints at different levels (file, module, line, function)

Regards,
Sudeep

2012-10-17 09:40:01

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Improve debug prints

On 17 October 2012 14:55, Sudeep KarkadaNagesha
<[email protected]> wrote:
> With CONFIG_DYNAMIC_DEBUG, you can control(enable/disable) debug
> prints at different levels (file, module, line, function)

Quickly went through this :)
http://www.kernel.org/doc/ols/2009/ols2009-pages-39-46.pdf

My usecase is a bit different. I want to see all bootprints with
cpufreq pr_debug
prints. So, this prefixing will help there.

Also, i am not sure if dynamic debug will help in boot prints too?

--
viresh

2012-10-17 09:58:09

by Sudeep KarkadaNagesha

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Improve debug prints

On 17/10/12 10:39, Viresh Kumar wrote:
> On 17 October 2012 14:55, Sudeep KarkadaNagesha
> <[email protected]> wrote:
>> With CONFIG_DYNAMIC_DEBUG, you can control(enable/disable) debug
>> prints at different levels (file, module, line, function)
>
> Quickly went through this :)
> http://www.kernel.org/doc/ols/2009/ols2009-pages-39-46.pdf
>
May be kernel/Documentation/dynamic-debug-howto.txt is more up-to-date.

> My usecase is a bit different. I want to see all bootprints with
> cpufreq pr_debug
> prints. So, this prefixing will help there.
>
> Also, i am not sure if dynamic debug will help in boot prints too?
The document covers "Debug messages during Boot Process" even though I
have never tried :)

Regards,
Sudeep

2012-10-17 10:23:40

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Improve debug prints

On 17 October 2012 15:28, Sudeep KarkadaNagesha
<[email protected]> wrote:
> May be kernel/Documentation/dynamic-debug-howto.txt is more up-to-date.

yes.

>> My usecase is a bit different. I want to see all bootprints with
>> cpufreq pr_debug
>> prints. So, this prefixing will help there.

> The document covers "Debug messages during Boot Process" even though I have
> never tried :)

Looks like it does. That's good.
But this patch is anyway required, because of the earlier mentioned reason.

--
viresh

2012-10-24 21:56:15

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Improve debug prints

On Wednesday 17 of October 2012 12:03:31 Viresh Kumar wrote:
> On 17 October 2012 11:56, Joe Perches <[email protected]> wrote:
> > Oh good, but please use a space between KBUILD_MODNAME
> > and the quoted ": ".
>
> Anything technical behind it or just for code formatting? As output is
> same in both cases :)
>
> ------------------------8<----------------------8<-------------------
>
> From: Viresh Kumar <[email protected]>
> Date: Wed, 17 Oct 2012 10:38:31 +0530
> Subject: [PATCH] cpufreq: Improve debug prints
>
> With debug options on, it is difficult to locate cpufreq core's debug prints.
> Fix this by prefixing debug prints with KBUILD_MODNAME.
>
> Signed-off-by: Viresh Kumar <[email protected]>


Applied to linux-pm.git/linux-next as v3.8 material.

Thanks,
Rafael


> ---
> drivers/cpufreq/cpufreq.c | 2 ++
> drivers/cpufreq/cpufreq_performance.c | 2 ++
> drivers/cpufreq/cpufreq_powersave.c | 2 ++
> drivers/cpufreq/cpufreq_userspace.c | 2 ++
> drivers/cpufreq/freq_table.c | 2 ++
> 5 files changed, 10 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index db6e337..85df538 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -15,6 +15,8 @@
> *
> */
>
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/init.h>
> diff --git a/drivers/cpufreq/cpufreq_performance.c
> b/drivers/cpufreq/cpufreq_performance.c
> index f13a8a9..ceee068 100644
> --- a/drivers/cpufreq/cpufreq_performance.c
> +++ b/drivers/cpufreq/cpufreq_performance.c
> @@ -10,6 +10,8 @@
> *
> */
>
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> #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 4c2eb51..2d948a1 100644
> --- a/drivers/cpufreq/cpufreq_powersave.c
> +++ b/drivers/cpufreq/cpufreq_powersave.c
> @@ -10,6 +10,8 @@
> *
> */
>
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> #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 bedac1a..c8c3d29 100644
> --- a/drivers/cpufreq/cpufreq_userspace.c
> +++ b/drivers/cpufreq/cpufreq_userspace.c
> @@ -11,6 +11,8 @@
> *
> */
>
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> #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 90431cb..49cda25 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -9,6 +9,8 @@
> *
> */
>
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/init.h>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.