[1/9] ext4: Add -DDEBUG to Makefile

Submitted by Joe Perches on March 16, 2012, 12:07 a.m.

Details

Message ID 7ee60f3b750f4500f9bdcb64f358acbf17987036.1331856300.git.joe@perches.com
State Rejected, archived
Headers show

Commit Message

Joe Perches March 16, 2012, 12:07 a.m.
Add -DDEBUG to enable future use of pr_debug.
No changes to objects as no DEBUG uses currently exist.

Signed-off-by: Joe Perches <joe@perches.com>
---
 fs/ext4/Makefile |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Comments

David Daney March 16, 2012, 4:03 p.m.
On 03/15/2012 05:07 PM, Joe Perches wrote:
> Add -DDEBUG to enable future use of pr_debug.
> No changes to objects as no DEBUG uses currently exist.
>
> Signed-off-by: Joe Perches<joe@perches.com>
> ---
>   fs/ext4/Makefile |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile
> index 56fd8f86..617a5d8 100644
> --- a/fs/ext4/Makefile
> +++ b/fs/ext4/Makefile
> @@ -2,6 +2,8 @@
>   # Makefile for the linux ext4-filesystem routines.
>   #
>
> +ccflags-$(CONFIG_EXT4_FS) := -DDEBUG

In many other susbsystems/drivers, the definition of DEBUG is gated by a 
separate Kconfig symbol used to select debugging just for that 
susbsystem/driver (see CONFIG_MMC_DEBUG for example).

Why aren't you doing the same here?

David Daney
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches March 16, 2012, 4:29 p.m.
On Fri, 2012-03-16 at 09:03 -0700, David Daney wrote:
> On 03/15/2012 05:07 PM, Joe Perches wrote:
> > Add -DDEBUG to enable future use of pr_debug.
> > No changes to objects as no DEBUG uses currently exist.
[]
> > diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile
[]
> > +ccflags-$(CONFIG_EXT4_FS) := -DDEBUG

> In many other susbsystems/drivers, the definition of DEBUG is gated by a 
> separate Kconfig symbol used to select debugging just for that 
> susbsystem/driver (see CONFIG_MMC_DEBUG for example).
> 
> Why aren't you doing the same here?

It can be done later with an additional symbol if
desired.

For now the current code uses printk(KERN_DEBUG and a
straight conversion to pr_debug would eliminate the
KERN_DEBUG the output.

pr_debug without #define DEBUG or dynamic_debug is
compiled away to nothing.


--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o March 19, 2012, 4:39 a.m.
On Fri, Mar 16, 2012 at 09:03:57AM -0700, David Daney wrote:
> 
> In many other susbsystems/drivers, the definition of DEBUG is gated
> by a separate Kconfig symbol used to select debugging just for that
> susbsystem/driver (see CONFIG_MMC_DEBUG for example).

In ext4 we have many different debugging patches, and in general you'd
never want to enable them all at once (you'd get way too much noise).
The debugging statements are there when a developer is debugging very
specific section of code (i.e., the directory index, or the extent
tree, or the block allocator, etc.).  It's really only ext4 developers
who need to use those debugging statements, and even for them it's
quite rare.

So I've never considered it worthwhile to enable them via a CONFIG_*
menu item; developers who are debugging a specific problem will
generally just drop in the specific #define on an ad hoc basis, and
that works fine.

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches March 19, 2012, 4:26 p.m.
On Mon, 2012-03-19 at 00:39 -0400, Ted Ts'o wrote:
> So I've never considered it worthwhile to enable them via a CONFIG_*
> menu item; developers who are debugging a specific problem will
> generally just drop in the specific #define on an ad hoc basis, and
> that works fine.

CONFIG_EXT4_DEBUG_MASK=value and/or a module_param(debug) is
possible to add but as Ted said, it's not really too valuable.

dynamic_debugging the always compiled-in current KERN_DEBUG
or eliminating them altogether might have some value though.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
ddaney.cavm@gmail.com March 19, 2012, 6:48 p.m.
On 03/18/2012 09:39 PM, Ted Ts'o wrote:
> On Fri, Mar 16, 2012 at 09:03:57AM -0700, David Daney wrote:
>>
>> In many other susbsystems/drivers, the definition of DEBUG is gated
>> by a separate Kconfig symbol used to select debugging just for that
>> susbsystem/driver (see CONFIG_MMC_DEBUG for example).
>
> In ext4 we have many different debugging patches, and in general you'd
> never want to enable them all at once (you'd get way too much noise).
> The debugging statements are there when a developer is debugging very
> specific section of code (i.e., the directory index, or the extent
> tree, or the block allocator, etc.).  It's really only ext4 developers
> who need to use those debugging statements, and even for them it's
> quite rare.
>
> So I've never considered it worthwhile to enable them via a CONFIG_*
> menu item; developers who are debugging a specific problem will
> generally just drop in the specific #define on an ad hoc basis, and
> that works fine.
>

If you want to unconditionally define DEBUG for all of ext4, that is 
fine with me.

I was just noting that it seemed a little odd to me, and different from 
what is done elsewhere.

Thanks,
David Daney
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o March 20, 2012, 1:05 a.m.
On Mon, Mar 19, 2012 at 11:48:12AM -0700, David Daney wrote:
> 
> If you want to unconditionally define DEBUG for all of ext4, that is
> fine with me.

Since I don't see the value of pr_debug, and don't plan to use it, I
don't plan to unconditionally define DEBUG.  :-)

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch hide | download patch | download mbox

diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile
index 56fd8f86..617a5d8 100644
--- a/fs/ext4/Makefile
+++ b/fs/ext4/Makefile
@@ -2,6 +2,8 @@ 
 # Makefile for the linux ext4-filesystem routines.
 #
 
+ccflags-$(CONFIG_EXT4_FS) := -DDEBUG
+
 obj-$(CONFIG_EXT4_FS) += ext4.o
 
 ext4-y	:= balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o page-io.o \