LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
@ 2007-07-01  7:37 Mingming Cao
  2007-07-03  6:49 ` Aneesh Kumar K.V
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Mingming Cao @ 2007-07-01  7:37 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel, linux-ext4, nfsv4

This patch is on top of i_version_update_vfs.
The i_version field of the inode is set on inode creation and incremented when
the inode is being modified.

Signed-off-by: Jean Noel Cordenner <jean-noel.cordenner@bull.net>
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-13 17:16:28.000000000 -0700
+++ linux-2.6.22-rc4/fs/ext4/ialloc.c	2007-06-13 17:24:45.000000000 -0700
@@ -565,6 +565,7 @@ got:
 	inode->i_blocks = 0;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = ei->i_crtime =
 						       ext4_current_time(inode);
+	inode->i_version = 1;
 
 	memset(ei->i_data, 0, sizeof(ei->i_data));
 	ei->i_dir_start_lookup = 0;
Index: linux-2.6.22-rc4/fs/ext4/inode.c
===================================================================
--- linux-2.6.22-rc4.orig/fs/ext4/inode.c	2007-06-13 17:21:29.000000000 -0700
+++ linux-2.6.22-rc4/fs/ext4/inode.c	2007-06-13 17:24:45.000000000 -0700
@@ -3082,6 +3082,7 @@ int ext4_mark_iloc_dirty(handle_t *handl
 {
 	int err = 0;
 
+	inode->i_version++;
 	/* the do_update_inode consumes one bh->b_count */
 	get_bh(iloc->bh);
 
Index: linux-2.6.22-rc4/fs/ext4/super.c
===================================================================
--- linux-2.6.22-rc4.orig/fs/ext4/super.c	2007-06-13 17:19:11.000000000 -0700
+++ linux-2.6.22-rc4/fs/ext4/super.c	2007-06-13 17:24:45.000000000 -0700
@@ -2846,8 +2846,8 @@ out:
 		i_size_write(inode, off+len-towrite);
 		EXT4_I(inode)->i_disksize = inode->i_size;
 	}
-	inode->i_version++;
 	inode->i_mtime = inode->i_ctime = CURRENT_TIME;
+	inode->i_version = 1;
 	ext4_mark_inode_dirty(handle, inode);
 	mutex_unlock(&inode->i_mutex);
 	return len - towrite;



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
  2007-07-01  7:37 [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update Mingming Cao
@ 2007-07-03  6:49 ` Aneesh Kumar K.V
  2007-07-03 21:41   ` Andreas Dilger
  2007-07-10 23:31 ` Andrew Morton
  2007-07-11  8:47 ` Christoph Hellwig
  2 siblings, 1 reply; 8+ messages in thread
From: Aneesh Kumar K.V @ 2007-07-03  6:49 UTC (permalink / raw)
  To: cmm; +Cc: linux-fsdevel, linux-kernel, linux-ext4, nfsv4



Mingming Cao wrote:
> 
> Index: linux-2.6.22-rc4/fs/ext4/super.c
> ===================================================================
> --- linux-2.6.22-rc4.orig/fs/ext4/super.c	2007-06-13 17:19:11.000000000 -0700
> +++ linux-2.6.22-rc4/fs/ext4/super.c	2007-06-13 17:24:45.000000000 -0700
> @@ -2846,8 +2846,8 @@ out:
>  		i_size_write(inode, off+len-towrite);
>  		EXT4_I(inode)->i_disksize = inode->i_size;
>  	}
> -	inode->i_version++;
>  	inode->i_mtime = inode->i_ctime = CURRENT_TIME;
> +	inode->i_version = 1;
>  	ext4_mark_inode_dirty(handle, inode);
>  	mutex_unlock(&inode->i_mutex);
>  	return len - towrite;


Is this correct ? . Why do we set the qutoa file inodes version to 1  during write ?


- aneesh

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
  2007-07-03  6:49 ` Aneesh Kumar K.V
@ 2007-07-03 21:41   ` Andreas Dilger
  0 siblings, 0 replies; 8+ messages in thread
From: Andreas Dilger @ 2007-07-03 21:41 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: cmm, linux-fsdevel, linux-kernel, linux-ext4, nfsv4

On Jul 03, 2007  12:19 +0530, Aneesh Kumar K.V wrote:
> Mingming Cao wrote:
> >Index: linux-2.6.22-rc4/fs/ext4/super.c
> >===================================================================
> >--- linux-2.6.22-rc4.orig/fs/ext4/super.c	2007-06-13 
> >17:19:11.000000000 -0700
> >+++ linux-2.6.22-rc4/fs/ext4/super.c	2007-06-13 17:24:45.000000000 -0700
> >@@ -2846,8 +2846,8 @@ out:
> > 		i_size_write(inode, off+len-towrite);
> > 		EXT4_I(inode)->i_disksize = inode->i_size;
> > 	}
> >-	inode->i_version++;
> > 	inode->i_mtime = inode->i_ctime = CURRENT_TIME;
> >+	inode->i_version = 1;
> > 	ext4_mark_inode_dirty(handle, inode);
> > 	mutex_unlock(&inode->i_mutex);
> > 	return len - towrite;
> 
> 
> Is this correct ? . Why do we set the qutoa file inodes version to 1  
> during write ?

Hmm, I thought we had previously fixed this?

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
  2007-07-01  7:37 [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update Mingming Cao
  2007-07-03  6:49 ` Aneesh Kumar K.V
@ 2007-07-10 23:31 ` Andrew Morton
  2007-07-11  8:47 ` Christoph Hellwig
  2 siblings, 0 replies; 8+ messages in thread
From: Andrew Morton @ 2007-07-10 23:31 UTC (permalink / raw)
  To: cmm; +Cc: linux-fsdevel, linux-kernel, linux-ext4, nfsv4

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

> This patch is on top of i_version_update_vfs.
> The i_version field of the inode is set on inode creation and incremented when
> the inode is being modified.
> 

Again, I don't think I've ever seen this patch before.  It is at least a
month old.

> 
> Index: linux-2.6.22-rc4/fs/ext4/ialloc.c
> ===================================================================
> --- linux-2.6.22-rc4.orig/fs/ext4/ialloc.c	2007-06-13 17:16:28.000000000 -0700
> +++ linux-2.6.22-rc4/fs/ext4/ialloc.c	2007-06-13 17:24:45.000000000 -0700
> @@ -565,6 +565,7 @@ got:
>  	inode->i_blocks = 0;
>  	inode->i_mtime = inode->i_atime = inode->i_ctime = ei->i_crtime =
>  						       ext4_current_time(inode);
> +	inode->i_version = 1;
>  
>  	memset(ei->i_data, 0, sizeof(ei->i_data));
>  	ei->i_dir_start_lookup = 0;
> Index: linux-2.6.22-rc4/fs/ext4/inode.c
> ===================================================================
> --- linux-2.6.22-rc4.orig/fs/ext4/inode.c	2007-06-13 17:21:29.000000000 -0700
> +++ linux-2.6.22-rc4/fs/ext4/inode.c	2007-06-13 17:24:45.000000000 -0700
> @@ -3082,6 +3082,7 @@ int ext4_mark_iloc_dirty(handle_t *handl
>  {
>  	int err = 0;
>  
> +	inode->i_version++;
>  	/* the do_update_inode consumes one bh->b_count */
>  	get_bh(iloc->bh);
>  
> Index: linux-2.6.22-rc4/fs/ext4/super.c
> ===================================================================
> --- linux-2.6.22-rc4.orig/fs/ext4/super.c	2007-06-13 17:19:11.000000000 -0700
> +++ linux-2.6.22-rc4/fs/ext4/super.c	2007-06-13 17:24:45.000000000 -0700
> @@ -2846,8 +2846,8 @@ out:
>  		i_size_write(inode, off+len-towrite);
>  		EXT4_I(inode)->i_disksize = inode->i_size;
>  	}
> -	inode->i_version++;
>  	inode->i_mtime = inode->i_ctime = CURRENT_TIME;
> +	inode->i_version = 1;
>  	ext4_mark_inode_dirty(handle, inode);
>  	mutex_unlock(&inode->i_mutex);
>  	return len - towrite;

ext4 already has code to update i_version on directories.  Here we appear
to be udpating it on regular files?

But for what reason?  The changelog doesn't say?

AFAICT the code forgets to update i_version during file overwrites (unless
the overwrite was over a hole).  But without a decent description of this
change I cannot tell whether this was a bug.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
  2007-07-01  7:37 [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update Mingming Cao
  2007-07-03  6:49 ` Aneesh Kumar K.V
  2007-07-10 23:31 ` Andrew Morton
@ 2007-07-11  8:47 ` Christoph Hellwig
  2007-07-11 11:52   ` Andreas Dilger
  2007-07-11 11:54   ` Trond Myklebust
  2 siblings, 2 replies; 8+ messages in thread
From: Christoph Hellwig @ 2007-07-11  8:47 UTC (permalink / raw)
  To: Mingming Cao; +Cc: linux-fsdevel, linux-kernel, linux-ext4, nfsv4

On Sun, Jul 01, 2007 at 03:37:45AM -0400, Mingming Cao wrote:
> This patch is on top of i_version_update_vfs.
> The i_version field of the inode is set on inode creation and incremented when
> the inode is being modified.

Which is not what i_version is supposed to do.  It'll get you tons of misses
for NFSv3 filehandles that rely on the generation staying the same for the
same file.  Please add a new field for the NFSv4 sequence counter instead
of making i_version unuseable.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
  2007-07-11  8:47 ` Christoph Hellwig
@ 2007-07-11 11:52   ` Andreas Dilger
  2007-07-11 12:02     ` Christoph Hellwig
  2007-07-11 11:54   ` Trond Myklebust
  1 sibling, 1 reply; 8+ messages in thread
From: Andreas Dilger @ 2007-07-11 11:52 UTC (permalink / raw)
  To: Christoph Hellwig, Mingming Cao, linux-fsdevel, linux-kernel,
	linux-ext4, nfsv4

On Jul 11, 2007  09:47 +0100, Christoph Hellwig wrote:
> On Sun, Jul 01, 2007 at 03:37:45AM -0400, Mingming Cao wrote:
> > This patch is on top of i_version_update_vfs.
> > The i_version field of the inode is set on inode creation and incremented
> > when the inode is being modified.
> 
> Which is not what i_version is supposed to do.  It'll get you tons of misses
> for NFSv3 filehandles that rely on the generation staying the same for the
> same file.  Please add a new field for the NFSv4 sequence counter instead
> of making i_version unuseable.

You are confusing i_generation (the instance of this inode number) with
i_version (whether this file has been modified)?

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
  2007-07-11  8:47 ` Christoph Hellwig
  2007-07-11 11:52   ` Andreas Dilger
@ 2007-07-11 11:54   ` Trond Myklebust
  1 sibling, 0 replies; 8+ messages in thread
From: Trond Myklebust @ 2007-07-11 11:54 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Mingming Cao, linux-fsdevel, linux-kernel, linux-ext4, nfsv4

On Wed, 2007-07-11 at 09:47 +0100, Christoph Hellwig wrote:
> On Sun, Jul 01, 2007 at 03:37:45AM -0400, Mingming Cao wrote:
> > This patch is on top of i_version_update_vfs.
> > The i_version field of the inode is set on inode creation and incremented when
> > the inode is being modified.
> 
> Which is not what i_version is supposed to do.  It'll get you tons of misses
> for NFSv3 filehandles that rely on the generation staying the same for the
> same file.  Please add a new field for the NFSv4 sequence counter instead
> of making i_version unuseable.

Aren't you confusing i_version and i_generation here? Those are two
separate inode fields.

Cheers
  Trond


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
  2007-07-11 11:52   ` Andreas Dilger
@ 2007-07-11 12:02     ` Christoph Hellwig
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2007-07-11 12:02 UTC (permalink / raw)
  To: Christoph Hellwig, Mingming Cao, linux-fsdevel, linux-kernel,
	linux-ext4, nfsv4

On Wed, Jul 11, 2007 at 05:52:24AM -0600, Andreas Dilger wrote:
> On Jul 11, 2007  09:47 +0100, Christoph Hellwig wrote:
> > On Sun, Jul 01, 2007 at 03:37:45AM -0400, Mingming Cao wrote:
> > > This patch is on top of i_version_update_vfs.
> > > The i_version field of the inode is set on inode creation and incremented
> > > when the inode is being modified.
> > 
> > Which is not what i_version is supposed to do.  It'll get you tons of misses
> > for NFSv3 filehandles that rely on the generation staying the same for the
> > same file.  Please add a new field for the NFSv4 sequence counter instead
> > of making i_version unuseable.
> 
> You are confusing i_generation (the instance of this inode number) with
> i_version (whether this file has been modified)?

Yes, sorry.  Objection dropped.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2007-07-11 12:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-07-01  7:37 [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update Mingming Cao
2007-07-03  6:49 ` Aneesh Kumar K.V
2007-07-03 21:41   ` Andreas Dilger
2007-07-10 23:31 ` Andrew Morton
2007-07-11  8:47 ` Christoph Hellwig
2007-07-11 11:52   ` Andreas Dilger
2007-07-11 12:02     ` Christoph Hellwig
2007-07-11 11:54   ` Trond Myklebust

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).