2007-06-08 04:45:57

by Jose R. Santos

[permalink] [raw]
Subject: [PATCH] JBD2: Change debug file path

The jbd2-debug file used to be located in /proc/sys/fs/jbd2-debug, but
create_proc_entry() does not do lookups on file names with more that one
directory deep. This causes the entry creation to fail and hence, no proc
file is created. This patch moves the file to /proc/jbd2-degug.

The file could be move to /proc/fs/jbd2/jbd2-debug, but it would require
some minor alterations to the jbd-stats patch.

This is also broken on the original jbd code.

Signed-off-by: Jose R. Santos <[email protected]>
---
Index: linux-2.6.22-rc3/fs/Kconfig
===================================================================
--- linux-2.6.22-rc3.orig/fs/Kconfig 2007-05-25 21:55:14.000000000 -0500
+++ linux-2.6.22-rc3/fs/Kconfig 2007-06-07 23:17:54.110109520 -0500
@@ -260,10 +260,10 @@ config JBD2_DEBUG
By default, the debugging output will be turned off.

If you select Y here, then you will be able to turn on debugging
- with "echo N > /proc/sys/fs/jbd2-debug", where N is a number between
+ with "echo N > /proc/jbd2-debug", where N is a number between
1 and 5. The higher the number, the more debugging output is
generated. To turn debugging off again, do
- "echo 0 > /proc/sys/fs/jbd2-debug".
+ "echo 0 > /proc/jbd2-debug".

config FS_MBCACHE
# Meta block cache for Extended Attributes (ext2/ext3/ext4)
Index: linux-2.6.22-rc3/fs/jbd2/journal.c
===================================================================
--- linux-2.6.22-rc3.orig/fs/jbd2/journal.c 2007-06-07 23:10:58.838650182 -0500
+++ linux-2.6.22-rc3/fs/jbd2/journal.c 2007-06-07 23:17:54.113442624 -0500
@@ -2288,7 +2288,7 @@ static int write_jbd_debug(struct file *
return count;
}

-#define JBD_PROC_NAME "sys/fs/jbd2-debug"
+#define JBD_PROC_NAME "jbd2-debug"

static void __init create_jbd_proc_entry(void)
{



-JRS


2007-06-08 07:08:14

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH] JBD2: Change debug file path

On Jun 07, 2007 23:45 -0500, Jose R. Santos wrote:
> The jbd2-debug file used to be located in /proc/sys/fs/jbd2-debug, but
> create_proc_entry() does not do lookups on file names with more that one
> directory deep. This causes the entry creation to fail and hence, no proc
> file is created. This patch moves the file to /proc/jbd2-degug.
>
> The file could be move to /proc/fs/jbd2/jbd2-debug, but it would require
> some minor alterations to the jbd-stats patch.

I don't think we really want to be adding top-level files in /proc.
What about using the "debugfs" filesystem (not to be confused with
the e2fsprogs 'debugfs' command)?

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

2007-06-08 12:02:11

by Jose R. Santos

[permalink] [raw]
Subject: Re: [PATCH] JBD2: Change debug file path

On Fri, 8 Jun 2007 01:08:12 -0600
Andreas Dilger <[email protected]> wrote:

> On Jun 07, 2007 23:45 -0500, Jose R. Santos wrote:
> > The jbd2-debug file used to be located in /proc/sys/fs/jbd2-debug, but
> > create_proc_entry() does not do lookups on file names with more that one
> > directory deep. This causes the entry creation to fail and hence, no proc
> > file is created. This patch moves the file to /proc/jbd2-degug.
> >
> > The file could be move to /proc/fs/jbd2/jbd2-debug, but it would require
> > some minor alterations to the jbd-stats patch.
>
> I don't think we really want to be adding top-level files in /proc.
> What about using the "debugfs" filesystem (not to be confused with
> the e2fsprogs 'debugfs' command)?

I agree. Was feeling kind of lazy though and didn't want to to much
time on this issue. :)

I'll look into changing jbd and jbd2 into using debugfs.

-JRS

2007-06-09 03:58:18

by Jose R. Santos

[permalink] [raw]
Subject: Re: [PATCH][RFC] JBD2: Use DebugFS for jbd2 debug config option.

On Fri, 8 Jun 2007 01:08:12 -0600
Andreas Dilger <[email protected]> wrote:

> On Jun 07, 2007 23:45 -0500, Jose R. Santos wrote:
> > The jbd2-debug file used to be located in /proc/sys/fs/jbd2-debug, but
> > create_proc_entry() does not do lookups on file names with more that one
> > directory deep. This causes the entry creation to fail and hence, no proc
> > file is created. This patch moves the file to /proc/jbd2-degug.
> >
> > The file could be move to /proc/fs/jbd2/jbd2-debug, but it would require
> > some minor alterations to the jbd-stats patch.
>
> I don't think we really want to be adding top-level files in /proc.
> What about using the "debugfs" filesystem (not to be confused with
> the e2fsprogs 'debugfs' command)?

How about this then? Moved the file to use debugfs as well as having
the nice effect of removing more lines than what it adds.

Signed-off-by: Jose R. Santos <[email protected]>
---
fs/jbd2/journal.c | 62 20 + 42 - 0 !
include/linux/jbd2.h | 2 1 + 1 - 0 !
2 files changed, 21 insertions(+), 43 deletions(-)

Index: linux-2.6.22-rc4/fs/jbd2/journal.c
===================================================================
--- linux-2.6.22-rc4.orig/fs/jbd2/journal.c 2007-06-08 15:03:14.353372404 -0500
+++ linux-2.6.22-rc4/fs/jbd2/journal.c 2007-06-08 22:44:04.476375738 -0500
@@ -36,6 +36,7 @@
#include <linux/poison.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#include <linux/debugfs.h>

#include <asm/uaccess.h>
#include <asm/page.h>
@@ -2256,60 +2257,37 @@ void jbd2_journal_put_journal_head(struc
* /proc tunables
*/
#if defined(CONFIG_JBD2_DEBUG)
-int jbd2_journal_enable_debug;
+u16 jbd2_journal_enable_debug;
EXPORT_SYMBOL(jbd2_journal_enable_debug);
#endif

-#if defined(CONFIG_JBD2_DEBUG) && defined(CONFIG_PROC_FS)
+#if defined(CONFIG_JBD2_DEBUG) && defined(CONFIG_DEBUG_FS)

-static struct proc_dir_entry *proc_jbd_debug;
+#define JBD2_DEBUG_NAME "jbd2-debug"

-static int read_jbd_debug(char *page, char **start, off_t off,
- int count, int *eof, void *data)
-{
- int ret;
-
- ret = sprintf(page + off, "%d\n", jbd2_journal_enable_debug);
- *eof = 1;
- return ret;
-}
+struct dentry *jbd2_debugfs_dir, *jbd2_debug;

-static int write_jbd_debug(struct file *file, const char __user *buffer,
- unsigned long count, void *data)
+static void __init jbd2_create_debugfs_entry(void)
{
- char buf[32];
-
- if (count > ARRAY_SIZE(buf) - 1)
- count = ARRAY_SIZE(buf) - 1;
- if (copy_from_user(buf, buffer, count))
- return -EFAULT;
- buf[ARRAY_SIZE(buf) - 1] = '\0';
- jbd2_journal_enable_debug = simple_strtoul(buf, NULL, 10);
- return count;
-}
-
-#define JBD_PROC_NAME "sys/fs/jbd2-debug"
-
-static void __init create_jbd_proc_entry(void)
-{
- proc_jbd_debug = create_proc_entry(JBD_PROC_NAME, 0644, NULL);
- if (proc_jbd_debug) {
- /* Why is this so hard? */
- proc_jbd_debug->read_proc = read_jbd_debug;
- proc_jbd_debug->write_proc = write_jbd_debug;
- }
+ jbd2_debugfs_dir = debugfs_create_dir("jbd2", NULL);
+ if (jbd2_debugfs_dir)
+ jbd2_debug = debugfs_create_u16(JBD2_DEBUG_NAME, S_IRUGO,
+ jbd2_debugfs_dir,
+ &jbd2_journal_enable_debug);
}

-static void __exit jbd2_remove_jbd_proc_entry(void)
+static void __exit jbd2_remove_debugfs_entry(void)
{
- if (proc_jbd_debug)
- remove_proc_entry(JBD_PROC_NAME, NULL);
+ if (jbd2_debug)
+ debugfs_remove(jbd2_debug);
+ if (jbd2_debugfs_dir)
+ debugfs_remove(jbd2_debugfs_dir);
}

#else

-#define create_jbd_proc_entry() do {} while (0)
-#define jbd2_remove_jbd_proc_entry() do {} while (0)
+#define jbd2_create_debugfs_entry() do {} while (0)
+#define jbd2_remove_debugfs_entry() do {} while (0)

#endif

@@ -2391,7 +2369,7 @@ static int __init journal_init(void)
ret = journal_init_caches();
if (ret != 0)
jbd2_journal_destroy_caches();
- create_jbd_proc_entry();
+ jbd2_create_debugfs_entry();
jbd2_create_jbd_stats_proc_entry();
return ret;
}
@@ -2403,7 +2381,7 @@ static void __exit journal_exit(void)
if (n)
printk(KERN_EMERG "JBD: leaked %d journal_heads!\n", n);
#endif
- jbd2_remove_jbd_proc_entry();
+ jbd2_remove_debugfs_entry();
jbd2_remove_jbd_stats_proc_entry();
jbd2_journal_destroy_caches();
}
Index: linux-2.6.22-rc4/include/linux/jbd2.h
===================================================================
--- linux-2.6.22-rc4.orig/include/linux/jbd2.h 2007-06-08 18:01:41.027527067 -0500
+++ linux-2.6.22-rc4/include/linux/jbd2.h 2007-06-08 18:02:27.351010026 -0500
@@ -57,7 +57,7 @@
* CONFIG_JBD2_DEBUG is on.
*/
#define JBD_EXPENSIVE_CHECKING
-extern int jbd2_journal_enable_debug;
+extern u16 jbd2_journal_enable_debug;

#define jbd_debug(n, f, a...) \
do { \



-JRS

2007-06-11 22:04:48

by Mingming Cao

[permalink] [raw]
Subject: Re: [PATCH][RFC] JBD2: Use DebugFS for jbd2 debug config option.

On Fri, 2007-06-08 at 22:57 -0500, Jose R. Santos wrote:
> On Fri, 8 Jun 2007 01:08:12 -0600
> Andreas Dilger <[email protected]> wrote:
>
> > On Jun 07, 2007 23:45 -0500, Jose R. Santos wrote:
> > > The jbd2-debug file used to be located in /proc/sys/fs/jbd2-debug, but
> > > create_proc_entry() does not do lookups on file names with more that one
> > > directory deep. This causes the entry creation to fail and hence, no proc
> > > file is created. This patch moves the file to /proc/jbd2-degug.
> > >
> > > The file could be move to /proc/fs/jbd2/jbd2-debug, but it would require
> > > some minor alterations to the jbd-stats patch.
> >
> > I don't think we really want to be adding top-level files in /proc.
> > What about using the "debugfs" filesystem (not to be confused with
> > the e2fsprogs 'debugfs' command)?
>
> How about this then? Moved the file to use debugfs as well as having
> the nice effect of removing more lines than what it adds.
>
Shall we do this for ext3 as well?

Mingming
> Signed-off-by: Jose R. Santos <[email protected]>
> ---
> fs/jbd2/journal.c | 62 20 + 42 - 0 !
> include/linux/jbd2.h | 2 1 + 1 - 0 !
> 2 files changed, 21 insertions(+), 43 deletions(-)
>
> Index: linux-2.6.22-rc4/fs/jbd2/journal.c
> ===================================================================
> --- linux-2.6.22-rc4.orig/fs/jbd2/journal.c 2007-06-08 15:03:14.353372404 -0500
> +++ linux-2.6.22-rc4/fs/jbd2/journal.c 2007-06-08 22:44:04.476375738 -0500
> @@ -36,6 +36,7 @@
> #include <linux/poison.h>
> #include <linux/proc_fs.h>
> #include <linux/seq_file.h>
> +#include <linux/debugfs.h>
>
> #include <asm/uaccess.h>
> #include <asm/page.h>
> @@ -2256,60 +2257,37 @@ void jbd2_journal_put_journal_head(struc
> * /proc tunables
> */
> #if defined(CONFIG_JBD2_DEBUG)
> -int jbd2_journal_enable_debug;
> +u16 jbd2_journal_enable_debug;
> EXPORT_SYMBOL(jbd2_journal_enable_debug);
> #endif
>
> -#if defined(CONFIG_JBD2_DEBUG) && defined(CONFIG_PROC_FS)
> +#if defined(CONFIG_JBD2_DEBUG) && defined(CONFIG_DEBUG_FS)
>
> -static struct proc_dir_entry *proc_jbd_debug;
> +#define JBD2_DEBUG_NAME "jbd2-debug"
>
> -static int read_jbd_debug(char *page, char **start, off_t off,
> - int count, int *eof, void *data)
> -{
> - int ret;
> -
> - ret = sprintf(page + off, "%d\n", jbd2_journal_enable_debug);
> - *eof = 1;
> - return ret;
> -}
> +struct dentry *jbd2_debugfs_dir, *jbd2_debug;
>
> -static int write_jbd_debug(struct file *file, const char __user *buffer,
> - unsigned long count, void *data)
> +static void __init jbd2_create_debugfs_entry(void)
> {
> - char buf[32];
> -
> - if (count > ARRAY_SIZE(buf) - 1)
> - count = ARRAY_SIZE(buf) - 1;
> - if (copy_from_user(buf, buffer, count))
> - return -EFAULT;
> - buf[ARRAY_SIZE(buf) - 1] = '\0';
> - jbd2_journal_enable_debug = simple_strtoul(buf, NULL, 10);
> - return count;
> -}
> -
> -#define JBD_PROC_NAME "sys/fs/jbd2-debug"
> -
> -static void __init create_jbd_proc_entry(void)
> -{
> - proc_jbd_debug = create_proc_entry(JBD_PROC_NAME, 0644, NULL);
> - if (proc_jbd_debug) {
> - /* Why is this so hard? */
> - proc_jbd_debug->read_proc = read_jbd_debug;
> - proc_jbd_debug->write_proc = write_jbd_debug;
> - }
> + jbd2_debugfs_dir = debugfs_create_dir("jbd2", NULL);
> + if (jbd2_debugfs_dir)
> + jbd2_debug = debugfs_create_u16(JBD2_DEBUG_NAME, S_IRUGO,
> + jbd2_debugfs_dir,
> + &jbd2_journal_enable_debug);
> }
>
> -static void __exit jbd2_remove_jbd_proc_entry(void)
> +static void __exit jbd2_remove_debugfs_entry(void)
> {
> - if (proc_jbd_debug)
> - remove_proc_entry(JBD_PROC_NAME, NULL);
> + if (jbd2_debug)
> + debugfs_remove(jbd2_debug);
> + if (jbd2_debugfs_dir)
> + debugfs_remove(jbd2_debugfs_dir);
> }
>
> #else
>
> -#define create_jbd_proc_entry() do {} while (0)
> -#define jbd2_remove_jbd_proc_entry() do {} while (0)
> +#define jbd2_create_debugfs_entry() do {} while (0)
> +#define jbd2_remove_debugfs_entry() do {} while (0)
>
> #endif
>
> @@ -2391,7 +2369,7 @@ static int __init journal_init(void)
> ret = journal_init_caches();
> if (ret != 0)
> jbd2_journal_destroy_caches();
> - create_jbd_proc_entry();
> + jbd2_create_debugfs_entry();
> jbd2_create_jbd_stats_proc_entry();
> return ret;
> }
> @@ -2403,7 +2381,7 @@ static void __exit journal_exit(void)
> if (n)
> printk(KERN_EMERG "JBD: leaked %d journal_heads!\n", n);
> #endif
> - jbd2_remove_jbd_proc_entry();
> + jbd2_remove_debugfs_entry();
> jbd2_remove_jbd_stats_proc_entry();
> jbd2_journal_destroy_caches();
> }
> Index: linux-2.6.22-rc4/include/linux/jbd2.h
> ===================================================================
> --- linux-2.6.22-rc4.orig/include/linux/jbd2.h 2007-06-08 18:01:41.027527067 -0500
> +++ linux-2.6.22-rc4/include/linux/jbd2.h 2007-06-08 18:02:27.351010026 -0500
> @@ -57,7 +57,7 @@
> * CONFIG_JBD2_DEBUG is on.
> */
> #define JBD_EXPENSIVE_CHECKING
> -extern int jbd2_journal_enable_debug;
> +extern u16 jbd2_journal_enable_debug;
>
> #define jbd_debug(n, f, a...) \
> do { \
>
>
>
> -JRS
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2007-06-11 22:23:13

by Jose R. Santos

[permalink] [raw]
Subject: Re: [PATCH][RFC] JBD2: Use DebugFS for jbd2 debug config option.

On Mon, 11 Jun 2007 15:04:42 -0700
Mingming Cao <[email protected]> wrote:

> On Fri, 2007-06-08 at 22:57 -0500, Jose R. Santos wrote:
> > On Fri, 8 Jun 2007 01:08:12 -0600
> > Andreas Dilger <[email protected]> wrote:
> >
> > > On Jun 07, 2007 23:45 -0500, Jose R. Santos wrote:
> > > > The jbd2-debug file used to be located in /proc/sys/fs/jbd2-debug, but
> > > > create_proc_entry() does not do lookups on file names with more that one
> > > > directory deep. This causes the entry creation to fail and hence, no proc
> > > > file is created. This patch moves the file to /proc/jbd2-degug.
> > > >
> > > > The file could be move to /proc/fs/jbd2/jbd2-debug, but it would require
> > > > some minor alterations to the jbd-stats patch.
> > >
> > > I don't think we really want to be adding top-level files in /proc.
> > > What about using the "debugfs" filesystem (not to be confused with
> > > the e2fsprogs 'debugfs' command)?
> >
> > How about this then? Moved the file to use debugfs as well as having
> > the nice effect of removing more lines than what it adds.
> >
> Shall we do this for ext3 as well?

We should since its broken in the old JBD code as well. I'll make a
patch and post it to fsdevel and LKML.

-JRS