I enabled config_jbd_debug in the hope that it may help track down the
lockup I'm seeing, but unfortunately the /proc entry does not get
created.
any ideas how to fix this ?
My machine is an Athlon 64X2 - fedora 7 x86_64 - 2.6.23-rc7
CONFIG_EXT3_FS=m
CONFIG_JBD=m
CONFIG_JBD_DEBUG=y
I added the following patch that shows create_proc_entry is failing
Richard
diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
index 06ab3c1..40e7ea3 100644
--- a/fs/jbd/journal.c
+++ b/fs/jbd/journal.c
@@ -1983,7 +1983,8 @@ static void __init create_jbd_proc_entry(void)
/* Why is this so hard? */
proc_jbd_debug->read_proc = read_jbd_debug;
proc_jbd_debug->write_proc = write_jbd_debug;
- }
+ } else
+ printk( KERN_WARNING "jbd:cannot create proc entry : " JBD_PROC_NAME "\n");
}
static void __exit remove_jbd_proc_entry(void)
> I enabled config_jbd_debug in the hope that it may help track down the
> lockup I'm seeing, but unfortunately the /proc entry does not get
> created.
>
> any ideas how to fix this ?
Attached is a patch that should fix it. Andrew, would you queue it up?
Honza
JBD debug code used old way of creating proc entries for jbd-debug file.
Change it to use sysctl instead.
Signed-off-by: Jan Kara <[email protected]>
diff -rupX /home/jack/.kerndiffexclude linux-2.6.23-rc6/fs/jbd/journal.c linux-2.6.23-rc6-1-jbddebug_fix/fs/jbd/journal.c
--- linux-2.6.23-rc6/fs/jbd/journal.c 2007-09-25 12:39:53.000000000 +0200
+++ linux-2.6.23-rc6-1-jbddebug_fix/fs/jbd/journal.c 2007-09-25 13:05:05.000000000 +0200
@@ -1944,58 +1944,29 @@ void journal_put_journal_head(struct jou
#if defined(CONFIG_JBD_DEBUG)
int journal_enable_debug;
EXPORT_SYMBOL(journal_enable_debug);
-#endif
-
-#if defined(CONFIG_JBD_DEBUG) && defined(CONFIG_PROC_FS)
-
-static struct proc_dir_entry *proc_jbd_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", journal_enable_debug);
- *eof = 1;
- return ret;
-}
-
-static int write_jbd_debug(struct file *file, const char __user *buffer,
- unsigned long count, void *data)
-{
- 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';
- journal_enable_debug = simple_strtoul(buf, NULL, 10);
- return count;
-}
+static struct ctl_table_header *jbd_debug_table;
-#define JBD_PROC_NAME "sys/fs/jbd-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;
- }
-}
-
-static void __exit remove_jbd_proc_entry(void)
-{
- if (proc_jbd_debug)
- remove_proc_entry(JBD_PROC_NAME, NULL);
-}
-
-#else
-
-#define create_jbd_proc_entry() do {} while (0)
-#define remove_jbd_proc_entry() do {} while (0)
+static ctl_table fs_table[] = {
+ {
+ .ctl_name = -1, /* Don't want it */
+ .procname = "jbd-debug",
+ .data = &journal_enable_debug,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = &proc_dointvec,
+ },
+ { .ctl_name = 0 },
+};
+
+static ctl_table sys_table[] = {
+ {
+ .ctl_name = CTL_FS,
+ .procname = "fs",
+ .mode = 0555,
+ .child = fs_table,
+ },
+ { .ctl_name = 0 },
+};
#endif
@@ -2054,7 +2025,10 @@ static int __init journal_init(void)
ret = journal_init_caches();
if (ret != 0)
journal_destroy_caches();
- create_jbd_proc_entry();
+
+#ifdef CONFIG_JBD_DEBUG
+ jbd_debug_table = register_sysctl_table(sys_table);
+#endif
return ret;
}
@@ -2064,8 +2038,8 @@ static void __exit journal_exit(void)
int n = atomic_read(&nr_journal_heads);
if (n)
printk(KERN_EMERG "JBD: leaked %d journal_heads!\n", n);
+ unregister_sysctl_table(jbd_debug_table);
#endif
- remove_jbd_proc_entry();
journal_destroy_caches();
}
Jan Kara wrote:
>>
> -#define create_jbd_proc_entry() do {} while (0)
> -#define remove_jbd_proc_entry() do {} while (0)
> +static ctl_table fs_table[] = {
> + {
> + .ctl_name = -1, /* Don't want it */
shouldn't this be CTL_UNNUMBERED ?
> + .procname = "jbd-debug",
> + .data = &journal_enable_debug,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = &proc_dointvec,
> + },
> + { .ctl_name = 0 },
> +};
>
-aneesh
>
>
> Jan Kara wrote:
> >>
> >-#define create_jbd_proc_entry() do {} while (0)
> >-#define remove_jbd_proc_entry() do {} while (0)
> >+static ctl_table fs_table[] = {
> >+ {
> >+ .ctl_name = -1, /* Don't want it */
>
>
>
> shouldn't this be CTL_UNNUMBERED ?
Oh, it should be. I didn't notice we have this :) Thanks for notifying
me. Attached is a fixed version.
>
> >+ .procname = "jbd-debug",
> >+ .data = &journal_enable_debug,
> >+ .maxlen = sizeof(int),
> >+ .mode = 0644,
> >+ .proc_handler = &proc_dointvec,
> >+ },
> >+ { .ctl_name = 0 },
> >+};
> >
--------
JBD debug code used old way of creating proc entries for jbd-debug file.
Change it to use sysctl instead.
Signed-off-by: Jan Kara <[email protected]>
diff -rupX /home/jack/.kerndiffexclude linux-2.6.23-rc6/fs/jbd/journal.c linux-2.6.23-rc6-1-jbddebug_fix/fs/jbd/journal.c
--- linux-2.6.23-rc6/fs/jbd/journal.c 2007-09-25 12:39:53.000000000 +0200
+++ linux-2.6.23-rc6-1-jbddebug_fix/fs/jbd/journal.c 2007-09-25 14:09:11.000000000 +0200
@@ -1944,58 +1944,29 @@ void journal_put_journal_head(struct jou
#if defined(CONFIG_JBD_DEBUG)
int journal_enable_debug;
EXPORT_SYMBOL(journal_enable_debug);
-#endif
-
-#if defined(CONFIG_JBD_DEBUG) && defined(CONFIG_PROC_FS)
-
-static struct proc_dir_entry *proc_jbd_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", journal_enable_debug);
- *eof = 1;
- return ret;
-}
-
-static int write_jbd_debug(struct file *file, const char __user *buffer,
- unsigned long count, void *data)
-{
- 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';
- journal_enable_debug = simple_strtoul(buf, NULL, 10);
- return count;
-}
+static struct ctl_table_header *jbd_debug_table;
-#define JBD_PROC_NAME "sys/fs/jbd-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;
- }
-}
-
-static void __exit remove_jbd_proc_entry(void)
-{
- if (proc_jbd_debug)
- remove_proc_entry(JBD_PROC_NAME, NULL);
-}
-
-#else
-
-#define create_jbd_proc_entry() do {} while (0)
-#define remove_jbd_proc_entry() do {} while (0)
+static ctl_table fs_table[] = {
+ {
+ .ctl_name = CTL_UNNUMBERED,
+ .procname = "jbd-debug",
+ .data = &journal_enable_debug,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = &proc_dointvec,
+ },
+ { .ctl_name = 0 },
+};
+
+static ctl_table sys_table[] = {
+ {
+ .ctl_name = CTL_FS,
+ .procname = "fs",
+ .mode = 0555,
+ .child = fs_table,
+ },
+ { .ctl_name = 0 },
+};
#endif
@@ -2054,7 +2025,10 @@ static int __init journal_init(void)
ret = journal_init_caches();
if (ret != 0)
journal_destroy_caches();
- create_jbd_proc_entry();
+
+#ifdef CONFIG_JBD_DEBUG
+ jbd_debug_table = register_sysctl_table(sys_table);
+#endif
return ret;
}
@@ -2064,8 +2038,8 @@ static void __exit journal_exit(void)
int n = atomic_read(&nr_journal_heads);
if (n)
printk(KERN_EMERG "JBD: leaked %d journal_heads!\n", n);
+ unregister_sysctl_table(jbd_debug_table);
#endif
- remove_jbd_proc_entry();
journal_destroy_caches();
}
On Tue, 25 Sep 2007 13:50:46 +0200
Jan Kara <[email protected]> wrote:
> > Jan Kara wrote:
> > >>
> > >-#define create_jbd_proc_entry() do {} while (0)
> > >-#define remove_jbd_proc_entry() do {} while (0)
> > >+static ctl_table fs_table[] = {
> > >+ {
> > >+ .ctl_name = -1, /* Don't want it */
> >
> >
> >
> > shouldn't this be CTL_UNNUMBERED ?
> Oh, it should be. I didn't notice we have this :) Thanks for notifying
> me. Attached is a fixed version.
This was fixed in JBD2 by moving the jbd-debug file to debugfs:
http://lkml.org/lkml/2007/7/11/334
Since this code is already in the kernel, we should keep it consistent.
-JRS
On Tue, 25 Sep 2007 07:49:38 -0500
"Jose R. Santos" <[email protected]> wrote:
> On Tue, 25 Sep 2007 13:50:46 +0200
> Jan Kara <[email protected]> wrote:
> > > Jan Kara wrote:
> > > >>
> > > >-#define create_jbd_proc_entry() do {} while (0)
> > > >-#define remove_jbd_proc_entry() do {} while (0)
> > > >+static ctl_table fs_table[] = {
> > > >+ {
> > > >+ .ctl_name = -1, /* Don't want it */
> > >
> > >
> > >
> > > shouldn't this be CTL_UNNUMBERED ?
> > Oh, it should be. I didn't notice we have this :) Thanks for notifying
> > me. Attached is a fixed version.
>
> This was fixed in JBD2 by moving the jbd-debug file to debugfs:
> http://lkml.org/lkml/2007/7/11/334
>
> Since this code is already in the kernel, we should keep it consistent.
>
OK. Here's a quick patch to fix this. Adapted from the JBD2 patch.
Let me know what you think.
-JRS
commit 6cbd2ce05b7504514707ce825170a5d77abf6a6e
Author: root <[email protected]>
Date: Thu Jun 14 09:40:09 2007 -0500
The jbd-debug file used to be located in /proc/sys/fs/jbd-debug, but
create_proc_entry() does not do lookups on file names that are more that one
directory deep. This causes the entry creation to fail and hence, no proc
file is created.
Instead of fixing this on procfs might as well move the jbd2-debug file to
debugfs which would be the preferred location for this kind of tunable. The
new location is now /sys/kernel/debug/jbd/jbd-debug.
Signed-off-by: Jose R. Santos <[email protected]>
diff --git a/fs/Kconfig b/fs/Kconfig
index 58a0650..a8937a6 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -219,7 +219,7 @@ config JBD
config JBD_DEBUG
bool "JBD (ext3) debugging support"
- depends on JBD
+ depends on JBD && DEBUG_FS
help
If you are using the ext3 journaled file system (or potentially any
other file system/device using JBD), this option allows you to
@@ -228,10 +228,10 @@ config JBD_DEBUG
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/jbd-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/jbd-debug".
+ with "echo N > /sys/kernel/debug/jbd/jbd-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 > /sys/kernel/debug/jbd/jbd-debug".
config JBD2
tristate
diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
index 06ab3c1..3cad624 100644
--- a/fs/jbd/journal.c
+++ b/fs/jbd/journal.c
@@ -35,6 +35,7 @@
#include <linux/kthread.h>
#include <linux/poison.h>
#include <linux/proc_fs.h>
+#include <linux/debugfs.h>
#include <asm/uaccess.h>
#include <asm/page.h>
@@ -1939,63 +1940,38 @@ void journal_put_journal_head(struct journal_head *jh)
}
/*
- * /proc tunables
+ * debugfs tunables
*/
#if defined(CONFIG_JBD_DEBUG)
-int journal_enable_debug;
+u8 journal_enable_debug;
EXPORT_SYMBOL(journal_enable_debug);
#endif
-#if defined(CONFIG_JBD_DEBUG) && defined(CONFIG_PROC_FS)
+#if defined(CONFIG_JBD_DEBUG) && defined(CONFIG_DEBUG_FS)
-static struct proc_dir_entry *proc_jbd_debug;
+struct dentry *jbd_debugfs_dir, *jbd_debug;
-static int read_jbd_debug(char *page, char **start, off_t off,
- int count, int *eof, void *data)
+static void __init create_jbd_debugfs_entry(void)
{
- int ret;
-
- ret = sprintf(page + off, "%d\n", journal_enable_debug);
- *eof = 1;
- return ret;
-}
-
-static int write_jbd_debug(struct file *file, const char __user *buffer,
- unsigned long count, void *data)
-{
- 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';
- journal_enable_debug = simple_strtoul(buf, NULL, 10);
- return count;
-}
-
-#define JBD_PROC_NAME "sys/fs/jbd-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;
- }
+ jbd_debugfs_dir = debugfs_create_dir("jbd", NULL);
+ if (jbd_debugfs_dir)
+ jbd_debug = debugfs_create_u8("jbd-debug", S_IRUGO,
+ jbd_debugfs_dir,
+ &journal_enable_debug);
}
-static void __exit remove_jbd_proc_entry(void)
+static void __exit remove_jbd_debugfs_entry(void)
{
- if (proc_jbd_debug)
- remove_proc_entry(JBD_PROC_NAME, NULL);
+ if (jbd_debug)
+ debugfs_remove(jbd_debug);
+ if (jbd_debugfs_dir)
+ debugfs_remove(jbd_debugfs_dir);
}
#else
-#define create_jbd_proc_entry() do {} while (0)
-#define remove_jbd_proc_entry() do {} while (0)
+#define create_jbd_debugfs_entry() do {} while (0)
+#define remove_jbd_debugfs_entry() do {} while (0)
#endif
@@ -2054,7 +2030,7 @@ static int __init journal_init(void)
ret = journal_init_caches();
if (ret != 0)
journal_destroy_caches();
- create_jbd_proc_entry();
+ create_jbd_debugfs_entry();
return ret;
}
@@ -2065,7 +2041,7 @@ static void __exit journal_exit(void)
if (n)
printk(KERN_EMERG "JBD: leaked %d journal_heads!\n", n);
#endif
- remove_jbd_proc_entry();
+ remove_jbd_debugfs_entry();
journal_destroy_caches();
}
diff --git a/include/linux/jbd.h b/include/linux/jbd.h
index 4527375..526548d 100644
--- a/include/linux/jbd.h
+++ b/include/linux/jbd.h
@@ -57,7 +57,7 @@
* CONFIG_JBD_DEBUG is on.
*/
#define JBD_EXPENSIVE_CHECKING
-extern int journal_enable_debug;
+extern u8 journal_enable_debug;
#define jbd_debug(n, f, a...) \
do { \
> On Tue, 25 Sep 2007 07:49:38 -0500
> "Jose R. Santos" <[email protected]> wrote:
>
> > On Tue, 25 Sep 2007 13:50:46 +0200
> > Jan Kara <[email protected]> wrote:
> > > > Jan Kara wrote:
> > > > >>
> > > > >-#define create_jbd_proc_entry() do {} while (0)
> > > > >-#define remove_jbd_proc_entry() do {} while (0)
> > > > >+static ctl_table fs_table[] = {
> > > > >+ {
> > > > >+ .ctl_name = -1, /* Don't want it */
> > > >
> > > >
> > > >
> > > > shouldn't this be CTL_UNNUMBERED ?
> > > Oh, it should be. I didn't notice we have this :) Thanks for notifying
> > > me. Attached is a fixed version.
> >
> > This was fixed in JBD2 by moving the jbd-debug file to debugfs:
> > http://lkml.org/lkml/2007/7/11/334
> >
> > Since this code is already in the kernel, we should keep it consistent.
> >
>
> OK. Here's a quick patch to fix this. Adapted from the JBD2 patch.
> Let me know what you think.
Looks fine - exactly what I've just done here :).
Honza
> commit 6cbd2ce05b7504514707ce825170a5d77abf6a6e
> Author: root <[email protected]>
> Date: Thu Jun 14 09:40:09 2007 -0500
>
> The jbd-debug file used to be located in /proc/sys/fs/jbd-debug, but
> create_proc_entry() does not do lookups on file names that are more that one
> directory deep. This causes the entry creation to fail and hence, no proc
> file is created.
>
> Instead of fixing this on procfs might as well move the jbd2-debug file to
> debugfs which would be the preferred location for this kind of tunable. The
> new location is now /sys/kernel/debug/jbd/jbd-debug.
>
>
> Signed-off-by: Jose R. Santos <[email protected]>
You can add Signed-off-by: Jan Kara <[email protected]>
>
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 58a0650..a8937a6 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -219,7 +219,7 @@ config JBD
>
> config JBD_DEBUG
> bool "JBD (ext3) debugging support"
> - depends on JBD
> + depends on JBD && DEBUG_FS
> help
> If you are using the ext3 journaled file system (or potentially any
> other file system/device using JBD), this option allows you to
> @@ -228,10 +228,10 @@ config JBD_DEBUG
> 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/jbd-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/jbd-debug".
> + with "echo N > /sys/kernel/debug/jbd/jbd-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 > /sys/kernel/debug/jbd/jbd-debug".
>
> config JBD2
> tristate
> diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
> index 06ab3c1..3cad624 100644
> --- a/fs/jbd/journal.c
> +++ b/fs/jbd/journal.c
> @@ -35,6 +35,7 @@
> #include <linux/kthread.h>
> #include <linux/poison.h>
> #include <linux/proc_fs.h>
> +#include <linux/debugfs.h>
>
> #include <asm/uaccess.h>
> #include <asm/page.h>
> @@ -1939,63 +1940,38 @@ void journal_put_journal_head(struct journal_head *jh)
> }
>
> /*
> - * /proc tunables
> + * debugfs tunables
> */
> #if defined(CONFIG_JBD_DEBUG)
> -int journal_enable_debug;
> +u8 journal_enable_debug;
> EXPORT_SYMBOL(journal_enable_debug);
> #endif
>
> -#if defined(CONFIG_JBD_DEBUG) && defined(CONFIG_PROC_FS)
> +#if defined(CONFIG_JBD_DEBUG) && defined(CONFIG_DEBUG_FS)
>
> -static struct proc_dir_entry *proc_jbd_debug;
> +struct dentry *jbd_debugfs_dir, *jbd_debug;
>
> -static int read_jbd_debug(char *page, char **start, off_t off,
> - int count, int *eof, void *data)
> +static void __init create_jbd_debugfs_entry(void)
> {
> - int ret;
> -
> - ret = sprintf(page + off, "%d\n", journal_enable_debug);
> - *eof = 1;
> - return ret;
> -}
> -
> -static int write_jbd_debug(struct file *file, const char __user *buffer,
> - unsigned long count, void *data)
> -{
> - 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';
> - journal_enable_debug = simple_strtoul(buf, NULL, 10);
> - return count;
> -}
> -
> -#define JBD_PROC_NAME "sys/fs/jbd-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;
> - }
> + jbd_debugfs_dir = debugfs_create_dir("jbd", NULL);
> + if (jbd_debugfs_dir)
> + jbd_debug = debugfs_create_u8("jbd-debug", S_IRUGO,
> + jbd_debugfs_dir,
> + &journal_enable_debug);
> }
>
> -static void __exit remove_jbd_proc_entry(void)
> +static void __exit remove_jbd_debugfs_entry(void)
> {
> - if (proc_jbd_debug)
> - remove_proc_entry(JBD_PROC_NAME, NULL);
> + if (jbd_debug)
> + debugfs_remove(jbd_debug);
> + if (jbd_debugfs_dir)
> + debugfs_remove(jbd_debugfs_dir);
> }
>
> #else
>
> -#define create_jbd_proc_entry() do {} while (0)
> -#define remove_jbd_proc_entry() do {} while (0)
> +#define create_jbd_debugfs_entry() do {} while (0)
> +#define remove_jbd_debugfs_entry() do {} while (0)
>
> #endif
>
> @@ -2054,7 +2030,7 @@ static int __init journal_init(void)
> ret = journal_init_caches();
> if (ret != 0)
> journal_destroy_caches();
> - create_jbd_proc_entry();
> + create_jbd_debugfs_entry();
> return ret;
> }
>
> @@ -2065,7 +2041,7 @@ static void __exit journal_exit(void)
> if (n)
> printk(KERN_EMERG "JBD: leaked %d journal_heads!\n", n);
> #endif
> - remove_jbd_proc_entry();
> + remove_jbd_debugfs_entry();
> journal_destroy_caches();
> }
>
> diff --git a/include/linux/jbd.h b/include/linux/jbd.h
> index 4527375..526548d 100644
> --- a/include/linux/jbd.h
> +++ b/include/linux/jbd.h
> @@ -57,7 +57,7 @@
> * CONFIG_JBD_DEBUG is on.
> */
> #define JBD_EXPENSIVE_CHECKING
> -extern int journal_enable_debug;
> +extern u8 journal_enable_debug;
>
> #define jbd_debug(n, f, a...) \
> do { \
--
Jan Kara <[email protected]>
SuSE CR Labs
On Tue, 25 Sep 2007 16:36:08 +0200
Jan Kara <[email protected]> wrote:
> > On Tue, 25 Sep 2007 07:49:38 -0500
> > "Jose R. Santos" <[email protected]> wrote:
> >
> > > On Tue, 25 Sep 2007 13:50:46 +0200
> > > Jan Kara <[email protected]> wrote:
> > > > > Jan Kara wrote:
> > > > > >>
> > > > > >-#define create_jbd_proc_entry() do {} while (0)
> > > > > >-#define remove_jbd_proc_entry() do {} while (0)
> > > > > >+static ctl_table fs_table[] = {
> > > > > >+ {
> > > > > >+ .ctl_name = -1, /* Don't want it */
> > > > >
> > > > >
> > > > >
> > > > > shouldn't this be CTL_UNNUMBERED ?
> > > > Oh, it should be. I didn't notice we have this :) Thanks for notifying
> > > > me. Attached is a fixed version.
> > >
> > > This was fixed in JBD2 by moving the jbd-debug file to debugfs:
> > > http://lkml.org/lkml/2007/7/11/334
> > >
> > > Since this code is already in the kernel, we should keep it consistent.
> > >
> >
> > OK. Here's a quick patch to fix this. Adapted from the JBD2 patch.
> > Let me know what you think.
> Looks fine - exactly what I've just done here :).
hm. I found rather a lot of issues. If this patch is derived from the
JBD2 patch then perhaps the JBD2 patch needs some looking at.
> > Signed-off-by: Jose R. Santos <[email protected]>
> You can add Signed-off-by: Jan Kara <[email protected]>
I suspect you might be getting your signed-off-bys and acked-bys mixed up.
(If not this patch, then the previous one). Please see
Documentation/SubmittingPatches section 13 for the difference.
Jose, please review and if possible runtime test these proposed changes?
From: Andrew Morton <[email protected]>
- use `#ifdef foo' instead of `#if defined(foo)'
- CONFIG_JBD_DEBUG depends on CONFIG_DEBUG_FS so we don't need to duplicate
that logic in the .c file ifdefs
- Make journal_enable_debug __read_mostly just for the heck of it
- Make jbd_debugfs_dir and jbd_debug static
- debugfs_remove(NULL) is legal: remove unneeded tests
- jbd_create_debugfs_entry is a better name than create_jbd_debugfs_entry
- ditto remove_jbd_debugfs_entry
- C functions are preferred over macros
Cc: "Jose R. Santos" <[email protected]>
Cc: <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Jose R. Santos <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
diff -puN fs/jbd/journal.c~jbd-config_jbd_debug-cannot-create-proc-entry-fix fs/jbd/journal.c
--- a/fs/jbd/journal.c~jbd-config_jbd_debug-cannot-create-proc-entry-fix
+++ a/fs/jbd/journal.c
@@ -1853,16 +1853,15 @@ void journal_put_journal_head(struct jou
/*
* debugfs tunables
*/
-#if defined(CONFIG_JBD_DEBUG)
-u8 journal_enable_debug;
-EXPORT_SYMBOL(journal_enable_debug);
-#endif
+#ifdef CONFIG_JBD_DEBUG
-#if defined(CONFIG_JBD_DEBUG) && defined(CONFIG_DEBUG_FS)
+u8 journal_enable_debug __read_mostly;
+EXPORT_SYMBOL(journal_enable_debug);
-struct dentry *jbd_debugfs_dir, *jbd_debug;
+static struct dentry *jbd_debugfs_dir;
+static struct dentry *jbd_debug;
-static void __init create_jbd_debugfs_entry(void)
+static void __init jbd_create_debugfs_entry(void)
{
jbd_debugfs_dir = debugfs_create_dir("jbd", NULL);
if (jbd_debugfs_dir)
@@ -1871,18 +1870,21 @@ static void __init create_jbd_debugfs_en
&journal_enable_debug);
}
-static void __exit remove_jbd_debugfs_entry(void)
+static void __exit jbd_remove_debugfs_entry(void)
{
- if (jbd_debug)
- debugfs_remove(jbd_debug);
- if (jbd_debugfs_dir)
- debugfs_remove(jbd_debugfs_dir);
+ debugfs_remove(jbd_debug);
+ debugfs_remove(jbd_debugfs_dir);
}
#else
-#define create_jbd_debugfs_entry() do {} while (0)
-#define remove_jbd_debugfs_entry() do {} while (0)
+static inline void jbd_create_debugfs_entry(void)
+{
+}
+
+static inline void jbd_remove_debugfs_entry(void)
+{
+}
#endif
@@ -1940,7 +1942,7 @@ static int __init journal_init(void)
ret = journal_init_caches();
if (ret != 0)
journal_destroy_caches();
- create_jbd_debugfs_entry();
+ jbd_create_debugfs_entry();
return ret;
}
@@ -1951,7 +1953,7 @@ static void __exit journal_exit(void)
if (n)
printk(KERN_EMERG "JBD: leaked %d journal_heads!\n", n);
#endif
- remove_jbd_debugfs_entry();
+ jbd_remove_debugfs_entry();
journal_destroy_caches();
}
_
On Wed, 26 Sep 2007 14:35:39 -0700
Andrew Morton <[email protected]> wrote:
> On Tue, 25 Sep 2007 16:36:08 +0200
> Jan Kara <[email protected]> wrote:
>
> > > On Tue, 25 Sep 2007 07:49:38 -0500
> > > "Jose R. Santos" <[email protected]> wrote:
> > >
> > > > On Tue, 25 Sep 2007 13:50:46 +0200
> > > > Jan Kara <[email protected]> wrote:
> > > > > > Jan Kara wrote:
> > > > > > >>
> > > > > > >-#define create_jbd_proc_entry() do {} while (0)
> > > > > > >-#define remove_jbd_proc_entry() do {} while (0)
> > > > > > >+static ctl_table fs_table[] = {
> > > > > > >+ {
> > > > > > >+ .ctl_name = -1, /* Don't want it */
> > > > > >
> > > > > >
> > > > > >
> > > > > > shouldn't this be CTL_UNNUMBERED ?
> > > > > Oh, it should be. I didn't notice we have this :) Thanks for notifying
> > > > > me. Attached is a fixed version.
> > > >
> > > > This was fixed in JBD2 by moving the jbd-debug file to debugfs:
> > > > http://lkml.org/lkml/2007/7/11/334
> > > >
> > > > Since this code is already in the kernel, we should keep it consistent.
> > > >
> > >
> > > OK. Here's a quick patch to fix this. Adapted from the JBD2 patch.
> > > Let me know what you think.
> > Looks fine - exactly what I've just done here :).
>
> hm. I found rather a lot of issues. If this patch is derived from the
> JBD2 patch then perhaps the JBD2 patch needs some looking at.
Some of the changes do apply to the JBD2 patch. I'll send a cleanup patch.
>
> > > Signed-off-by: Jose R. Santos <[email protected]>
> > You can add Signed-off-by: Jan Kara <[email protected]>
>
> I suspect you might be getting your signed-off-bys and acked-bys mixed up.
> (If not this patch, then the previous one). Please see
> Documentation/SubmittingPatches section 13 for the difference.
>
> Jose, please review and if possible runtime test these proposed changes?
Agree with all the changes and they worked as expected on my system.
> From: Andrew Morton <[email protected]>
>
> - use `#ifdef foo' instead of `#if defined(foo)'
>
> - CONFIG_JBD_DEBUG depends on CONFIG_DEBUG_FS so we don't need to duplicate
> that logic in the .c file ifdefs
>
> - Make journal_enable_debug __read_mostly just for the heck of it
>
> - Make jbd_debugfs_dir and jbd_debug static
>
> - debugfs_remove(NULL) is legal: remove unneeded tests
>
> - jbd_create_debugfs_entry is a better name than create_jbd_debugfs_entry
>
> - ditto remove_jbd_debugfs_entry
>
> - C functions are preferred over macros
>
> Cc: "Jose R. Santos" <[email protected]>
> Cc: <[email protected]>
> Cc: Jan Kara <[email protected]>
> Cc: Jose R. Santos <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
Acked-by: Jose R. Santos <[email protected]>
-JRS