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

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

[Mingming Cao]

> 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 <jrs@us.ibm.com>
---
 fs/jbd2/journal.c    |   62    20 +    42 -    0 !
 include/linux/jbd2.h |    2    1 +     1 -     0 !
 2 files changed, 21 insertions(+), 43 deletions(-)

Index: linux-2.6.22-rc4/fs/jbd2/journal.c
===================================================================
--- linux-2.6.22-rc4.orig/fs/jbd2/journal.c	2007-06-11 16:16:18.000000000 -0700
+++ linux-2.6.22-rc4/fs/jbd2/journal.c	2007-06-11 16:36:10.000000000 -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)
 
-static struct proc_dir_entry *proc_jbd_debug;
+#define JBD2_DEBUG_NAME "jbd2-debug"
 
-static int read_jbd_debug(char *page, char **start, off_t off,
-			  int count, int *eof, void *data)
-{
-	int ret;
-
-	ret = sprintf(page + off, "%d\n", jbd2_journal_enable_debug);
-	*eof = 1;
-	return ret;
-}
+struct dentry *jbd2_debugfs_dir, *jbd2_debug;
 
-static int write_jbd_debug(struct file *file, const char __user *buffer,
-			   unsigned long count, void *data)
+static void __init jbd2_create_debugfs_entry(void)
 {
-	char buf[32];
-
-	if (count > ARRAY_SIZE(buf) - 1)
-		count = ARRAY_SIZE(buf) - 1;
-	if (copy_from_user(buf, buffer, count))
-		return -EFAULT;
-	buf[ARRAY_SIZE(buf) - 1] = '\0';
-	jbd2_journal_enable_debug = simple_strtoul(buf, NULL, 10);
-	return count;
-}
-
-#define JBD_PROC_NAME "sys/fs/jbd2-debug"
-
-static void __init create_jbd_proc_entry(void)
-{
-	proc_jbd_debug = create_proc_entry(JBD_PROC_NAME, 0644, NULL);
-	if (proc_jbd_debug) {
-		/* Why is this so hard? */
-		proc_jbd_debug->read_proc = read_jbd_debug;
-		proc_jbd_debug->write_proc = write_jbd_debug;
-	}
+	jbd2_debugfs_dir = debugfs_create_dir("jbd2", NULL);
+	if (jbd2_debugfs_dir)
+		jbd2_debug = debugfs_create_u16(JBD2_DEBUG_NAME, S_IRUGO,
+						jbd2_debugfs_dir,
+						&jbd2_journal_enable_debug);
 }
 
-static void __exit jbd2_remove_jbd_proc_entry(void)
+static void __exit jbd2_remove_debugfs_entry(void)
 {
-	if (proc_jbd_debug)
-		remove_proc_entry(JBD_PROC_NAME, NULL);
+	if (jbd2_debug)
+		debugfs_remove(jbd2_debug);
+	if (jbd2_debugfs_dir)
+		debugfs_remove(jbd2_debugfs_dir);
 }
 
 #else
 
-#define create_jbd_proc_entry() do {} while (0)
-#define jbd2_remove_jbd_proc_entry() do {} while (0)
+#define jbd2_create_debugfs_entry() do {} while (0)
+#define jbd2_remove_debugfs_entry() do {} while (0)
 
 #endif
 
@@ -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.000000000 -0700
+++ linux-2.6.22-rc4/include/linux/jbd2.h	2007-06-11 16:35:25.000000000 -0700
@@ -57,7 +57,7 @@
  * CONFIG_JBD2_DEBUG is on.
  */
 #define JBD_EXPENSIVE_CHECKING
-extern int jbd2_journal_enable_debug;
+extern u16 jbd2_journal_enable_debug;
 
 #define jbd_debug(n, f, a...)						\
 	do {								\

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

[Andrew Morton]

On Sun, 01 Jul 2007 03:36:48 -0400
Mingming Cao <cmm@us.ibm.com> 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 <jrs@us.ibm.com>

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.

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

>  fs/jbd2/journal.c    |   62    20 +    42 -    0 !
>  include/linux/jbd2.h |    2    1 +     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.


> 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.000000000 -0700
> +++ linux-2.6.22-rc4/fs/jbd2/journal.c	2007-06-11 16:36:10.000000000 -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?

>  
> -#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.

>  #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.000000000 -0700
> +++ linux-2.6.22-rc4/include/linux/jbd2.h	2007-06-11 16:35:25.000000000 -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.


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

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

[Jose R. Santos]

On Tue, 10 Jul 2007 16:30:25 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Sun, 01 Jul 2007 03:36:48 -0400
> Mingming Cao <cmm@us.ibm.com> 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 <jrs@us.ibm.com>
> 
> 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    |   62    20 +    42 -    0 !
> >  include/linux/jbd2.h |    2    1 +     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.000000000 -0700
> > +++ linux-2.6.22-rc4/fs/jbd2/journal.c	2007-06-11 16:36:10.000000000 -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.000000000 -0700
> > +++ linux-2.6.22-rc4/include/linux/jbd2.h	2007-06-11 16:35:25.000000000 -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

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

[Mingming Cao]

On Wed, 2007-07-11 at 00:38 -0500, Jose R. Santos wrote:
> On Tue, 10 Jul 2007 16:30:25 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Sun, 01 Jul 2007 03:36:48 -0400
> > Mingming Cao <cmm@us.ibm.com> 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 <jrs@us.ibm.com>
> > 
> > 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    |   62    20 +    42 -    0 !
> > >  include/linux/jbd2.h |    2    1 +     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.000000000 -0700
> > > +++ linux-2.6.22-rc4/fs/jbd2/journal.c	2007-06-11 16:36:10.000000000 -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. :) 
> 

Yes, I remember I did, that discovered some inconsistency in ext4 code,
which has already been fixed.

> > >  
> > > -#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.000000000 -0700
> > > +++ linux-2.6.22-rc4/include/linux/jbd2.h	2007-06-11 16:35:25.000000000 -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

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

[Andrew Morton]

On Wed, 11 Jul 2007 00:38:09 -0500 "Jose R. Santos" <jrs@us.ibm.com> wrote:

> 
> > 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?

All these changes are logically connected (aren't they?).  A single patch
is fine.

> > 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. :)
> 

I don't think that making it all per-superblock is worth the effort - it's
a developer-only thing and developer will have the knowledge to test ext4
on an otherwise-ext3 setup if they're really fussed about the accuracy.

So yes, a bare make-it-work patch sounds appropriate.  Or remove it, but
hey, it might be useful.  The timestamping stuff certainly looks useful.

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

[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 <jrs@us.ibm.com>
---
 fs/Kconfig           |   10 	5 +	5 -	0 !
 fs/jbd2/journal.c    |   67 	27 +	40 -	0 !
 include/linux/jbd2.h |    2 	1 +	1 -	0 !
 3 files changed, 33 insertions(+), 46 deletions(-)

Index: linux-2.6/fs/jbd2/journal.c
===================================================================
--- linux-2.6.orig/fs/jbd2/journal.c	2007-07-11 09:46:25.000000000 -0500
+++ linux-2.6/fs/jbd2/journal.c	2007-07-11 11:31:30.000000000 -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.000000000 -0500
+++ linux-2.6/include/linux/jbd2.h	2007-07-11 10:37:06.000000000 -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/Kconfig	2007-06-22 09:45:51.000000000 -0500
+++ linux-2.6/fs/Kconfig	2007-07-11 12:23:04.000000000 -0500
@@ -251,7 +251,7 @@ config JBD2
 
 config JBD2_DEBUG
 	bool "JBD2 (ext4dev/ext4) debugging support"
-	depends on JBD2
+	depends on JBD2 && DEBUG_FS
 	help
 	  If you are using the ext4dev/ext4 journaled file system (or
 	  potentially any other filesystem/device using JBD2), this option
@@ -260,10 +260,10 @@ config JBD2_DEBUG
 	  By default, the debugging output will be turned off.
 
 	  If you select Y here, then you will be able to turn on debugging
-	  with "echo N > /proc/sys/fs/jbd2-debug", where N is a number between
-	  1 and 5. The higher the number, the more debugging output is
-	  generated.  To turn debugging off again, do
-	  "echo 0 > /proc/sys/fs/jbd2-debug".
+	  with "echo N > /sys/kernel/debug/jbd2/jbd2-debug", where N is a
+	  number between 1 and 5. The higher the number, the more debugging
+	  output is generated.  To turn debugging off again, do
+	  "echo 0 > /sys/kernel/debug/jbd2/jbd2-debug".
 
 config FS_MBCACHE
 # Meta block cache for Extended Attributes (ext2/ext3/ext4)

[PATCH 1/1] ext4: JBD->JBD2 naming cleanups

[Mingming Cao]

On Tue, 2007-07-10 at 16:30 -0700, Andrew Morton wrote:

> > 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.000000000 -0700
> > +++ linux-2.6.22-rc4/include/linux/jbd2.h	2007-06-11 16:35:25.000000000 -0700
> > @@ -57,7 +57,7 @@
> >   * CONFIG_JBD2_DEBUG is on.
> >   */
> >  #define JBD_EXPENSIVE_CHECKING
> 
> JBD2?
> 

Some cleanups in ext4/JBD2 to follow the naming fules:change micros name
from JBD_XXX to JBD2_XXX.


Signed-off-by: Mingming Cao <cmm@us.ibm.com>
---
 fs/ext4/extents.c         |    2 +-
 fs/ext4/super.c           |    2 +-
 fs/jbd2/commit.c          |    2 +-
 fs/jbd2/journal.c         |   26 +++++++++++++-------------
 fs/jbd2/recovery.c        |    2 +-
 fs/jbd2/revoke.c          |    4 ++--
 include/linux/ext4_jbd2.h |    6 +++---
 include/linux/jbd2.h      |   30 +++++++++++++++---------------
 8 files changed, 37 insertions(+), 37 deletions(-)

Index: linux-2.6.22/fs/ext4/super.c
===================================================================
--- linux-2.6.22.orig/fs/ext4/super.c	2007-07-13 17:17:29.000000000 -0700
+++ linux-2.6.22/fs/ext4/super.c	2007-07-13 17:17:33.000000000 -0700
@@ -967,7 +967,7 @@ static int parse_options (char *options,
 			if (option < 0)
 				return 0;
 			if (option == 0)
-				option = JBD_DEFAULT_MAX_COMMIT_AGE;
+				option = JBD2_DEFAULT_MAX_COMMIT_AGE;
 			sbi->s_commit_interval = HZ * option;
 			break;
 		case Opt_data_journal:
Index: linux-2.6.22/include/linux/ext4_jbd2.h
===================================================================
--- linux-2.6.22.orig/include/linux/ext4_jbd2.h	2007-07-13 17:02:08.000000000 -0700
+++ linux-2.6.22/include/linux/ext4_jbd2.h	2007-07-13 17:39:41.000000000 -0700
@@ -12,8 +12,8 @@
  * Ext4-specific journaling extensions.
  */
 
-#ifndef _LINUX_EXT4_JBD_H
-#define _LINUX_EXT4_JBD_H
+#ifndef _LINUX_EXT4_JBD2_H
+#define _LINUX_EXT4_JBD2_H
 
 #include <linux/fs.h>
 #include <linux/jbd2.h>
@@ -228,4 +228,4 @@ static inline int ext4_should_writeback_
 	return 0;
 }
 
-#endif	/* _LINUX_EXT4_JBD_H */
+#endif	/* _LINUX_EXT4_JBD2_H */
Index: linux-2.6.22/include/linux/jbd2.h
===================================================================
--- linux-2.6.22.orig/include/linux/jbd2.h	2007-07-13 17:17:28.000000000 -0700
+++ linux-2.6.22/include/linux/jbd2.h	2007-07-13 17:31:33.000000000 -0700
@@ -13,8 +13,8 @@
  * filesystem journaling support.
  */
 
-#ifndef _LINUX_JBD_H
-#define _LINUX_JBD_H
+#ifndef _LINUX_JBD2_H
+#define _LINUX_JBD2_H
 
 /* Allow this file to be included directly into e2fsprogs */
 #ifndef __KERNEL__
@@ -37,26 +37,26 @@
 #define journal_oom_retry 1
 
 /*
- * Define JBD_PARANIOD_IOFAIL to cause a kernel BUG() if ext3 finds
+ * Define JBD2_PARANIOD_IOFAIL to cause a kernel BUG() if ext4 finds
  * certain classes of error which can occur due to failed IOs.  Under
- * normal use we want ext3 to continue after such errors, because
+ * normal use we want ext4 to continue after such errors, because
  * hardware _can_ fail, but for debugging purposes when running tests on
  * known-good hardware we may want to trap these errors.
  */
-#undef JBD_PARANOID_IOFAIL
+#undef JBD2_PARANOID_IOFAIL
 
 /*
  * The default maximum commit age, in seconds.
  */
-#define JBD_DEFAULT_MAX_COMMIT_AGE 5
+#define JBD2_DEFAULT_MAX_COMMIT_AGE 5
 
 #ifdef CONFIG_JBD2_DEBUG
 /*
- * Define JBD_EXPENSIVE_CHECKING to enable more expensive internal
+ * Define JBD2_EXPENSIVE_CHECKING to enable more expensive internal
  * consistency checks.  By default we don't do this unless
  * CONFIG_JBD2_DEBUG is on.
  */
-#define JBD_EXPENSIVE_CHECKING
+#define JBD2_EXPENSIVE_CHECKING
 extern u8 jbd2_journal_enable_debug;
 
 #define jbd_debug(n, f, a...)						\
@@ -185,8 +185,8 @@ typedef struct journal_block_tag_s
 	__be32		t_blocknr_high; /* most-significant high 32bits. */
 } journal_block_tag_t;
 
-#define JBD_TAG_SIZE32 (offsetof(journal_block_tag_t, t_blocknr_high))
-#define JBD_TAG_SIZE64 (sizeof(journal_block_tag_t))
+#define JBD2_TAG_SIZE32 (offsetof(journal_block_tag_t, t_blocknr_high))
+#define JBD2_TAG_SIZE64 (sizeof(journal_block_tag_t))
 
 /*
  * The revoke descriptor: used on disk to describe a series of blocks to
@@ -282,8 +282,8 @@ typedef struct journal_superblock_s
 #include <linux/fs.h>
 #include <linux/sched.h>
 
-#define JBD_ASSERTIONS
-#ifdef JBD_ASSERTIONS
+#define JBD2_ASSERTIONS
+#ifdef JBD2_ASSERTIONS
 #define J_ASSERT(assert)						\
 do {									\
 	if (!(assert)) {						\
@@ -310,9 +310,9 @@ void buffer_assertion_failure(struct buf
 
 #else
 #define J_ASSERT(assert)	do { } while (0)
-#endif		/* JBD_ASSERTIONS */
+#endif		/* JBD2_ASSERTIONS */
 
-#if defined(JBD_PARANOID_IOFAIL)
+#if defined(JBD2_PARANOID_IOFAIL)
 #define J_EXPECT(expr, why...)		J_ASSERT(expr)
 #define J_EXPECT_BH(bh, expr, why...)	J_ASSERT_BH(bh, expr)
 #define J_EXPECT_JH(jh, expr, why...)	J_ASSERT_JH(jh, expr)
@@ -1224,4 +1224,4 @@ extern int jbd_blocks_per_page(struct in
 
 #endif	/* __KERNEL__ */
 
-#endif	/* _LINUX_JBD_H */
+#endif	/* _LINUX_JBD2_H */
Index: linux-2.6.22/fs/jbd2/commit.c
===================================================================
--- linux-2.6.22.orig/fs/jbd2/commit.c	2007-07-13 17:20:12.000000000 -0700
+++ linux-2.6.22/fs/jbd2/commit.c	2007-07-13 17:20:39.000000000 -0700
@@ -356,7 +356,7 @@ static inline void write_tag_block(int t
 				   unsigned long long block)
 {
 	tag->t_blocknr = cpu_to_be32(block & (u32)~0);
-	if (tag_bytes > JBD_TAG_SIZE32)
+	if (tag_bytes > JBD2_TAG_SIZE32)
 		tag->t_blocknr_high = cpu_to_be32((block >> 31) >> 1);
 }
 
Index: linux-2.6.22/fs/jbd2/journal.c
===================================================================
--- linux-2.6.22.orig/fs/jbd2/journal.c	2007-07-13 17:20:56.000000000 -0700
+++ linux-2.6.22/fs/jbd2/journal.c	2007-07-13 17:21:45.000000000 -0700
@@ -974,7 +974,7 @@ static journal_t * journal_init_common (
 	spin_lock_init(&journal->j_list_lock);
 	spin_lock_init(&journal->j_state_lock);
 
-	journal->j_commit_interval = (HZ * JBD_DEFAULT_MAX_COMMIT_AGE);
+	journal->j_commit_interval = (HZ * JBD2_DEFAULT_MAX_COMMIT_AGE);
 
 	/* The journal is marked for error until we succeed with recovery! */
 	journal->j_flags = JBD2_ABORT;
@@ -1957,9 +1957,9 @@ int jbd2_journal_blocks_per_page(struct 
 size_t journal_tag_bytes(journal_t *journal)
 {
 	if (JBD2_HAS_INCOMPAT_FEATURE(journal, JBD2_FEATURE_INCOMPAT_64BIT))
-		return JBD_TAG_SIZE64;
+		return JBD2_TAG_SIZE64;
 	else
-		return JBD_TAG_SIZE32;
+		return JBD2_TAG_SIZE32;
 }
 
 /*
@@ -1979,11 +1979,11 @@ void * __jbd2_kmalloc (const char *where
  * cause bh to cross page boundary.
  */
 
-#define JBD_MAX_SLABS 5
-#define JBD_SLAB_INDEX(size)  (size >> 11)
+#define JBD2_MAX_SLABS 5
+#define JBD2_SLAB_INDEX(size)  (size >> 11)
 
-static struct kmem_cache *jbd_slab[JBD_MAX_SLABS];
-static const char *jbd_slab_names[JBD_MAX_SLABS] = {
+static struct kmem_cache *jbd_slab[JBD2_MAX_SLABS];
+static const char *jbd_slab_names[JBD2_MAX_SLABS] = {
 	"jbd2_1k", "jbd2_2k", "jbd2_4k", NULL, "jbd2_8k"
 };
 
@@ -1991,7 +1991,7 @@ static void jbd2_journal_destroy_jbd_sla
 {
 	int i;
 
-	for (i = 0; i < JBD_MAX_SLABS; i++) {
+	for (i = 0; i < JBD2_MAX_SLABS; i++) {
 		if (jbd_slab[i])
 			kmem_cache_destroy(jbd_slab[i]);
 		jbd_slab[i] = NULL;
@@ -2000,9 +2000,9 @@ static void jbd2_journal_destroy_jbd_sla
 
 static int jbd2_journal_create_jbd_slab(size_t slab_size)
 {
-	int i = JBD_SLAB_INDEX(slab_size);
+	int i = JBD2_SLAB_INDEX(slab_size);
 
-	BUG_ON(i >= JBD_MAX_SLABS);
+	BUG_ON(i >= JBD2_MAX_SLABS);
 
 	/*
 	 * Check if we already have a slab created for this size
@@ -2028,7 +2028,7 @@ void * jbd2_slab_alloc(size_t size, gfp_
 {
 	int idx;
 
-	idx = JBD_SLAB_INDEX(size);
+	idx = JBD2_SLAB_INDEX(size);
 	BUG_ON(jbd_slab[idx] == NULL);
 	return kmem_cache_alloc(jbd_slab[idx], flags | __GFP_NOFAIL);
 }
@@ -2037,7 +2037,7 @@ void jbd2_slab_free(void *ptr,  size_t s
 {
 	int idx;
 
-	idx = JBD_SLAB_INDEX(size);
+	idx = JBD2_SLAB_INDEX(size);
 	BUG_ON(jbd_slab[idx] == NULL);
 	kmem_cache_free(jbd_slab[idx], ptr);
 }
@@ -2107,7 +2107,7 @@ static void journal_free_journal_head(st
 {
 #ifdef CONFIG_JBD2_DEBUG
 	atomic_dec(&nr_journal_heads);
-	memset(jh, JBD_POISON_FREE, sizeof(*jh));
+	memset(jh, JBD2_POISON_FREE, sizeof(*jh));
 #endif
 	kmem_cache_free(jbd2_journal_head_cache, jh);
 }
Index: linux-2.6.22/fs/jbd2/recovery.c
===================================================================
--- linux-2.6.22.orig/fs/jbd2/recovery.c	2007-07-13 17:21:58.000000000 -0700
+++ linux-2.6.22/fs/jbd2/recovery.c	2007-07-13 17:22:08.000000000 -0700
@@ -312,7 +312,7 @@ int jbd2_journal_skip_recovery(journal_t
 static inline unsigned long long read_tag_block(int tag_bytes, journal_block_tag_t *tag)
 {
 	unsigned long long block = be32_to_cpu(tag->t_blocknr);
-	if (tag_bytes > JBD_TAG_SIZE32)
+	if (tag_bytes > JBD2_TAG_SIZE32)
 		block |= (u64)be32_to_cpu(tag->t_blocknr_high) << 32;
 	return block;
 }
Index: linux-2.6.22/fs/jbd2/revoke.c
===================================================================
--- linux-2.6.22.orig/fs/jbd2/revoke.c	2007-07-13 17:22:26.000000000 -0700
+++ linux-2.6.22/fs/jbd2/revoke.c	2007-07-13 17:22:42.000000000 -0700
@@ -351,7 +351,7 @@ int jbd2_journal_revoke(handle_t *handle
 		if (bh)
 			BUFFER_TRACE(bh, "found on hash");
 	}
-#ifdef JBD_EXPENSIVE_CHECKING
+#ifdef JBD2_EXPENSIVE_CHECKING
 	else {
 		struct buffer_head *bh2;
 
@@ -452,7 +452,7 @@ int jbd2_journal_cancel_revoke(handle_t 
 		}
 	}
 
-#ifdef JBD_EXPENSIVE_CHECKING
+#ifdef JBD2_EXPENSIVE_CHECKING
 	/* There better not be one left behind by now! */
 	record = find_revoke_record(journal, bh->b_blocknr);
 	J_ASSERT_JH(jh, record == NULL);
Index: linux-2.6.22/fs/ext4/extents.c
===================================================================
--- linux-2.6.22.orig/fs/ext4/extents.c	2007-07-13 17:59:00.000000000 -0700
+++ linux-2.6.22/fs/ext4/extents.c	2007-07-13 17:59:44.000000000 -0700
@@ -33,7 +33,7 @@
 #include <linux/fs.h>
 #include <linux/time.h>
 #include <linux/ext4_jbd2.h>
-#include <linux/jbd.h>
+#include <linux/jbd2.h>
 #include <linux/highuid.h>
 #include <linux/pagemap.h>
 #include <linux/quotaops.h>

ext4 patch queue updated

[Mingming Cao]

I have updated ext4 patch queue.  The updated ext4 patch queue includes
updated patches from Kalpak, Jose, and me.  It now addresses most of
review comments from Andrew, including checkpatch.pl complains, add
"diffstat"  etc. Also I noticed Amit has updated fallocate() patch
series as well.

The patches that still need more attentions are:

ext4-journal_chksum-2.6.20.patch
jbd-stats-through-procfs_fix.patch
and inode version patch series


Thanks,
Mingming Cao

[EXT4 set 3][PATCH 1/1] ext4 nanosecond timestamp

[Mingming Cao]

This patch is a spinoff of the old nanosecond patches.

It includes some cleanups and addition of a creation timestamp. The
EXT3_FEATURE_RO_COMPAT_EXTRA_ISIZE flag has also been added along with
s_{min, want}_extra_isize fields in struct ext3_super_block.

Signed-off-by: Andreas Dilger <adilger@clusterfs.com>
Signed-off-by: Kalpak Shah <kalpak@clusterfs.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
Signed-off-by: Mingming Cao <cmm@us.ibm.com>

Index: linux-2.6.22-rc4/fs/ext4/ialloc.c
===================================================================
--- linux-2.6.22-rc4.orig/fs/ext4/ialloc.c	2007-06-11 17:22:15.000000000 -0700
+++ linux-2.6.22-rc4/fs/ext4/ialloc.c	2007-06-11 17:39:05.000000000 -0700
@@ -563,7 +563,8 @@
 	inode->i_ino = ino;
 	/* This is the optimal IO size (for stat), not the fs block size */
 	inode->i_blocks = 0;
-	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
+	inode->i_mtime = inode->i_atime = inode->i_ctime = ei->i_crtime =
+						       ext4_current_time(inode);
 
 	memset(ei->i_data, 0, sizeof(ei->i_data));
 	ei->i_dir_start_lookup = 0;
@@ -595,9 +596,8 @@
 	spin_unlock(&sbi->s_next_gen_lock);
 
 	ei->i_state = EXT4_STATE_NEW;
-	ei->i_extra_isize =
-		(EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) ?
-		sizeof(struct ext4_inode) - EXT4_GOOD_OLD_INODE_SIZE : 0;
+
+	ei->i_extra_isize = EXT4_SB(sb)->s_want_extra_isize;
 
 	ret = inode;
 	if(DQUOT_ALLOC_INODE(inode)) {
Index: linux-2.6.22-rc4/fs/ext4/inode.c
===================================================================
--- linux-2.6.22-rc4.orig/fs/ext4/inode.c	2007-06-11 17:24:28.000000000 -0700
+++ linux-2.6.22-rc4/fs/ext4/inode.c	2007-06-11 17:39:05.000000000 -0700
@@ -726,7 +726,7 @@
 
 	/* We are done with atomic stuff, now do the rest of housekeeping */
 
-	inode->i_ctime = CURRENT_TIME_SEC;
+	inode->i_ctime = ext4_current_time(inode);
 	ext4_mark_inode_dirty(handle, inode);
 
 	/* had we spliced it onto indirect block? */
@@ -2375,7 +2375,7 @@
 	ext4_discard_reservation(inode);
 
 	mutex_unlock(&ei->truncate_mutex);
-	inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
+	inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
 	ext4_mark_inode_dirty(handle, inode);
 
 	/*
@@ -2629,10 +2629,6 @@
 	}
 	inode->i_nlink = le16_to_cpu(raw_inode->i_links_count);
 	inode->i_size = le32_to_cpu(raw_inode->i_size);
-	inode->i_atime.tv_sec = (signed)le32_to_cpu(raw_inode->i_atime);
-	inode->i_ctime.tv_sec = (signed)le32_to_cpu(raw_inode->i_ctime);
-	inode->i_mtime.tv_sec = (signed)le32_to_cpu(raw_inode->i_mtime);
-	inode->i_atime.tv_nsec = inode->i_ctime.tv_nsec = inode->i_mtime.tv_nsec = 0;
 
 	ei->i_state = 0;
 	ei->i_dir_start_lookup = 0;
@@ -2708,6 +2704,11 @@
 	} else
 		ei->i_extra_isize = 0;
 
+	EXT4_INODE_GET_XTIME(i_ctime, inode, raw_inode);
+	EXT4_INODE_GET_XTIME(i_mtime, inode, raw_inode);
+	EXT4_INODE_GET_XTIME(i_atime, inode, raw_inode);
+	EXT4_EINODE_GET_XTIME(i_crtime, ei, raw_inode);
+
 	if (S_ISREG(inode->i_mode)) {
 		inode->i_op = &ext4_file_inode_operations;
 		inode->i_fop = &ext4_file_operations;
@@ -2789,9 +2790,12 @@
 	}
 	raw_inode->i_links_count = cpu_to_le16(inode->i_nlink);
 	raw_inode->i_size = cpu_to_le32(ei->i_disksize);
-	raw_inode->i_atime = cpu_to_le32(inode->i_atime.tv_sec);
-	raw_inode->i_ctime = cpu_to_le32(inode->i_ctime.tv_sec);
-	raw_inode->i_mtime = cpu_to_le32(inode->i_mtime.tv_sec);
+
+	EXT4_INODE_SET_XTIME(i_ctime, inode, raw_inode);
+	EXT4_INODE_SET_XTIME(i_mtime, inode, raw_inode);
+	EXT4_INODE_SET_XTIME(i_atime, inode, raw_inode);
+	EXT4_EINODE_SET_XTIME(i_crtime, ei, raw_inode);
+
 	raw_inode->i_blocks = cpu_to_le32(inode->i_blocks);
 	raw_inode->i_dtime = cpu_to_le32(ei->i_dtime);
 	raw_inode->i_flags = cpu_to_le32(ei->i_flags);
Index: linux-2.6.22-rc4/fs/ext4/ioctl.c
===================================================================
--- linux-2.6.22-rc4.orig/fs/ext4/ioctl.c	2007-06-11 17:25:11.000000000 -0700
+++ linux-2.6.22-rc4/fs/ext4/ioctl.c	2007-06-11 17:39:05.000000000 -0700
@@ -97,7 +97,7 @@
 		ei->i_flags = flags;
 
 		ext4_set_inode_flags(inode);
-		inode->i_ctime = CURRENT_TIME_SEC;
+		inode->i_ctime = ext4_current_time(inode);
 
 		err = ext4_mark_iloc_dirty(handle, inode, &iloc);
 flags_err:
@@ -134,7 +134,7 @@
 			return PTR_ERR(handle);
 		err = ext4_reserve_inode_write(handle, inode, &iloc);
 		if (err == 0) {
-			inode->i_ctime = CURRENT_TIME_SEC;
+			inode->i_ctime = ext4_current_time(inode);
 			inode->i_generation = generation;
 			err = ext4_mark_iloc_dirty(handle, inode, &iloc);
 		}
Index: linux-2.6.22-rc4/fs/ext4/namei.c
===================================================================
--- linux-2.6.22-rc4.orig/fs/ext4/namei.c	2007-06-11 17:22:15.000000000 -0700
+++ linux-2.6.22-rc4/fs/ext4/namei.c	2007-06-11 17:39:05.000000000 -0700
@@ -1285,7 +1285,7 @@
 	 * happen is that the times are slightly out of date
 	 * and/or different from the directory change time.
 	 */
-	dir->i_mtime = dir->i_ctime = CURRENT_TIME_SEC;
+	dir->i_mtime = dir->i_ctime = ext4_current_time(dir);
 	ext4_update_dx_flag(dir);
 	dir->i_version++;
 	ext4_mark_inode_dirty(handle, dir);
@@ -2046,7 +2046,7 @@
 	 * recovery. */
 	inode->i_size = 0;
 	ext4_orphan_add(handle, inode);
-	inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME_SEC;
+	inode->i_ctime = dir->i_ctime = dir->i_mtime = ext4_current_time(inode);
 	ext4_mark_inode_dirty(handle, inode);
 	drop_nlink(dir);
 	ext4_update_dx_flag(dir);
@@ -2096,13 +2096,13 @@
 	retval = ext4_delete_entry(handle, dir, de, bh);
 	if (retval)
 		goto end_unlink;
-	dir->i_ctime = dir->i_mtime = CURRENT_TIME_SEC;
+	dir->i_ctime = dir->i_mtime = ext4_current_time(dir);
 	ext4_update_dx_flag(dir);
 	ext4_mark_inode_dirty(handle, dir);
 	drop_nlink(inode);
 	if (!inode->i_nlink)
 		ext4_orphan_add(handle, inode);
-	inode->i_ctime = dir->i_ctime;
+	inode->i_ctime = ext4_current_time(inode);
 	ext4_mark_inode_dirty(handle, inode);
 	retval = 0;
 
@@ -2193,7 +2193,7 @@
 	if (IS_DIRSYNC(dir))
 		handle->h_sync = 1;
 
-	inode->i_ctime = CURRENT_TIME_SEC;
+	inode->i_ctime = ext4_current_time(inode);
 	inc_nlink(inode);
 	atomic_inc(&inode->i_count);
 
@@ -2295,7 +2295,7 @@
 	 * Like most other Unix systems, set the ctime for inodes on a
 	 * rename.
 	 */
-	old_inode->i_ctime = CURRENT_TIME_SEC;
+	old_inode->i_ctime = ext4_current_time(old_inode);
 	ext4_mark_inode_dirty(handle, old_inode);
 
 	/*
@@ -2328,9 +2328,9 @@
 
 	if (new_inode) {
 		drop_nlink(new_inode);
-		new_inode->i_ctime = CURRENT_TIME_SEC;
+		new_inode->i_ctime = ext4_current_time(new_inode);
 	}
-	old_dir->i_ctime = old_dir->i_mtime = CURRENT_TIME_SEC;
+	old_dir->i_ctime = old_dir->i_mtime = ext4_current_time(old_dir);
 	ext4_update_dx_flag(old_dir);
 	if (dir_bh) {
 		BUFFER_TRACE(dir_bh, "get_write_access");
Index: linux-2.6.22-rc4/fs/ext4/super.c
===================================================================
--- linux-2.6.22-rc4.orig/fs/ext4/super.c	2007-06-11 17:28:09.000000000 -0700
+++ linux-2.6.22-rc4/fs/ext4/super.c	2007-06-11 17:39:05.000000000 -0700
@@ -1642,6 +1642,8 @@
 				sbi->s_inode_size);
 			goto failed_mount;
 		}
+		if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE)
+			sb->s_time_gran = 1 << (EXT4_EPOCH_BITS - 2);
 	}
 	sbi->s_frag_size = EXT4_MIN_FRAG_SIZE <<
 				   le32_to_cpu(es->s_log_frag_size);
@@ -1865,6 +1867,32 @@
 	}
 
 	ext4_setup_super (sb, es, sb->s_flags & MS_RDONLY);
+
+	/* determine the minimum size of new large inodes, if present */
+	if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE) {
+		sbi->s_want_extra_isize = sizeof(struct ext4_inode) -
+						     EXT4_GOOD_OLD_INODE_SIZE;
+		if (EXT4_HAS_RO_COMPAT_FEATURE(sb,
+				       EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE)) {
+			if (sbi->s_want_extra_isize <
+			    le16_to_cpu(es->s_want_extra_isize))
+				sbi->s_want_extra_isize =
+					le16_to_cpu(es->s_want_extra_isize);
+			if (sbi->s_want_extra_isize <
+			    le16_to_cpu(es->s_min_extra_isize))
+				sbi->s_want_extra_isize =
+					le16_to_cpu(es->s_min_extra_isize);
+		}
+	}
+	/* Check if enough inode space is available */
+	if (EXT4_GOOD_OLD_INODE_SIZE + sbi->s_want_extra_isize >
+							sbi->s_inode_size) {
+		sbi->s_want_extra_isize = sizeof(struct ext4_inode) -
+						       EXT4_GOOD_OLD_INODE_SIZE;
+		printk(KERN_INFO "EXT4-fs: required extra inode space not"
+			"available.\n");
+	}
+
 	/*
 	 * akpm: core read_super() calls in here with the superblock locked.
 	 * That deadlocks, because orphan cleanup needs to lock the superblock
Index: linux-2.6.22-rc4/fs/ext4/xattr.c
===================================================================
--- linux-2.6.22-rc4.orig/fs/ext4/xattr.c	2007-06-11 17:22:15.000000000 -0700
+++ linux-2.6.22-rc4/fs/ext4/xattr.c	2007-06-11 17:39:05.000000000 -0700
@@ -1013,7 +1013,7 @@
 	}
 	if (!error) {
 		ext4_xattr_update_super_block(handle, inode->i_sb);
-		inode->i_ctime = CURRENT_TIME_SEC;
+		inode->i_ctime = ext4_current_time(inode);
 		error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
 		/*
 		 * The bh is consumed by ext4_mark_iloc_dirty, even with
Index: linux-2.6.22-rc4/include/linux/ext4_fs.h
===================================================================
--- linux-2.6.22-rc4.orig/include/linux/ext4_fs.h	2007-06-11 17:36:13.000000000 -0700
+++ linux-2.6.22-rc4/include/linux/ext4_fs.h	2007-06-11 17:41:55.000000000 -0700
@@ -288,7 +288,7 @@
 	__le16	i_uid;		/* Low 16 bits of Owner Uid */
 	__le32	i_size;		/* Size in bytes */
 	__le32	i_atime;	/* Access time */
-	__le32	i_ctime;	/* Creation time */
+	__le32	i_ctime;	/* Inode Change time */
 	__le32	i_mtime;	/* Modification time */
 	__le32	i_dtime;	/* Deletion Time */
 	__le16	i_gid;		/* Low 16 bits of Group Id */
@@ -337,10 +337,74 @@
 	} osd2;				/* OS dependent 2 */
 	__le16	i_extra_isize;
 	__le16	i_pad1;
+	__le32  i_ctime_extra;  /* extra Change time      (nsec << 2 | epoch) */
+	__le32  i_mtime_extra;  /* extra Modification time(nsec << 2 | epoch) */
+	__le32  i_atime_extra;  /* extra Access time      (nsec << 2 | epoch) */
+	__le32  i_crtime;       /* File Creation time */
+	__le32  i_crtime_extra; /* extra FileCreationtime (nsec << 2 | epoch) */
 };
 
 #define i_size_high	i_dir_acl
 
+#define EXT4_EPOCH_BITS 2
+#define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1)
+#define EXT4_NSEC_MASK  (~0UL << EXT4_EPOCH_BITS)
+
+#define EXT4_FITS_IN_INODE(ext4_inode, einode, field)	\
+	((offsetof(typeof(*ext4_inode), field) +	\
+	  sizeof((ext4_inode)->field))			\
+	<= (EXT4_GOOD_OLD_INODE_SIZE +			\
+	    (einode)->i_extra_isize))			\
+
+static inline __le32 ext4_encode_extra_time(struct timespec *time)
+{
+       return cpu_to_le32((sizeof(time->tv_sec) > 4 ?
+			   time->tv_sec >> 32 : 0) |
+			   ((time->tv_nsec << 2) & EXT4_NSEC_MASK));
+}
+
+static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
+{
+       if (sizeof(time->tv_sec) > 4)
+	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
+			       << 32;
+       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> 2;
+}
+
+#define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)			       \
+do {									       \
+	(raw_inode)->xtime = cpu_to_le32((inode)->xtime.tv_sec);	       \
+	if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra))     \
+		(raw_inode)->xtime ## _extra =				       \
+				ext4_encode_extra_time(&(inode)->xtime);       \
+} while (0)
+
+#define EXT4_EINODE_SET_XTIME(xtime, einode, raw_inode)			       \
+do {									       \
+	if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime))		       \
+		(raw_inode)->xtime = cpu_to_le32((einode)->xtime.tv_sec);      \
+	if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra))	       \
+		(raw_inode)->xtime ## _extra =				       \
+				ext4_encode_extra_time(&(einode)->xtime);      \
+} while (0)
+
+#define EXT4_INODE_GET_XTIME(xtime, inode, raw_inode)			       \
+do {									       \
+	(inode)->xtime.tv_sec = le32_to_cpu((raw_inode)->xtime);	       \
+	if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra))     \
+		ext4_decode_extra_time(&(inode)->xtime,			       \
+				       raw_inode->xtime ## _extra);	       \
+} while (0)
+
+#define EXT4_EINODE_GET_XTIME(xtime, einode, raw_inode)			       \
+do {									       \
+	if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime))		       \
+		(einode)->xtime.tv_sec = le32_to_cpu((raw_inode)->xtime);      \
+	if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra))	       \
+		ext4_decode_extra_time(&(einode)->xtime,		       \
+				       raw_inode->xtime ## _extra);	       \
+} while (0)
+
 #if defined(__KERNEL__) || defined(__linux__)
 #define i_reserved1	osd1.linux1.l_i_reserved1
 #define i_frag		osd2.linux2.l_i_frag
@@ -539,6 +603,13 @@
 	return container_of(inode, struct ext4_inode_info, vfs_inode);
 }
 
+static inline struct timespec ext4_current_time(struct inode *inode)
+{
+	return (inode->i_sb->s_time_gran < NSEC_PER_SEC) ?
+		current_fs_time(inode->i_sb) : CURRENT_TIME_SEC;
+}
+
+
 static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino)
 {
 	return ino == EXT4_ROOT_INO ||
@@ -609,6 +680,7 @@
 #define EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER	0x0001
 #define EXT4_FEATURE_RO_COMPAT_LARGE_FILE	0x0002
 #define EXT4_FEATURE_RO_COMPAT_BTREE_DIR	0x0004
+#define EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE	0x0040
 
 #define EXT4_FEATURE_INCOMPAT_COMPRESSION	0x0001
 #define EXT4_FEATURE_INCOMPAT_FILETYPE		0x0002
@@ -626,6 +698,7 @@
 					 EXT4_FEATURE_INCOMPAT_64BIT)
 #define EXT4_FEATURE_RO_COMPAT_SUPP	(EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER| \
 					 EXT4_FEATURE_RO_COMPAT_LARGE_FILE| \
+					 EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE | \
 					 EXT4_FEATURE_RO_COMPAT_BTREE_DIR)
 
 /*
Index: linux-2.6.22-rc4/include/linux/ext4_fs_i.h
===================================================================
--- linux-2.6.22-rc4.orig/include/linux/ext4_fs_i.h	2007-06-11 17:22:15.000000000 -0700
+++ linux-2.6.22-rc4/include/linux/ext4_fs_i.h	2007-06-11 17:39:05.000000000 -0700
@@ -153,6 +153,7 @@
 
 	unsigned long i_ext_generation;
 	struct ext4_ext_cache i_cached_extent;
+	struct timespec i_crtime;
 };
 
 #endif	/* _LINUX_EXT4_FS_I */
Index: linux-2.6.22-rc4/include/linux/ext4_fs_sb.h
===================================================================
--- linux-2.6.22-rc4.orig/include/linux/ext4_fs_sb.h	2007-06-11 17:28:15.000000000 -0700
+++ linux-2.6.22-rc4/include/linux/ext4_fs_sb.h	2007-06-11 17:39:05.000000000 -0700
@@ -79,6 +79,7 @@
 	char *s_qf_names[MAXQUOTAS];		/* Names of quota files with journalled quota */
 	int s_jquota_fmt;			/* Format of quota to use */
 #endif
+	unsigned int s_want_extra_isize; /* New inodes should reserve # bytes */
 
 #ifdef EXTENTS_STATS
 	/* ext4 extents stats */

Re: [EXT4 set 3][PATCH 1/1] ext4 nanosecond timestamp

[Andrew Morton]

On Sun, 01 Jul 2007 03:36:56 -0400
Mingming Cao <cmm@us.ibm.com> wrote:

> This patch is a spinoff of the old nanosecond patches.

I don't know what the "old nanosecond patches" are.  A link to a suitable
changlog for those patches would do in a pinch.  Preferable would be to
write a proper changelog for this patch.

> It includes some cleanups and addition of a creation timestamp. The
> EXT3_FEATURE_RO_COMPAT_EXTRA_ISIZE flag has also been added along with
> s_{min, want}_extra_isize fields in struct ext3_super_block.
> 
> Signed-off-by: Andreas Dilger <adilger@clusterfs.com>
> Signed-off-by: Kalpak Shah <kalpak@clusterfs.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> Signed-off-by: Mingming Cao <cmm@us.ibm.com>
> 
> Index: linux-2.6.22-rc4/fs/ext4/ialloc.c

Please include diffstat output when preparing patches.

> +
> +#define EXT4_FITS_IN_INODE(ext4_inode, einode, field)	\
> +	((offsetof(typeof(*ext4_inode), field) +	\
> +	  sizeof((ext4_inode)->field))			\
> +	<= (EXT4_GOOD_OLD_INODE_SIZE +			\
> +	    (einode)->i_extra_isize))			\

Please add explanatory commentary to EXT4_FITS_IN_INODE(): tell readers
under what circumstances something will not fit in an inode and what the
consequences of this are.

> +static inline __le32 ext4_encode_extra_time(struct timespec *time)
> +{
> +       return cpu_to_le32((sizeof(time->tv_sec) > 4 ?
> +			   time->tv_sec >> 32 : 0) |
> +			   ((time->tv_nsec << 2) & EXT4_NSEC_MASK));
> +}
> +
> +static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
> +{
> +       if (sizeof(time->tv_sec) > 4)
> +	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
> +			       << 32;
> +       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> 2;
> +}

Consider uninlining these functions.

> +#define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)			       \
> +do {									       \
> +	(raw_inode)->xtime = cpu_to_le32((inode)->xtime.tv_sec);	       \
> +	if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra))     \
> +		(raw_inode)->xtime ## _extra =				       \
> +				ext4_encode_extra_time(&(inode)->xtime);       \
> +} while (0)
> +
> +#define EXT4_EINODE_SET_XTIME(xtime, einode, raw_inode)			       \
> +do {									       \
> +	if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime))		       \
> +		(raw_inode)->xtime = cpu_to_le32((einode)->xtime.tv_sec);      \
> +	if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra))	       \
> +		(raw_inode)->xtime ## _extra =				       \
> +				ext4_encode_extra_time(&(einode)->xtime);      \
> +} while (0)
> +
> +#define EXT4_INODE_GET_XTIME(xtime, inode, raw_inode)			       \
> +do {									       \
> +	(inode)->xtime.tv_sec = le32_to_cpu((raw_inode)->xtime);	       \
> +	if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra))     \
> +		ext4_decode_extra_time(&(inode)->xtime,			       \
> +				       raw_inode->xtime ## _extra);	       \
> +} while (0)
> +
> +#define EXT4_EINODE_GET_XTIME(xtime, einode, raw_inode)			       \
> +do {									       \
> +	if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime))		       \
> +		(einode)->xtime.tv_sec = le32_to_cpu((raw_inode)->xtime);      \
> +	if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra))	       \
> +		ext4_decode_extra_time(&(einode)->xtime,		       \
> +				       raw_inode->xtime ## _extra);	       \
> +} while (0)

Ugly.  I expect these could be implemented as plain old C functions. 
Caller could pass in the address of the ext4_inode field which the function
is to operate upon.

>  #if defined(__KERNEL__) || defined(__linux__)

(What's the __linux__ for?)

>  #define i_reserved1	osd1.linux1.l_i_reserved1
>  #define i_frag		osd2.linux2.l_i_frag
> @@ -539,6 +603,13 @@
>  	return container_of(inode, struct ext4_inode_info, vfs_inode);
>  }
>  
> +static inline struct timespec ext4_current_time(struct inode *inode)
> +{
> +	return (inode->i_sb->s_time_gran < NSEC_PER_SEC) ?
> +		current_fs_time(inode->i_sb) : CURRENT_TIME_SEC;
> +}

Now, I've forgotten how this works.  Remind me, please.  Can ext4
filesystems ever have one-second timestamp granularity?  If so, how does
one cause that to come about?

> --- linux-2.6.22-rc4.orig/include/linux/ext4_fs_i.h	2007-06-11 17:22:15.000000000 -0700
> +++ linux-2.6.22-rc4/include/linux/ext4_fs_i.h	2007-06-11 17:39:05.000000000 -0700
> @@ -153,6 +153,7 @@
>  
>  	unsigned long i_ext_generation;
>  	struct ext4_ext_cache i_cached_extent;
> +	struct timespec i_crtime;
>  };

It is unobvious what this field does.  Please prefer to add commentary to
_all_ struct fields - it really helps.

I thought checkpatch was going to have a little whine about that but the
version I have here doesn't.

>  
>  #endif	/* _LINUX_EXT4_FS_I */
> Index: linux-2.6.22-rc4/include/linux/ext4_fs_sb.h
> ===================================================================
> --- linux-2.6.22-rc4.orig/include/linux/ext4_fs_sb.h	2007-06-11 17:28:15.000000000 -0700
> +++ linux-2.6.22-rc4/include/linux/ext4_fs_sb.h	2007-06-11 17:39:05.000000000 -0700
> @@ -79,6 +79,7 @@
>  	char *s_qf_names[MAXQUOTAS];		/* Names of quota files with journalled quota */
>  	int s_jquota_fmt;			/* Format of quota to use */
>  #endif
> +	unsigned int s_want_extra_isize; /* New inodes should reserve # bytes */
>  
>  #ifdef EXTENTS_STATS

OK, I can kind-of see how this is working, but some overall description of
how the inode sizing design operates would be helpful.  It would certainly
make reviewing of this proposed change more fruitful.  Perhaps that new
comment over EXT4_FITS_IN_INODE() would be a suitable place.

Re: [EXT4 set 3][PATCH 1/1] ext4 nanosecond timestamp

[Mingming Cao]

On Tue, 2007-07-10 at 16:30 -0700, Andrew Morton wrote:
> On Sun, 01 Jul 2007 03:36:56 -0400
> Mingming Cao <cmm@us.ibm.com> wrote:
> 
> > This patch is a spinoff of the old nanosecond patches.
> 
> I don't know what the "old nanosecond patches" are.  A link to a suitable
> changlog for those patches would do in a pinch.  Preferable would be to
> write a proper changelog for this patch.
> 
> > It includes some cleanups and addition of a creation timestamp. The
> > EXT3_FEATURE_RO_COMPAT_EXTRA_ISIZE flag has also been added along with
> > s_{min, want}_extra_isize fields in struct ext3_super_block.
> > 
> > Signed-off-by: Andreas Dilger <adilger@clusterfs.com>
> > Signed-off-by: Kalpak Shah <kalpak@clusterfs.com>
> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> > Signed-off-by: Mingming Cao <cmm@us.ibm.com>
> > 
> > Index: linux-2.6.22-rc4/fs/ext4/ialloc.c
> 
> Please include diffstat output when preparing patches.
> 
> > +
> > +#define EXT4_FITS_IN_INODE(ext4_inode, einode, field)	\
> > +	((offsetof(typeof(*ext4_inode), field) +	\
> > +	  sizeof((ext4_inode)->field))			\
> > +	<= (EXT4_GOOD_OLD_INODE_SIZE +			\
> > +	    (einode)->i_extra_isize))			\
> 
> Please add explanatory commentary to EXT4_FITS_IN_INODE(): tell readers
> under what circumstances something will not fit in an inode and what the
> consequences of this are.
> 
> > +static inline __le32 ext4_encode_extra_time(struct timespec *time)
> > +{
> > +       return cpu_to_le32((sizeof(time->tv_sec) > 4 ?
> > +			   time->tv_sec >> 32 : 0) |
> > +			   ((time->tv_nsec << 2) & EXT4_NSEC_MASK));
> > +}
> > +
> > +static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
> > +{
> > +       if (sizeof(time->tv_sec) > 4)
> > +	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
> > +			       << 32;
> > +       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> 2;
> > +}
> 
> Consider uninlining these functions.
> 
I got compile warining after apply Kalpal's update nanosecond patch,
which makes these two function inline. It complains these functions are
defined but not used. It's being used only in the following
micros(EXT4_INODE_SET_XTIME etc).  So if the .c file included the
ext4_fs.h but not using the micros, the compile will think these two
functions are not used.

Mingming

> > +#define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)			       \
> > +do {									       \
> > +	(raw_inode)->xtime = cpu_to_le32((inode)->xtime.tv_sec);	       \
> > +	if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra))     \
> > +		(raw_inode)->xtime ## _extra =				       \
> > +				ext4_encode_extra_time(&(inode)->xtime);       \
> > +} while (0)
> > +
> > +#define EXT4_EINODE_SET_XTIME(xtime, einode, raw_inode)			       \
> > +do {									       \
> > +	if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime))		       \
> > +		(raw_inode)->xtime = cpu_to_le32((einode)->xtime.tv_sec);      \
> > +	if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra))	       \
> > +		(raw_inode)->xtime ## _extra =				       \
> > +				ext4_encode_extra_time(&(einode)->xtime);      \
> > +} while (0)
> > +
> > +#define EXT4_INODE_GET_XTIME(xtime, inode, raw_inode)			       \
> > +do {									       \
> > +	(inode)->xtime.tv_sec = le32_to_cpu((raw_inode)->xtime);	       \
> > +	if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra))     \
> > +		ext4_decode_extra_time(&(inode)->xtime,			       \
> > +				       raw_inode->xtime ## _extra);	       \
> > +} while (0)
> > +
> > +#define EXT4_EINODE_GET_XTIME(xtime, einode, raw_inode)			       \
> > +do {									       \
> > +	if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime))		       \
> > +		(einode)->xtime.tv_sec = le32_to_cpu((raw_inode)->xtime);      \
> > +	if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra))	       \
> > +		ext4_decode_extra_time(&(einode)->xtime,		       \
> > +				       raw_inode->xtime ## _extra);	       \
> > +} while (0)
> 
> Ugly.  I expect these could be implemented as plain old C functions. 
> Caller could pass in the address of the ext4_inode field which the function
> is to operate upon.
> 
> >  #if defined(__KERNEL__) || defined(__linux__)
> 
> (What's the __linux__ for?)
> 
> >  #define i_reserved1	osd1.linux1.l_i_reserved1
> >  #define i_frag		osd2.linux2.l_i_frag
> > @@ -539,6 +603,13 @@
> >  	return container_of(inode, struct ext4_inode_info, vfs_inode);
> >  }
> >  
> > +static inline struct timespec ext4_current_time(struct inode *inode)
> > +{
> > +	return (inode->i_sb->s_time_gran < NSEC_PER_SEC) ?
> > +		current_fs_time(inode->i_sb) : CURRENT_TIME_SEC;
> > +}
> 
> Now, I've forgotten how this works.  Remind me, please.  Can ext4
> filesystems ever have one-second timestamp granularity?  If so, how does
> one cause that to come about?
> 
> > --- linux-2.6.22-rc4.orig/include/linux/ext4_fs_i.h	2007-06-11 17:22:15.000000000 -0700
> > +++ linux-2.6.22-rc4/include/linux/ext4_fs_i.h	2007-06-11 17:39:05.000000000 -0700
> > @@ -153,6 +153,7 @@
> >  
> >  	unsigned long i_ext_generation;
> >  	struct ext4_ext_cache i_cached_extent;
> > +	struct timespec i_crtime;
> >  };
> 
> It is unobvious what this field does.  Please prefer to add commentary to
> _all_ struct fields - it really helps.
> 
> I thought checkpatch was going to have a little whine about that but the
> version I have here doesn't.
> 
> >  
> >  #endif	/* _LINUX_EXT4_FS_I */
> > Index: linux-2.6.22-rc4/include/linux/ext4_fs_sb.h
> > ===================================================================
> > --- linux-2.6.22-rc4.orig/include/linux/ext4_fs_sb.h	2007-06-11 17:28:15.000000000 -0700
> > +++ linux-2.6.22-rc4/include/linux/ext4_fs_sb.h	2007-06-11 17:39:05.000000000 -0700
> > @@ -79,6 +79,7 @@
> >  	char *s_qf_names[MAXQUOTAS];		/* Names of quota files with journalled quota */
> >  	int s_jquota_fmt;			/* Format of quota to use */
> >  #endif
> > +	unsigned int s_want_extra_isize; /* New inodes should reserve # bytes */
> >  
> >  #ifdef EXTENTS_STATS
> 
> OK, I can kind-of see how this is working, but some overall description of
> how the inode sizing design operates would be helpful.  It would certainly
> make reviewing of this proposed change more fruitful.  Perhaps that new
> comment over EXT4_FITS_IN_INODE() would be a suitable place.

Re: [EXT4 set 3][PATCH 1/1] ext4 nanosecond timestamp

[Kalpak Shah]

On Mon, 2007-07-16 at 17:49 -0700, Mingming Cao wrote:
> On Tue, 2007-07-10 at 16:30 -0700, Andrew Morton wrote:
> > On Sun, 01 Jul 2007 03:36:56 -0400
> > Mingming Cao <cmm@us.ibm.com> wrote:
> > > +static inline __le32 ext4_encode_extra_time(struct timespec *time)
> > > +{
> > > +       return cpu_to_le32((sizeof(time->tv_sec) > 4 ?
> > > +			   time->tv_sec >> 32 : 0) |
> > > +			   ((time->tv_nsec << 2) & EXT4_NSEC_MASK));
> > > +}
> > > +
> > > +static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
> > > +{
> > > +       if (sizeof(time->tv_sec) > 4)
> > > +	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
> > > +			       << 32;
> > > +       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> 2;
> > > +}
> > 
> > Consider uninlining these functions.
> > 
> I got compile warining after apply Kalpal's update nanosecond patch,
> which makes these two function inline. It complains these functions are
> defined but not used. It's being used only in the following
> micros(EXT4_INODE_SET_XTIME etc).  So if the .c file included the
> ext4_fs.h but not using the micros, the compile will think these two
> functions are not used.

The compile warnings were introduced because the functions were
uninlined. So we can either keep these functions inlined or consider
adding a "__used" attribute to these two functions. 

Thanks,
Kalpak.

Re: [EXT4 set 3][PATCH 1/1] ext4 nanosecond timestamp

[Mingming Cao]

On Tue, 2007-07-17 at 15:29 +0530, Kalpak Shah wrote:
> On Mon, 2007-07-16 at 17:49 -0700, Mingming Cao wrote:
> > On Tue, 2007-07-10 at 16:30 -0700, Andrew Morton wrote:
> > > On Sun, 01 Jul 2007 03:36:56 -0400
> > > Mingming Cao <cmm@us.ibm.com> wrote:
> > > > +static inline __le32 ext4_encode_extra_time(struct timespec *time)
> > > > +{
> > > > +       return cpu_to_le32((sizeof(time->tv_sec) > 4 ?
> > > > +			   time->tv_sec >> 32 : 0) |
> > > > +			   ((time->tv_nsec << 2) & EXT4_NSEC_MASK));
> > > > +}
> > > > +
> > > > +static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
> > > > +{
> > > > +       if (sizeof(time->tv_sec) > 4)
> > > > +	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
> > > > +			       << 32;
> > > > +       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> 2;
> > > > +}
> > > 
> > > Consider uninlining these functions.
> > > 
> > I got compile warining after apply Kalpal's update nanosecond patch,
> > which makes these two function inline. It complains these functions are
> > defined but not used. It's being used only in the following
> > micros(EXT4_INODE_SET_XTIME etc).  So if the .c file included the
> > ext4_fs.h but not using the micros, the compile will think these two
> > functions are not used.
> 
> The compile warnings were introduced because the functions were
> uninlined. So we can either keep these functions inlined or consider
> adding a "__used" attribute to these two functions. 
> 
okay for now I keep these functions inlined. 

> Thanks,
> Kalpak.
> 

ext4 patch queue updated

[Mingming Cao]

Hi Ted,

The ext4 patch queue has been updated with updates to address latest
review comments.  http://repo.or.cz/w/ext4-patch-queue.git

The series is being reordered: the patches still has outstanding
comments are moved to the end of the series. Per our discussion on
Monday, tThose patches are ready for submission are

# Rebased the patches to 2.6.22

# fallocate() syscall patches and ext4 fallocate() implementation
# Missing manpages
#ext4-fallocate-1-man-page
ext4-fallocate-2-syscall_i386_amd64_ppc
ext4-fallocate-3-ext4_support
ext4-fallocate-4-uninit_write_support
ext4-fallocate-5-new-ondisk-format

# Add mount option to turn off extents
ext4_noextent_mount_opt.patch

# Mounted ext4dev fs with extents by default for testing purpose,
# for Ext4 product release, extents mount option
# will be turn on only if the fs has EXTENTS feature on
ext4_extents_on_by_default.patch

# Propagate inode flags
ext4-propagate_flags.patch

# Add extent sanity checks
ext4-extent-sanity-checks.patch

# Bug fix:set 64bit JBD2 feature on >32bit ext4 fs
ext4_set_jbd2_64bit_feature.patch

# Fix: Rename CONFIG_JBD_DEBUG to CONFIG_JBD2_DEBUG
jbd2_config_jbd2_debug_fix.patch

# Export jbd2-debug via debugfs
jbd2_move_jbd2_debug_to_debugfs.patch

# Nanosecond timestamp support
ext4-nanosecond-patch
ext4_negative_timestamp_handle_fix.patch

# New patch to expand inode i_extra_isize to support features
# in high part of inode (>128 bytes)
ext4_expand_inode_extra_isize.patch

# Remove 32000 subdirs limit. 
ext4_remove_subdirs_limit.patch

# Various Cleanups
ext4-zero_user_page.patch
is_power_of_2-ext4-superc.patch
ext4-remove-extra-is_rdonly-check.patch
ext4_extent_compilation_fixes.patch
ext4_extent_macros_cleanup.patch

#JBD->JBD2 naming cleanups

ext4 patch queue updated

[Mingming Cao]

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]

Just updated the ext4 patch queue

http://repo.or.cz/w/ext4-patch-queue

Changes:

Added these three patches from Jose Santos:
ext4_set_jbd2_64bit_feature.patch
jbd2_config_jbd2_debug_fix.patch
jbd2_move_jbd2_debug_to_debugfs.patch

Reordered the series, move 
jbd-stats-through-procfs and ext4_remove_subdirs_limit.patch before
delayed allocation patches


Run checkpatch.pl http://lwn.net/Articles/237451/
I fixed coding style issues for most patches by hand except the delayed
allocation patch ext4-delayed-allocation.patch.  That need a bit more
work and requires someone family with the code better. Alex, can you
help on this? Attached is the checkpatch.pl output, thanks.


Mingming



[-- Attachment #2: a.out --]
[-- Type: text/plain, Size: 2741 bytes --]

printk() should include KERN_ facility level
#275: FILE: fs/ext4/writeback.c:73:
+#define wb_debug(fmt, a...)	printk(fmt, ##a);

do not use assignment in condition
#338: FILE: fs/ext4/writeback.c:136:
+		while (!bio && (nr_vecs /= 2))

printk() should include KERN_ facility level
#826: FILE: fs/ext4/writeback.c:624:
+			printk("no mem for ext4_wb_pages!\n");

#if 0 -- if this code redundant remove it
#946: FILE: fs/ext4/writeback.c:744:
+#if 0

line over 80 characters
#948: FILE: fs/ext4/writeback.c:746:
+					printk("#%u: wow! short extent %d for flush on #%lu\n",

printk() should include KERN_ facility level
#948: FILE: fs/ext4/writeback.c:746:
+					printk("#%u: wow! short extent %d for flush on #%lu\n",

line over 80 characters
#949: FILE: fs/ext4/writeback.c:747:
+						(unsigned) current->pid, wc.len, inode->i_ino);

line over 80 characters
#950: FILE: fs/ext4/writeback.c:748:
+					printk("#%u: done = %d, nr_to_write %ld, sync = %d\n",

printk() should include KERN_ facility level
#950: FILE: fs/ext4/writeback.c:748:
+					printk("#%u: done = %d, nr_to_write %ld, sync = %d\n",

line over 80 characters
#951: FILE: fs/ext4/writeback.c:749:
+						(unsigned) current->pid, done, wbc->nr_to_write,

printk() should include KERN_ facility level
#953: FILE: fs/ext4/writeback.c:751:
+					printk("#%u: written %d, extents %d\n",

line over 80 characters
#954: FILE: fs/ext4/writeback.c:752:
+						(unsigned) current->pid, written, extents);

printk() should include KERN_ facility level
#955: FILE: fs/ext4/writeback.c:753:
+					printk("#%u: cur %lu, prev %lu\n",

#if 0 -- if this code redundant remove it
#985: FILE: fs/ext4/writeback.c:783:
+#if 0

line over 80 characters
#991: FILE: fs/ext4/writeback.c:789:
+				atomic_inc(&EXT4_SB(inode->i_sb)->s_wb_congested);

printk() should include KERN_ facility level
#1370: FILE: fs/ext4/writeback.c:1168:
+	printk("EXT4-fs: writeback: %d blocks %d extents in %d reqs (%d ave)\n",

line over 80 characters
#1375: FILE: fs/ext4/writeback.c:1173:
+	printk("EXT4-fs: writeback: %d nr_to_write, %d congestions, %d singles\n",

printk() should include KERN_ facility level
#1375: FILE: fs/ext4/writeback.c:1173:
+	printk("EXT4-fs: writeback: %d nr_to_write, %d congestions, %d singles\n",

printk() should include KERN_ facility level
#1379: FILE: fs/ext4/writeback.c:1177:
+	printk("EXT4-fs: writeback: %d collisions, %d single-page collisions\n",

printk() should include KERN_ facility level
#1382: FILE: fs/ext4/writeback.c:1180:
+	printk("EXT4-fs: writeback: %d allocated, %d dropped\n",

Missing Signed-off-by: line(s)
Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Re: ext4 patch queue updated

[Alex Tomas]

OK

Mingming Cao wrote:
> Run checkpatch.pl http://lwn.net/Articles/237451/
> I fixed coding style issues for most patches by hand except the delayed
> allocation patch ext4-delayed-allocation.patch.  That need a bit more
> work and requires someone family with the code better. Alex, can you
> help on this? Attached is the checkpatch.pl output, thanks.