Re: JBD-DEBUG /proc/sys entry (again)

2007-10-04 Thread Jose R. Santos 
On Thu, 04 Oct 2007 16:28:07 +0400
Rusev <[EMAIL PROTECTED]> wrote:

> All that should be moved to DEBUGFS under /sys/kernel/debug  and so on
> -  that's right, a bit other issue
> is of interest for me.
> 
> My suggestion is that a few other problems with PROCFS exist:
> 
> From my observation there are two major issues are involved:
> 
> 1.  /proc/sys entry has very specific readdir operation (vs. other
> entries such as /proc/drivers and others)
> entries to /proc/sys is likely is to be performed by means of other API.
> (quick search found thet API explanation for 2.4.xx
> http://www.opentech.at/papers/embedded_proc/node33.html,
>  yet it looks to be a little change in 2.6)
> 
> 2.  function xlate_proc_name behaves not the way it specified in it's
> header comment:
>   [1]  minor issue is that proc_match wolud likely return "equals"
> as result of comparison of "sys" and "sysvipc"
>   [2] more significant issue is that it can't properly walk long
> paths  such as /proc/sys/jbd/jbd-debug,
>but only paths likes /proc/sys/jbd-debug  (just one step
> down, path walking is broken).
> 
> This way we can't add not only /proc/sys/jbd/jbd-debug but any path
> likes /proc/aaa/bbb/xxx-debug at one step.
> The entry /proc/sys is still specific, because even if fixing
> xlate_proc_name we can't see /proc/sys/jbd/jbd-debug
> in userspace and successfully see /proc/aaa/bbb/jbd-debug.

This patch is wrong.  xlate_pro_name() is meant to check if a given
path is valid, creating new directory entries is something that need to
be handle by the code that's creating the entries.

Also note that xlate_pro_name() is also called by remove_proc_entry()
so if I call it with a bogus path, this patch will end up creating new
directory entries which is not the intended result.

> That's because /proc/sys specific operator readdir blocks such PROCFS
> entries that they are NOTproperly registersd
>  with CTL_TABLE.
> 
> Yet I think that we have a general problem with
> adding-long-paths-in-one-step, which is addressed by the following patch:

This should not be done is user one step and for good reason.  If you
blindly create multiple directory entries in /proc, how are you going to
keep track of all the created entries when its time to remove them
(module unloading for example)?

If you enter an invalid path the original code is doing the right thing
by returning -ENOENT.

> 
> 
> 
> diff -uprN linux-2.6.21.orig/fs/proc/generic.c
> linux-2.6.21/fs/proc/generic.c
> --- linux-2.6.21.orig/fs/proc/generic.c 2007-09-13 15:36:07.0 +0400
> +++ linux-2.6.21/fs/proc/generic.c  2007-10-03 22:12:57.0 +0400
> @@ -298,6 +298,7 @@ static int xlate_proc_name(const char *n
> int len;
> int rtn = 0;
> 
> +

White space damage.

> spin_lock(_subdir_lock);
> de = _root;
> while (1) {
> @@ -305,24 +306,52 @@ static int xlate_proc_name(const char *n
> if (!next)
> break;
> 
> -   len = next - cp;
> -   for (de = de->subdir; de ; de = de->next) {
> -   if (proc_match(len, cp, de))
> -   break;
> -   }
> -   if (!de) {
> -   rtn = -ENOENT;
> -   goto out;
> -   }
> -   cp += len + 1;
> -   }
> +++next;
> +
> +
> +len = next - cp;
> +
> +if(de->subdir == NULL){
> +  /* directory "de" is empty, add myself to it now */
> +  char* my_name = kzalloc( (len - 1)  + 1, GFP_KERNEL);

You did not check if kzalloc was successfully.  If the allocation
fails, bad things will happen here.  Need to check the return status of
my_name and return -ENOMEM if the allocation fails.  This would of
course mean an API change and you would need make sure that all the
callers of xlate_proc_name handle the new return code correctly.

> +  memcpy(my_name, cp, len - 1);
> +  proc_mkdir(my_name,de);
> +  kfree(my_name);
> +}
> +
> +
> +struct proc_dir_entry   *parent_de = de;
> +for (de = parent_de->subdir; de ; de = de->next) {
> +  if (proc_match(len - 1, cp, de))
> +break;
> +
> +}
> +
> +if(de == NULL){
> +  /* we found no appropriate subdirectory, well create
> it now */

1. Email client cut the line.  Disable line wrapping.
2. Line too long - Documentation/CodingStyle

> +  char* my_name = kzalloc( (len - 1)  + 1, GFP_KERNEL);

Again, check for kzalloc return status.

> +  memcpy(my_name, cp, len - 1);
> +  de = proc_mkdir(my_name,parent_de);
> +  kfree(my_name);
> +}
> +
> +
> +

White space damage.

Re: JBD-DEBUG /proc/sys entry (again)

2007-10-04 Thread Jose R. Santos 
On Thu, 04 Oct 2007 16:28:07 +0400
Rusev [EMAIL PROTECTED] wrote:

 All that should be moved to DEBUGFS under /sys/kernel/debug  and so on
 -  that's right, a bit other issue
 is of interest for me.
 
 My suggestion is that a few other problems with PROCFS exist:
 
 From my observation there are two major issues are involved:
 
 1.  /proc/sys entry has very specific readdir operation (vs. other
 entries such as /proc/drivers and others)
 entries to /proc/sys is likely is to be performed by means of other API.
 (quick search found thet API explanation for 2.4.xx
 http://www.opentech.at/papers/embedded_proc/node33.html,
  yet it looks to be a little change in 2.6)
 
 2.  function xlate_proc_name behaves not the way it specified in it's
 header comment:
   [1]  minor issue is that proc_match wolud likely return equals
 as result of comparison of sys and sysvipc
   [2] more significant issue is that it can't properly walk long
 paths  such as /proc/sys/jbd/jbd-debug,
but only paths likes /proc/sys/jbd-debug  (just one step
 down, path walking is broken).
 
 This way we can't add not only /proc/sys/jbd/jbd-debug but any path
 likes /proc/aaa/bbb/xxx-debug at one step.
 The entry /proc/sys is still specific, because even if fixing
 xlate_proc_name we can't see /proc/sys/jbd/jbd-debug
 in userspace and successfully see /proc/aaa/bbb/jbd-debug.

This patch is wrong.  xlate_pro_name() is meant to check if a given
path is valid, creating new directory entries is something that need to
be handle by the code that's creating the entries.

Also note that xlate_pro_name() is also called by remove_proc_entry()
so if I call it with a bogus path, this patch will end up creating new
directory entries which is not the intended result.

 That's because /proc/sys specific operator readdir blocks such PROCFS
 entries that they are NOTproperly registersd
  with CTL_TABLE.
 
 Yet I think that we have a general problem with
 adding-long-paths-in-one-step, which is addressed by the following patch:

This should not be done is user one step and for good reason.  If you
blindly create multiple directory entries in /proc, how are you going to
keep track of all the created entries when its time to remove them
(module unloading for example)?

If you enter an invalid path the original code is doing the right thing
by returning -ENOENT.

 
 
 
 diff -uprN linux-2.6.21.orig/fs/proc/generic.c
 linux-2.6.21/fs/proc/generic.c
 --- linux-2.6.21.orig/fs/proc/generic.c 2007-09-13 15:36:07.0 +0400
 +++ linux-2.6.21/fs/proc/generic.c  2007-10-03 22:12:57.0 +0400
 @@ -298,6 +298,7 @@ static int xlate_proc_name(const char *n
 int len;
 int rtn = 0;
 
 +

White space damage.

 spin_lock(proc_subdir_lock);
 de = proc_root;
 while (1) {
 @@ -305,24 +306,52 @@ static int xlate_proc_name(const char *n
 if (!next)
 break;
 
 -   len = next - cp;
 -   for (de = de-subdir; de ; de = de-next) {
 -   if (proc_match(len, cp, de))
 -   break;
 -   }
 -   if (!de) {
 -   rtn = -ENOENT;
 -   goto out;
 -   }
 -   cp += len + 1;
 -   }
 +++next;
 +
 +
 +len = next - cp;
 +
 +if(de-subdir == NULL){
 +  /* directory de is empty, add myself to it now */
 +  char* my_name = kzalloc( (len - 1)  + 1, GFP_KERNEL);

You did not check if kzalloc was successfully.  If the allocation
fails, bad things will happen here.  Need to check the return status of
my_name and return -ENOMEM if the allocation fails.  This would of
course mean an API change and you would need make sure that all the
callers of xlate_proc_name handle the new return code correctly.

 +  memcpy(my_name, cp, len - 1);
 +  proc_mkdir(my_name,de);
 +  kfree(my_name);
 +}
 +
 +
 +struct proc_dir_entry   *parent_de = de;
 +for (de = parent_de-subdir; de ; de = de-next) {
 +  if (proc_match(len - 1, cp, de))
 +break;
 +
 +}
 +
 +if(de == NULL){
 +  /* we found no appropriate subdirectory, well create
 it now */

1. Email client cut the line.  Disable line wrapping.
2. Line too long - Documentation/CodingStyle

 +  char* my_name = kzalloc( (len - 1)  + 1, GFP_KERNEL);

Again, check for kzalloc return status.

 +  memcpy(my_name, cp, len - 1);
 +  de = proc_mkdir(my_name,parent_de);
 +  kfree(my_name);
 +}
 +
 +
 +

White space damage.  To many new lines.  One is enough.

 +if (!de){
 +rtn = -ENOENT;

[PATCH] JBD2: debug code cleanup.

2007-09-27 Thread Jose R. Santos 
From: Jose R. Santos <[EMAIL PROTECTED]>

JBD2: debug code cleanup.

Mostly stolen from akpm's JBD cleanup patch.

- use `#ifdef foo' instead of `#if defined(foo)'

- 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

- remove unnecessary empty loops

Signed-off-by: Jose R. Santos <[EMAIL PROTECTED]>
--

 fs/jbd2/journal.c |   20 ++--
 1 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index f37324a..9a7187f 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1953,16 +1953,14 @@ void jbd2_journal_put_journal_head(struct journal_head 
*jh)
 /*
  * debugfs tunables
  */
-#if defined(CONFIG_JBD2_DEBUG)
-u8 jbd2_journal_enable_debug;
+#ifdef CONFIG_JBD2_DEBUG
+u8 jbd2_journal_enable_debug __read_mostly;
 EXPORT_SYMBOL(jbd2_journal_enable_debug);
-#endif
-
-#if defined(CONFIG_JBD2_DEBUG) && defined(CONFIG_DEBUG_FS)
 
 #define JBD2_DEBUG_NAME "jbd2-debug"
 
-struct dentry *jbd2_debugfs_dir, *jbd2_debug;
+static struct dentry *jbd2_debugfs_dir;
+static struct dentry *jbd2_debug;
 
 static void __init jbd2_create_debugfs_entry(void)
 {
@@ -1975,24 +1973,18 @@ static void __init jbd2_create_debugfs_entry(void)
 
 static void __exit jbd2_remove_debugfs_entry(void)
 {
-   if (jbd2_debug)
-   debugfs_remove(jbd2_debug);
-   if (jbd2_debugfs_dir)
-   debugfs_remove(jbd2_debugfs_dir);
+   debugfs_remove(jbd2_debug);
+   debugfs_remove(jbd2_debugfs_dir);
 }
 
 #else
 
 static void __init jbd2_create_debugfs_entry(void)
 {
-   do {
-   } while (0);
 }
 
 static void __exit jbd2_remove_debugfs_entry(void)
 {
-   do {
-   } while (0);
 }
 
 #endif
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] JBD2: debug code cleanup.

2007-09-27 Thread Jose R. Santos 
From: Jose R. Santos [EMAIL PROTECTED]

JBD2: debug code cleanup.

Mostly stolen from akpm's JBD cleanup patch.

- use `#ifdef foo' instead of `#if defined(foo)'

- 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

- remove unnecessary empty loops

Signed-off-by: Jose R. Santos [EMAIL PROTECTED]
--

 fs/jbd2/journal.c |   20 ++--
 1 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index f37324a..9a7187f 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1953,16 +1953,14 @@ void jbd2_journal_put_journal_head(struct journal_head 
*jh)
 /*
  * debugfs tunables
  */
-#if defined(CONFIG_JBD2_DEBUG)
-u8 jbd2_journal_enable_debug;
+#ifdef CONFIG_JBD2_DEBUG
+u8 jbd2_journal_enable_debug __read_mostly;
 EXPORT_SYMBOL(jbd2_journal_enable_debug);
-#endif
-
-#if defined(CONFIG_JBD2_DEBUG)  defined(CONFIG_DEBUG_FS)
 
 #define JBD2_DEBUG_NAME jbd2-debug
 
-struct dentry *jbd2_debugfs_dir, *jbd2_debug;
+static struct dentry *jbd2_debugfs_dir;
+static struct dentry *jbd2_debug;
 
 static void __init jbd2_create_debugfs_entry(void)
 {
@@ -1975,24 +1973,18 @@ static void __init jbd2_create_debugfs_entry(void)
 
 static void __exit jbd2_remove_debugfs_entry(void)
 {
-   if (jbd2_debug)
-   debugfs_remove(jbd2_debug);
-   if (jbd2_debugfs_dir)
-   debugfs_remove(jbd2_debugfs_dir);
+   debugfs_remove(jbd2_debug);
+   debugfs_remove(jbd2_debugfs_dir);
 }
 
 #else
 
 static void __init jbd2_create_debugfs_entry(void)
 {
-   do {
-   } while (0);
 }
 
 static void __exit jbd2_remove_debugfs_entry(void)
 {
-   do {
-   } while (0);
 }
 
 #endif
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: jbd : config_jbd_debug cannot create /proc entry

2007-09-26 Thread Jose R. Santos 
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: jbd : config_jbd_debug cannot create /proc entry

2007-09-26 Thread Jose R. Santos 
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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] JBD: Fix JBD warnings when compiling with CONFIG_JBD_DEBUG

2007-09-25 Thread Jose R. Santos 
From: Jose R. Santos <[EMAIL PROTECTED]>

JBD: Fix JBD warnings when compiling with CONFIG_JBD_DEBUG

Note from Mingming's JBD2 fix:

Noticed all warnings are occurs when the debug level is 0. Then found
the "jbd2: Move jbd2-debug file to debugfs" patch
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=0f49d5d019afa4e94253bfc92f0daca3badb990b

changed the jbd2_journal_enable_debug from int type to u8, makes the
jbd_debug comparision is always true when the debugging level is 0. Thus
the compile warning occurs. 

Thought about changing the jbd2_journal_enable_debug data type back to
int, but can't, because the jbd2-debug is moved to debug fs, where
calling debugfs_create_u8() to create the debugfs entry needs the value
to be u8 type.

Even if we changed the data type back to int, the code is still buggy,
kernel should not print jbd2 debug message if the
jbd2_journal_enable_debug is set to 0. But this is not the case.

The fix is change the level of debugging to 1. The same should fixed in
ext3/JBD, but currently ext3 jbd-debug via /proc fs is broken, so we
probably should fix it all together.

Signed-off-by: Jose R. Santos <[EMAIL PROTECTED]>
--

 fs/ext3/inode.c   |2 +-
 fs/jbd/recovery.c |6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index de4e316..f5b0e79 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -2887,7 +2887,7 @@ int ext3_write_inode(struct inode *inode, int wait)
return 0;
 
if (ext3_journal_current_handle()) {
-   jbd_debug(0, "called recursively, non-PF_MEMALLOC!\n");
+   jbd_debug(1, "called recursively, non-PF_MEMALLOC!\n");
dump_stack();
return -EIO;
}
diff --git a/fs/jbd/recovery.c b/fs/jbd/recovery.c
index 2a5f4b8..c5d9694 100644
--- a/fs/jbd/recovery.c
+++ b/fs/jbd/recovery.c
@@ -250,10 +250,10 @@ int journal_recover(journal_t *journal)
if (!err)
err = do_one_pass(journal, , PASS_REPLAY);
 
-   jbd_debug(0, "JBD: recovery, exit status %d, "
+   jbd_debug(1, "JBD: recovery, exit status %d, "
  "recovered transactions %u to %u\n",
  err, info.start_transaction, info.end_transaction);
-   jbd_debug(0, "JBD: Replayed %d and revoked %d/%d blocks\n",
+   jbd_debug(1, "JBD: Replayed %d and revoked %d/%d blocks\n",
  info.nr_replays, info.nr_revoke_hits, info.nr_revokes);
 
/* Restart the log at the next transaction ID, thus invalidating
@@ -297,7 +297,7 @@ int journal_skip_recovery(journal_t *journal)
 #ifdef CONFIG_JBD_DEBUG
int dropped = info.end_transaction - 
be32_to_cpu(sb->s_sequence);
 #endif
-   jbd_debug(0,
+   jbd_debug(1,
  "JBD: ignoring %d transaction%s from the journal.\n",
  dropped, (dropped == 1) ? "" : "s");
journal->j_transaction_sequence = ++info.end_transaction;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] JBD: Export jbd-debug via debugfs

2007-09-25 Thread Jose R. Santos 
From: Jose R. Santos <[EMAIL PROTECTED]>

JBD: Export jbd-debug via debugfs

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]>
Signed-off-by: Jan Kara <[EMAIL PROTECTED]>
--

 fs/Kconfig  |   10 
 fs/jbd/journal.c|   65 ---
 include/linux/jbd.h |2 +-
 3 files changed, 31 insertions(+), 46 deletions(-)

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..1fb59fa 100644
--- a/fs/jbd/journal.c
+++ b/fs/jbd/journal.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1939,64 +1940,48 @@ 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;
+   jbd_debugfs_dir = debugfs_create_dir("jbd", NULL);
+   if (jbd_debugfs_dir)
+   jbd_debug = debugfs_create_u8("jbd-debug", S_IRUGO,
+  jbd_debugfs_dir,
+  _enable_debug);
 }
 
-static int write_jbd_debug(struct file *file, const char __user *buffer,
-  unsigned long count, void *data)
+static void __exit remove_jbd_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';
-   journal_enable_debug = simple_strtoul(buf, NULL, 10);
-   return count;
+   if (jbd_debug)
+   debugfs_remove(jbd_debug);
+   if (jbd_debugfs_dir)
+   debugfs_remove(jbd_debugfs_dir);
 }
 
-#define JBD_PROC_NAME "sys/fs/jbd-debug"
+#else
 
-static void __init create_jbd_proc_entry(void)
+static void __init create_jbd_debugfs_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;
-   }
+   do {
+   } while (0);
 }
 
-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);
+   do {
+   } while (0);
 }
 
-#else
-
-#define create_jbd_proc_entry() do {} while (0)
-#define remove_jbd_proc_entry() do {} while (0)
-
 #endif
 
 struct kmem_cache *jbd_handle_cache;
@@ -2054,7 +2039,7 @@ static int __init journal_init(void)
ret = journal_init_caches();
if (ret

Re: jbd : config_jbd_debug cannot create /proc entry

2007-09-25 Thread Jose R. Santos 
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 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -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",

Re: jbd : config_jbd_debug cannot create /proc entry

2007-09-25 Thread Jose R. Santos 
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: jbd : config_jbd_debug cannot create /proc entry

2007-09-25 Thread Jose R. Santos 
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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: jbd : config_jbd_debug cannot create /proc entry

2007-09-25 Thread Jose R. Santos 
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

[PATCH] JBD: Export jbd-debug via debugfs

2007-09-25 Thread Jose R. Santos 
From: Jose R. Santos [EMAIL PROTECTED]

JBD: Export jbd-debug via debugfs

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]
Signed-off-by: Jan Kara [EMAIL PROTECTED]
--

 fs/Kconfig  |   10 
 fs/jbd/journal.c|   65 ---
 include/linux/jbd.h |2 +-
 3 files changed, 31 insertions(+), 46 deletions(-)

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..1fb59fa 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,64 +1940,48 @@ 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;
+   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 int write_jbd_debug(struct file *file, const char __user *buffer,
-  unsigned long count, void *data)
+static void __exit remove_jbd_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';
-   journal_enable_debug = simple_strtoul(buf, NULL, 10);
-   return count;
+   if (jbd_debug)
+   debugfs_remove(jbd_debug);
+   if (jbd_debugfs_dir)
+   debugfs_remove(jbd_debugfs_dir);
 }
 
-#define JBD_PROC_NAME sys/fs/jbd-debug
+#else
 
-static void __init create_jbd_proc_entry(void)
+static void __init create_jbd_debugfs_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;
-   }
+   do {
+   } while (0);
 }
 
-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);
+   do {
+   } while (0);
 }
 
-#else
-
-#define create_jbd_proc_entry() do {} while (0)
-#define remove_jbd_proc_entry() do {} while (0)
-
 #endif
 
 struct kmem_cache *jbd_handle_cache;
@@ -2054,7 +2039,7 @@ static int __init journal_init(void)
ret = journal_init_caches();
if (ret != 0)
journal_destroy_caches();
-   create_jbd_proc_entry

[PATCH] JBD: Fix JBD warnings when compiling with CONFIG_JBD_DEBUG

2007-09-25 Thread Jose R. Santos 
From: Jose R. Santos [EMAIL PROTECTED]

JBD: Fix JBD warnings when compiling with CONFIG_JBD_DEBUG

Note from Mingming's JBD2 fix:

Noticed all warnings are occurs when the debug level is 0. Then found
the jbd2: Move jbd2-debug file to debugfs patch
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=0f49d5d019afa4e94253bfc92f0daca3badb990b

changed the jbd2_journal_enable_debug from int type to u8, makes the
jbd_debug comparision is always true when the debugging level is 0. Thus
the compile warning occurs. 

Thought about changing the jbd2_journal_enable_debug data type back to
int, but can't, because the jbd2-debug is moved to debug fs, where
calling debugfs_create_u8() to create the debugfs entry needs the value
to be u8 type.

Even if we changed the data type back to int, the code is still buggy,
kernel should not print jbd2 debug message if the
jbd2_journal_enable_debug is set to 0. But this is not the case.

The fix is change the level of debugging to 1. The same should fixed in
ext3/JBD, but currently ext3 jbd-debug via /proc fs is broken, so we
probably should fix it all together.

Signed-off-by: Jose R. Santos [EMAIL PROTECTED]
--

 fs/ext3/inode.c   |2 +-
 fs/jbd/recovery.c |6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index de4e316..f5b0e79 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -2887,7 +2887,7 @@ int ext3_write_inode(struct inode *inode, int wait)
return 0;
 
if (ext3_journal_current_handle()) {
-   jbd_debug(0, called recursively, non-PF_MEMALLOC!\n);
+   jbd_debug(1, called recursively, non-PF_MEMALLOC!\n);
dump_stack();
return -EIO;
}
diff --git a/fs/jbd/recovery.c b/fs/jbd/recovery.c
index 2a5f4b8..c5d9694 100644
--- a/fs/jbd/recovery.c
+++ b/fs/jbd/recovery.c
@@ -250,10 +250,10 @@ int journal_recover(journal_t *journal)
if (!err)
err = do_one_pass(journal, info, PASS_REPLAY);
 
-   jbd_debug(0, JBD: recovery, exit status %d, 
+   jbd_debug(1, JBD: recovery, exit status %d, 
  recovered transactions %u to %u\n,
  err, info.start_transaction, info.end_transaction);
-   jbd_debug(0, JBD: Replayed %d and revoked %d/%d blocks\n,
+   jbd_debug(1, JBD: Replayed %d and revoked %d/%d blocks\n,
  info.nr_replays, info.nr_revoke_hits, info.nr_revokes);
 
/* Restart the log at the next transaction ID, thus invalidating
@@ -297,7 +297,7 @@ int journal_skip_recovery(journal_t *journal)
 #ifdef CONFIG_JBD_DEBUG
int dropped = info.end_transaction - 
be32_to_cpu(sb-s_sequence);
 #endif
-   jbd_debug(0,
+   jbd_debug(1,
  JBD: ignoring %d transaction%s from the journal.\n,
  dropped, (dropped == 1) ?  : s);
journal-j_transaction_sequence = ++info.end_transaction;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ext4: FLEX_BG Kernel support v2.

2007-09-21 Thread Jose R. Santos 
On Fri, 21 Sep 2007 13:29:27 -0700
Badari Pulavarty <[EMAIL PROTECTED]> wrote:

> On Fri, 2007-09-21 at 09:06 -0500, Jose R. Santos wrote:
> > From: Jose R. Santos <[EMAIL PROTECTED]>
> > 
> > ext4: FLEX_BG Kernel support v2.
> > 
> 
> > @@ -702,13 +702,15 @@ static inline int ext4_valid_inum(struct super_block 
> > *sb, unsigned long ino)
> >  #define EXT4_FEATURE_INCOMPAT_META_BG  0x0010
> >  #define EXT4_FEATURE_INCOMPAT_EXTENTS  0x0040 /* extents 
> > support */
> >  #define EXT4_FEATURE_INCOMPAT_64BIT0x0080
> > +#define EXT4_FEATURE_INCOMPAT_FLEX_BG  0x0200
> 
> Any reason why 0x100 is skipped ?
> 
> Thanks,
> Badari
> 

Because 0x0100 is reserved for EXT4_FEATURE_INCOMPAT_MMP in e2fsprogs.

-JRS
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] ext4: FLEX_BG Kernel support v2.

2007-09-21 Thread Jose R. Santos 
From: Jose R. Santos <[EMAIL PROTECTED]>

ext4: FLEX_BG Kernel support v2.

This feature relaxes check restrictions on where each block groups meta data is 
located within the storage media.  This allows for the allocation of bitmaps or 
inode tables outside the block group boundaries in cases where bad blocks forces
us to look for new blocks which the owning block group can not satisfy.  This 
will also allow for new meta-data allocation schemes to improve performance and
scalability.

Signed-off-by: Jose R. Santos <[EMAIL PROTECTED]>
--

 fs/ext4/super.c |9 +++--
 include/linux/ext4_fs.h |4 +++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 4550b83..dbce81d 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1248,13 +1248,17 @@ static int ext4_check_descriptors (struct super_block * 
sb)
ext4_fsblk_t inode_table;
struct ext4_group_desc * gdp = NULL;
int desc_block = 0;
+   int flexbg_flag = 0;
int i;
 
+   if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG))
+   flexbg_flag = 1;
+
ext4_debug ("Checking group descriptors");
 
for (i = 0; i < sbi->s_groups_count; i++)
{
-   if (i == sbi->s_groups_count - 1)
+   if (i == sbi->s_groups_count - 1 || flexbg_flag)
last_block = ext4_blocks_count(sbi->s_es) - 1;
else
last_block = first_block +
@@ -1291,7 +1295,8 @@ static int ext4_check_descriptors (struct super_block * 
sb)
i, inode_table);
return 0;
}
-   first_block += EXT4_BLOCKS_PER_GROUP(sb);
+   if (!flexbg_flag)
+   first_block += EXT4_BLOCKS_PER_GROUP(sb);
gdp = (struct ext4_group_desc *)
((__u8 *)gdp + EXT4_DESC_SIZE(sb));
}
diff --git a/include/linux/ext4_fs.h b/include/linux/ext4_fs.h
index cdee7aa..d53e167 100644
--- a/include/linux/ext4_fs.h
+++ b/include/linux/ext4_fs.h
@@ -702,13 +702,15 @@ static inline int ext4_valid_inum(struct super_block *sb, 
unsigned long ino)
 #define EXT4_FEATURE_INCOMPAT_META_BG  0x0010
 #define EXT4_FEATURE_INCOMPAT_EXTENTS  0x0040 /* extents support */
 #define EXT4_FEATURE_INCOMPAT_64BIT0x0080
+#define EXT4_FEATURE_INCOMPAT_FLEX_BG  0x0200
 
 #define EXT4_FEATURE_COMPAT_SUPP   EXT2_FEATURE_COMPAT_EXT_ATTR
 #define EXT4_FEATURE_INCOMPAT_SUPP (EXT4_FEATURE_INCOMPAT_FILETYPE| \
 EXT4_FEATURE_INCOMPAT_RECOVER| \
 EXT4_FEATURE_INCOMPAT_META_BG| \
 EXT4_FEATURE_INCOMPAT_EXTENTS| \
-EXT4_FEATURE_INCOMPAT_64BIT)
+EXT4_FEATURE_INCOMPAT_64BIT| \
+EXT4_FEATURE_INCOMPAT_FLEX_BG)
 #define EXT4_FEATURE_RO_COMPAT_SUPP(EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER| \
 EXT4_FEATURE_RO_COMPAT_LARGE_FILE| \
 EXT4_FEATURE_RO_COMPAT_DIR_NLINK | \
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] ext4: FLEX_BG Kernel support v2.

2007-09-21 Thread Jose R. Santos 
From: Jose R. Santos [EMAIL PROTECTED]

ext4: FLEX_BG Kernel support v2.

This feature relaxes check restrictions on where each block groups meta data is 
located within the storage media.  This allows for the allocation of bitmaps or 
inode tables outside the block group boundaries in cases where bad blocks forces
us to look for new blocks which the owning block group can not satisfy.  This 
will also allow for new meta-data allocation schemes to improve performance and
scalability.

Signed-off-by: Jose R. Santos [EMAIL PROTECTED]
--

 fs/ext4/super.c |9 +++--
 include/linux/ext4_fs.h |4 +++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 4550b83..dbce81d 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1248,13 +1248,17 @@ static int ext4_check_descriptors (struct super_block * 
sb)
ext4_fsblk_t inode_table;
struct ext4_group_desc * gdp = NULL;
int desc_block = 0;
+   int flexbg_flag = 0;
int i;
 
+   if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG))
+   flexbg_flag = 1;
+
ext4_debug (Checking group descriptors);
 
for (i = 0; i  sbi-s_groups_count; i++)
{
-   if (i == sbi-s_groups_count - 1)
+   if (i == sbi-s_groups_count - 1 || flexbg_flag)
last_block = ext4_blocks_count(sbi-s_es) - 1;
else
last_block = first_block +
@@ -1291,7 +1295,8 @@ static int ext4_check_descriptors (struct super_block * 
sb)
i, inode_table);
return 0;
}
-   first_block += EXT4_BLOCKS_PER_GROUP(sb);
+   if (!flexbg_flag)
+   first_block += EXT4_BLOCKS_PER_GROUP(sb);
gdp = (struct ext4_group_desc *)
((__u8 *)gdp + EXT4_DESC_SIZE(sb));
}
diff --git a/include/linux/ext4_fs.h b/include/linux/ext4_fs.h
index cdee7aa..d53e167 100644
--- a/include/linux/ext4_fs.h
+++ b/include/linux/ext4_fs.h
@@ -702,13 +702,15 @@ static inline int ext4_valid_inum(struct super_block *sb, 
unsigned long ino)
 #define EXT4_FEATURE_INCOMPAT_META_BG  0x0010
 #define EXT4_FEATURE_INCOMPAT_EXTENTS  0x0040 /* extents support */
 #define EXT4_FEATURE_INCOMPAT_64BIT0x0080
+#define EXT4_FEATURE_INCOMPAT_FLEX_BG  0x0200
 
 #define EXT4_FEATURE_COMPAT_SUPP   EXT2_FEATURE_COMPAT_EXT_ATTR
 #define EXT4_FEATURE_INCOMPAT_SUPP (EXT4_FEATURE_INCOMPAT_FILETYPE| \
 EXT4_FEATURE_INCOMPAT_RECOVER| \
 EXT4_FEATURE_INCOMPAT_META_BG| \
 EXT4_FEATURE_INCOMPAT_EXTENTS| \
-EXT4_FEATURE_INCOMPAT_64BIT)
+EXT4_FEATURE_INCOMPAT_64BIT| \
+EXT4_FEATURE_INCOMPAT_FLEX_BG)
 #define EXT4_FEATURE_RO_COMPAT_SUPP(EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER| \
 EXT4_FEATURE_RO_COMPAT_LARGE_FILE| \
 EXT4_FEATURE_RO_COMPAT_DIR_NLINK | \
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ext4: FLEX_BG Kernel support v2.

2007-09-21 Thread Jose R. Santos 
On Fri, 21 Sep 2007 13:29:27 -0700
Badari Pulavarty [EMAIL PROTECTED] wrote:

 On Fri, 2007-09-21 at 09:06 -0500, Jose R. Santos wrote:
  From: Jose R. Santos [EMAIL PROTECTED]
  
  ext4: FLEX_BG Kernel support v2.
  
 
  @@ -702,13 +702,15 @@ static inline int ext4_valid_inum(struct super_block 
  *sb, unsigned long ino)
   #define EXT4_FEATURE_INCOMPAT_META_BG  0x0010
   #define EXT4_FEATURE_INCOMPAT_EXTENTS  0x0040 /* extents 
  support */
   #define EXT4_FEATURE_INCOMPAT_64BIT0x0080
  +#define EXT4_FEATURE_INCOMPAT_FLEX_BG  0x0200
 
 Any reason why 0x100 is skipped ?
 
 Thanks,
 Badari
 

Because 0x0100 is reserved for EXT4_FEATURE_INCOMPAT_MMP in e2fsprogs.

-JRS
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [EXT4 set 2][PATCH 5/5] cleanups: Export jbd2-debug via debugfs

2007-07-11 Thread Jose R. Santos 
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 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/jbd2/jbd2-debug.


Signed-off-by: Jose R. Santos <[EMAIL PROTECTED]>
---
 fs/Kconfig   |   105 + 5 - 0 !
 fs/jbd2/journal.c|   6727 +40 -0 !
 include/linux/jbd2.h |21 + 1 - 0 !
 3 files changed, 33 insertions(+), 46 deletions(-)

Index: linux-2.6/fs/jbd2/journal.c
===
--- linux-2.6.orig/fs/jbd2/journal.c2007-07-11 09:46:25.0 -0500
+++ linux-2.6/fs/jbd2/journal.c 2007-07-11 11:31:30.0 -0500
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1951,64 +1952,50 @@ void jbd2_journal_put_journal_head(struc
 }
 
 /*
- * /proc tunables
+ * debugfs tunables
  */
 #if defined(CONFIG_JBD2_DEBUG)
-int jbd2_journal_enable_debug;
+u8 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;
+struct dentry *jbd2_debugfs_dir, *jbd2_debug;
 
-   ret = sprintf(page + off, "%d\n", jbd2_journal_enable_debug);
-   *eof = 1;
-   return ret;
+static void __init jbd2_create_debugfs_entry(void)
+{
+   jbd2_debugfs_dir = debugfs_create_dir("jbd2", NULL);
+   if (jbd2_debugfs_dir)
+   jbd2_debug = debugfs_create_u8(JBD2_DEBUG_NAME, S_IRUGO,
+  jbd2_debugfs_dir,
+  _journal_enable_debug);
 }
 
-static int write_jbd_debug(struct file *file, const char __user *buffer,
-  unsigned long count, void *data)
+static void __exit jbd2_remove_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;
+   if (jbd2_debug)
+   debugfs_remove(jbd2_debug);
+   if (jbd2_debugfs_dir)
+   debugfs_remove(jbd2_debugfs_dir);
 }
 
-#define JBD_PROC_NAME "sys/fs/jbd2-debug"
+#else
 
-static void __init create_jbd_proc_entry(void)
+static void __init jbd2_create_debugfs_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;
-   }
+   do {
+   } while (0);
 }
 
-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);
+   do {
+   } while (0);
 }
 
-#else
-
-#define create_jbd_proc_entry() do {} while (0)
-#define jbd2_remove_jbd_proc_entry() do {} while (0)
-
 #endif
 
 struct kmem_cache *jbd2_handle_cache;
@@ -2067,7 +2054,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();
return ret;
 }
 
@@ -2078,7 +2065,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_journal_destroy_caches();
 }
 
Index: linux-2.6/include/linux/jbd2.h
===
--- linux-2.6.orig/include/linux/jbd2.h 2007-07-11 09:46:25.0 -0500
+++ linux-2.6/include/linux/jbd2.h  2007-07-11 10:37:06.0 -0500
@@ -57,7 +57,7 @@
  * CONFIG_JBD2_DEBUG is on.
  */
 #define JBD_EXPENSIVE_CHECKING
-extern int jbd2_journal_enable_debug;
+extern u8 jbd2_journal_enable_debug;
 
 #define jbd_debug(n, f, a...)  \
do {\
Index: linux-2.6/fs/Kconfig
===
---

Re: [EXT4 set 2][PATCH 3/5] cleanups: set_jbd2_64bit_feature for >16TB ext4 fs

2007-07-11 Thread Jose R. Santos 
Set the journals JBD2_FEATURE_INCOMPAT_64BIT on devices with more
than 32bit block sizes during mount time.  This ensure proper record
lenth when writing to the journal.

Signed-off-by: Jose R. Santos <[EMAIL PROTECTED]>
Signed-off-by: Andreas Dilger <[EMAIL PROTECTED]>
Signed-off-by: Mingming Cao <[EMAIL PROTECTED]>
Signed-off-by: Laurent Vivier <[EMAIL PROTECTED]>
---
 fs/ext4/super.c |7 7 + 0 - 0 !
 1 file changed, 7 insertions(+)

Index: linux-2.6/fs/ext4/super.c
===
--- linux-2.6.orig/fs/ext4/super.c  2007-07-11 09:36:00.0 -0500
+++ linux-2.6/fs/ext4/super.c   2007-07-11 09:36:20.0 -0500
@@ -1804,6 +1804,13 @@ static int ext4_fill_super (struct super
goto failed_mount3;
}
 
+   if (ext4_blocks_count(es) > 0xULL &&
+   !jbd2_journal_set_features(EXT4_SB(sb)->s_journal, 0, 0,
+  JBD2_FEATURE_INCOMPAT_64BIT)) {
+   printk(KERN_ERR "ext4: Failed to set 64-bit journal feature\n");
+   goto failed_mount4;
+   }
+
/* We have now updated the journal if required, so we can
 * validate the data journaling mode. */
switch (test_opt(sb, DATA_FLAGS)) {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [EXT4 set 2][PATCH 3/5] cleanups: set_jbd2_64bit_feature for 16TB ext4 fs

2007-07-11 Thread Jose R. Santos 
Set the journals JBD2_FEATURE_INCOMPAT_64BIT on devices with more
than 32bit block sizes during mount time.  This ensure proper record
lenth when writing to the journal.

Signed-off-by: Jose R. Santos [EMAIL PROTECTED]
Signed-off-by: Andreas Dilger [EMAIL PROTECTED]
Signed-off-by: Mingming Cao [EMAIL PROTECTED]
Signed-off-by: Laurent Vivier [EMAIL PROTECTED]
---
 fs/ext4/super.c |7 7 + 0 - 0 !
 1 file changed, 7 insertions(+)

Index: linux-2.6/fs/ext4/super.c
===
--- linux-2.6.orig/fs/ext4/super.c  2007-07-11 09:36:00.0 -0500
+++ linux-2.6/fs/ext4/super.c   2007-07-11 09:36:20.0 -0500
@@ -1804,6 +1804,13 @@ static int ext4_fill_super (struct super
goto failed_mount3;
}
 
+   if (ext4_blocks_count(es)  0xULL 
+   !jbd2_journal_set_features(EXT4_SB(sb)-s_journal, 0, 0,
+  JBD2_FEATURE_INCOMPAT_64BIT)) {
+   printk(KERN_ERR ext4: Failed to set 64-bit journal feature\n);
+   goto failed_mount4;
+   }
+
/* We have now updated the journal if required, so we can
 * validate the data journaling mode. */
switch (test_opt(sb, DATA_FLAGS)) {
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [EXT4 set 2][PATCH 5/5] cleanups: Export jbd2-debug via debugfs

2007-07-11 Thread Jose R. Santos 
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 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/jbd2/jbd2-debug.


Signed-off-by: Jose R. Santos [EMAIL PROTECTED]
---
 fs/Kconfig   |   105 + 5 - 0 !
 fs/jbd2/journal.c|   6727 +40 -0 !
 include/linux/jbd2.h |21 + 1 - 0 !
 3 files changed, 33 insertions(+), 46 deletions(-)

Index: linux-2.6/fs/jbd2/journal.c
===
--- linux-2.6.orig/fs/jbd2/journal.c2007-07-11 09:46:25.0 -0500
+++ linux-2.6/fs/jbd2/journal.c 2007-07-11 11:31:30.0 -0500
@@ -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
@@ -1951,64 +1952,50 @@ void jbd2_journal_put_journal_head(struc
 }
 
 /*
- * /proc tunables
+ * debugfs tunables
  */
 #if defined(CONFIG_JBD2_DEBUG)
-int jbd2_journal_enable_debug;
+u8 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;
+struct dentry *jbd2_debugfs_dir, *jbd2_debug;
 
-   ret = sprintf(page + off, %d\n, jbd2_journal_enable_debug);
-   *eof = 1;
-   return ret;
+static void __init jbd2_create_debugfs_entry(void)
+{
+   jbd2_debugfs_dir = debugfs_create_dir(jbd2, NULL);
+   if (jbd2_debugfs_dir)
+   jbd2_debug = debugfs_create_u8(JBD2_DEBUG_NAME, S_IRUGO,
+  jbd2_debugfs_dir,
+  jbd2_journal_enable_debug);
 }
 
-static int write_jbd_debug(struct file *file, const char __user *buffer,
-  unsigned long count, void *data)
+static void __exit jbd2_remove_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;
+   if (jbd2_debug)
+   debugfs_remove(jbd2_debug);
+   if (jbd2_debugfs_dir)
+   debugfs_remove(jbd2_debugfs_dir);
 }
 
-#define JBD_PROC_NAME sys/fs/jbd2-debug
+#else
 
-static void __init create_jbd_proc_entry(void)
+static void __init jbd2_create_debugfs_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;
-   }
+   do {
+   } while (0);
 }
 
-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);
+   do {
+   } while (0);
 }
 
-#else
-
-#define create_jbd_proc_entry() do {} while (0)
-#define jbd2_remove_jbd_proc_entry() do {} while (0)
-
 #endif
 
 struct kmem_cache *jbd2_handle_cache;
@@ -2067,7 +2054,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();
return ret;
 }
 
@@ -2078,7 +2065,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_journal_destroy_caches();
 }
 
Index: linux-2.6/include/linux/jbd2.h
===
--- linux-2.6.orig/include/linux/jbd2.h 2007-07-11 09:46:25.0 -0500
+++ linux-2.6/include/linux/jbd2.h  2007-07-11 10:37:06.0 -0500
@@ -57,7 +57,7 @@
  * CONFIG_JBD2_DEBUG is on.
  */
 #define JBD_EXPENSIVE_CHECKING
-extern int jbd2_journal_enable_debug;
+extern u8 jbd2_journal_enable_debug;
 
 #define jbd_debug(n, f, a...)  \
do {\
Index: linux-2.6/fs/Kconfig
===
--- linux-2.6.orig/fs

Re: [EXT4 set 2][PATCH 5/5] cleanups: Export jbd2-debug via debugfs

2007-07-10 Thread Jose R. Santos 
On Tue, 10 Jul 2007 16:30:25 -0700
Andrew Morton <[EMAIL PROTECTED]> wrote:

> On Sun, 01 Jul 2007 03:36:48 -0400
> Mingming Cao <[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]>
> 
> Please clean up the changelog.
> 
> The changelog should include information about the location and the content
> of these debugfs files.  it should provide any instructions which users
> will need to be able to create and use those files.

Will fix.

> Alternatively (and preferably) do this via an update to
> Documentation/filesystems/ext4.txt.

Seems like I also need to update the doc on Kconfig as well.  Do you
prefer this in separate patches? (current patch, kconfig patch, ext4
doc update patch?
 
> >  fs/jbd2/journal.c|   6220 +42 -0 !
> >  include/linux/jbd2.h |21 + 1 - 0 !
> >  2 files changed, 21 insertions(+), 43 deletions(-)
> 
> Again, this patch isn't in Ted's kernel.org directory and hasn't been in -mm.
> 
> Apart from the lack of testing and review which this causes, it means I
> can't just do `pushpatch name-of-this-patch' and look at it in tkdiff.  So
> I squint at the diff, but that's harder when the diff wasn't prepared with
> `diff -p'.  Oh well.

Will fix.

> 
> > Index: linux-2.6.22-rc4/fs/jbd2/journal.c
> > ===
> > --- linux-2.6.22-rc4.orig/fs/jbd2/journal.c 2007-06-11 16:16:18.0 
> > -0700
> > +++ linux-2.6.22-rc4/fs/jbd2/journal.c  2007-06-11 16:36:10.0 
> > -0700
> > @@ -35,6 +35,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #include 
> >  #include 
> > @@ -1954,60 +1955,37 @@
> >   * /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)
> 
> Has this been compile-tested with CONFIG_DEBUGFS=n?

I think I did, but honestly don't remember.  Will check with the new
patch. :) 

> >  
> > -#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)
> 
> I suggest that these be converted to (preferable) inline functions while
> you're there.

OK.

> >  #endif
> >  
> > @@ -2067,7 +2045,7 @@
> > ret = journal_init_caches();
> > if (ret != 0)
> > jbd2_journal_destroy_caches();
> > -   create_jbd_proc_entry();
> > +   jbd2_create_debugfs_entry();
> > return ret;
> >  }
> >  
> > @@ -2078,7 +2056,7 @@
> > if (n)
> > printk(KERN_EMERG "JBD: leaked %d journal_heads!\n", n);
> >  #endif
> > -   jbd2_remove_jbd_proc_entry();
> > +   jbd2_remove_debugfs_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-11 
> > 16:16:18.0 -0700
> > +++ linux-2.6.22-rc4/include/linux/jbd2.h   2007-06-11 16:35:25.0 
> > -0700
> > @@ -57,7 +57,7 @@
> >   * CONFIG_JBD2_DEBUG is on.
> >   */
> >  #define JBD_EXPENSIVE_CHECKING
> 
> JBD2?
>
> > -extern int jbd2_j

Re: [EXT4 set 2][PATCH 5/5] cleanups: Export jbd2-debug via debugfs

2007-07-10 Thread Jose R. Santos 
On Tue, 10 Jul 2007 16:30:25 -0700
Andrew Morton [EMAIL PROTECTED] wrote:

 On Sun, 01 Jul 2007 03:36:48 -0400
 Mingming Cao [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]
 
 Please clean up the changelog.
 
 The changelog should include information about the location and the content
 of these debugfs files.  it should provide any instructions which users
 will need to be able to create and use those files.

Will fix.

 Alternatively (and preferably) do this via an update to
 Documentation/filesystems/ext4.txt.

Seems like I also need to update the doc on Kconfig as well.  Do you
prefer this in separate patches? (current patch, kconfig patch, ext4
doc update patch?
 
   fs/jbd2/journal.c|   6220 +42 -0 !
   include/linux/jbd2.h |21 + 1 - 0 !
   2 files changed, 21 insertions(+), 43 deletions(-)
 
 Again, this patch isn't in Ted's kernel.org directory and hasn't been in -mm.
 
 Apart from the lack of testing and review which this causes, it means I
 can't just do `pushpatch name-of-this-patch' and look at it in tkdiff.  So
 I squint at the diff, but that's harder when the diff wasn't prepared with
 `diff -p'.  Oh well.

Will fix.

 
  Index: linux-2.6.22-rc4/fs/jbd2/journal.c
  ===
  --- linux-2.6.22-rc4.orig/fs/jbd2/journal.c 2007-06-11 16:16:18.0 
  -0700
  +++ linux-2.6.22-rc4/fs/jbd2/journal.c  2007-06-11 16:36:10.0 
  -0700
  @@ -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
  @@ -1954,60 +1955,37 @@
* /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)
 
 Has this been compile-tested with CONFIG_DEBUGFS=n?

I think I did, but honestly don't remember.  Will check with the new
patch. :) 

   
  -#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)
 
 I suggest that these be converted to (preferable) inline functions while
 you're there.

OK.

   #endif
   
  @@ -2067,7 +2045,7 @@
  ret = journal_init_caches();
  if (ret != 0)
  jbd2_journal_destroy_caches();
  -   create_jbd_proc_entry();
  +   jbd2_create_debugfs_entry();
  return ret;
   }
   
  @@ -2078,7 +2056,7 @@
  if (n)
  printk(KERN_EMERG JBD: leaked %d journal_heads!\n, n);
   #endif
  -   jbd2_remove_jbd_proc_entry();
  +   jbd2_remove_debugfs_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-11 
  16:16:18.0 -0700
  +++ linux-2.6.22-rc4/include/linux/jbd2.h   2007-06-11 16:35:25.0 
  -0700
  @@ -57,7 +57,7 @@
* CONFIG_JBD2_DEBUG is on.
*/
   #define JBD_EXPENSIVE_CHECKING
 
 JBD2?

  -extern int jbd2_journal_enable_debug;
  +extern u16 jbd2_journal_enable_debug;
 
 Why was this made 16-bit?  To save 2 bytes?  Could have saved 3 if we're
 going to do that.

OK.
 
 Shoudln't all this debug info be a per-superblock thing rather than
 kernel-wide?

I don't think it is worth pursuing this feature since this seems to
have been broken for a while now (its been there since the first git
revission in ext3) and nobody has noticed it until now.  It could be
address on a later patch though, since the initial purpose of the patch
was to fix the broken JBD2_DEBUG option. Of course, this may not be
clearly express in the changelog. :)

-JRS
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read

Re: [EXT4 set 6][PATCH 1/1]Export jbd stats through procfs

2007-07-01 Thread Jose R. Santos 
On Sun, 01 Jul 2007 03:38:10 -0400
Mingming Cao <[EMAIL PROTECTED]> wrote:

> [PATCH] jbd2 stats through procfs
> 
> The patch below updates the jbd stats patch to 2.6.20/jbd2.
> The initial patch was posted by Alex Tomas in December 2005
> (http://marc.info/?l=linux-ext4=113538565128617=2).
> It provides statistics via procfs such as transaction lifetime and size.
> 
> [ This probably should be rewritten to use debugfs?   -- Ted]

Was a decision ever made on whether this should remain on procfs or be
move to use debugfs?  I can recall this being discuss but don't recall
a firm decision on it.

-JRS
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [EXT4 set 6][PATCH 1/1]Export jbd stats through procfs

2007-07-01 Thread Jose R. Santos 
On Sun, 01 Jul 2007 03:38:10 -0400
Mingming Cao [EMAIL PROTECTED] wrote:

 [PATCH] jbd2 stats through procfs
 
 The patch below updates the jbd stats patch to 2.6.20/jbd2.
 The initial patch was posted by Alex Tomas in December 2005
 (http://marc.info/?l=linux-ext4m=113538565128617w=2).
 It provides statistics via procfs such as transaction lifetime and size.
 
 [ This probably should be rewritten to use debugfs?   -- Ted]

Was a decision ever made on whether this should remain on procfs or be
move to use debugfs?  I can recall this being discuss but don't recall
a firm decision on it.

-JRS
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/