| Message ID | 7ee60f3b750f4500f9bdcb64f358acbf17987036.1331856300.git.joe@perches.com |
|---|---|
| State | Rejected, archived |
| Headers | show |
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
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
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
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
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
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
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 \
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(-)