linux-ext4.vger.kernel.org archive mirror
 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
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ 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] 12+ 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-03  6:55 ` Aneesh Kumar K.V
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ 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] 12+ 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  6:55 ` Aneesh Kumar K.V
  2007-07-04  5:26   ` Aneesh Kumar K.V
  2007-07-04  5:26   ` Aneesh Kumar K.V
  2007-07-10 23:31 ` Andrew Morton
  2007-07-11  8:47 ` Christoph Hellwig
  3 siblings, 2 replies; 12+ messages in thread
From: Aneesh Kumar K.V @ 2007-07-03  6:55 UTC (permalink / raw)
  To: cmm; +Cc: linux-ext4



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.
> ta));
>  	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);
> 

If we bump i_version  in ext4_mark_iloc_dity then we should be removing the i_version update
at other places. A simple grep of ext4 dir shows i_version being updated followed by 
ext4_mark_inode_dirty.


-aneesh

^ permalink raw reply	[flat|nested] 12+ 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; 12+ 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] 12+ messages in thread

* Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
  2007-07-04  5:26   ` Aneesh Kumar K.V
@ 2007-07-04  3:31     ` Mingming Cao
  0 siblings, 0 replies; 12+ messages in thread
From: Mingming Cao @ 2007-07-04  3:31 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: linux-ext4

On Wed, 2007-07-04 at 10:56 +0530, Aneesh Kumar K.V wrote:
> 
> Aneesh Kumar K.V wrote:
> 
> > 
> > If we bump i_version  in ext4_mark_iloc_dity then we should be removing 
> > the i_version update
> > at other places. A simple grep of ext4 dir shows i_version being updated 
> > followed by ext4_mark_inode_dirty.
> > 
> > 
> 
> How about the below patch. I also removed the i_version = 1 from 
> ext4_new_inode because we do the same in ext4_alloc_inode. I also added
> the From:  line. 
> 

Acked.

Ha, I actually just finished on a incremental patch to cleanup redundant
i_version update.:) But I missed the ext4_new_inode().

Thanks. I will replace this patch in the ext4 patch queue


> -aneesh
> 
> From ccdfc4a6a8edbdbf5d21d32fd2350f36399a89f8 Mon Sep 17 00:00:00 2001
> From: Jean Noel Cordenner <jean-noel.cordenner@bull.net>
> Date: Wed, 4 Jul 2007 10:51:50 +0530
> Subject: [PATCH] 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: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Signed-off-by: Mingming Cao <cmm@us.ibm.com>
> ---
>  fs/ext4/inode.c |    1 +
>  fs/ext4/namei.c |    4 ----
>  fs/ext4/super.c |    1 -
>  3 files changed, 1 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index cb9f1f4..cd3e886 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3084,6 +3084,7 @@ int ext4_mark_iloc_dirty(handle_t *handle,
>  {
>  	int err = 0;
> 
> +	inode->i_version++;
>  	/* the do_update_inode consumes one bh->b_count */
>  	get_bh(iloc->bh);
> 
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 477b6d2..a1dc5ef 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1287,7 +1287,6 @@ static int add_dirent_to_buf(handle_t *handle, struct dentry *dentry,
>  	 */
>  	dir->i_mtime = dir->i_ctime = ext4_current_time(dir);
>  	ext4_update_dx_flag(dir);
> -	dir->i_version++;
>  	ext4_mark_inode_dirty(handle, dir);
>  	BUFFER_TRACE(bh, "call ext4_journal_dirty_metadata");
>  	err = ext4_journal_dirty_metadata(handle, bh);
> @@ -1606,7 +1605,6 @@ static int ext4_delete_entry (handle_t *handle,
>  						    le16_to_cpu(de->rec_len));
>  			else
>  				de->inode = 0;
> -			dir->i_version++;
>  			BUFFER_TRACE(bh, "call ext4_journal_dirty_metadata");
>  			ext4_journal_dirty_metadata(handle, bh);
>  			return 0;
> @@ -2039,7 +2037,6 @@ static int ext4_rmdir (struct inode * dir, struct dentry *dentry)
>  		ext4_warning (inode->i_sb, "ext4_rmdir",
>  			      "empty directory has nlink!=2 (%d)",
>  			      inode->i_nlink);
> -	inode->i_version++;
>  	clear_nlink(inode);
>  	/* There's no need to set i_disksize: the fact that i_nlink is
>  	 * zero will ensure that the right thing happens during any
> @@ -2284,7 +2281,6 @@ static int ext4_rename (struct inode * old_dir, struct dentry *old_dentry,
>  		if (EXT4_HAS_INCOMPAT_FEATURE(new_dir->i_sb,
>  					      EXT4_FEATURE_INCOMPAT_FILETYPE))
>  			new_de->file_type = old_de->file_type;
> -		new_dir->i_version++;
>  		BUFFER_TRACE(new_bh, "call ext4_journal_dirty_metadata");
>  		ext4_journal_dirty_metadata(handle, new_bh);
>  		brelse(new_bh);
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 914c273..55b99b3 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2846,7 +2846,6 @@ 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;
>  	ext4_mark_inode_dirty(handle, inode);
>  	mutex_unlock(&inode->i_mutex);

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

* Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
  2007-07-03  6:55 ` Aneesh Kumar K.V
@ 2007-07-04  5:26   ` Aneesh Kumar K.V
  2007-07-04  3:31     ` Mingming Cao
  2007-07-04  5:26   ` Aneesh Kumar K.V
  1 sibling, 1 reply; 12+ messages in thread
From: Aneesh Kumar K.V @ 2007-07-04  5:26 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: cmm, linux-ext4



Aneesh Kumar K.V wrote:

> 
> If we bump i_version  in ext4_mark_iloc_dity then we should be removing 
> the i_version update
> at other places. A simple grep of ext4 dir shows i_version being updated 
> followed by ext4_mark_inode_dirty.
> 
> 

How about the below patch. I also removed the i_version = 1 from 
ext4_new_inode because we do the same in ext4_alloc_inode. I also added
the From:  line. 

-aneesh

>From ccdfc4a6a8edbdbf5d21d32fd2350f36399a89f8 Mon Sep 17 00:00:00 2001
From: Jean Noel Cordenner <jean-noel.cordenner@bull.net>
Date: Wed, 4 Jul 2007 10:51:50 +0530
Subject: [PATCH] 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: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Mingming Cao <cmm@us.ibm.com>
---
 fs/ext4/inode.c |    1 +
 fs/ext4/namei.c |    4 ----
 fs/ext4/super.c |    1 -
 3 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index cb9f1f4..cd3e886 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3084,6 +3084,7 @@ int ext4_mark_iloc_dirty(handle_t *handle,
 {
 	int err = 0;
 
+	inode->i_version++;
 	/* the do_update_inode consumes one bh->b_count */
 	get_bh(iloc->bh);
 
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 477b6d2..a1dc5ef 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1287,7 +1287,6 @@ static int add_dirent_to_buf(handle_t *handle, struct dentry *dentry,
 	 */
 	dir->i_mtime = dir->i_ctime = ext4_current_time(dir);
 	ext4_update_dx_flag(dir);
-	dir->i_version++;
 	ext4_mark_inode_dirty(handle, dir);
 	BUFFER_TRACE(bh, "call ext4_journal_dirty_metadata");
 	err = ext4_journal_dirty_metadata(handle, bh);
@@ -1606,7 +1605,6 @@ static int ext4_delete_entry (handle_t *handle,
 						    le16_to_cpu(de->rec_len));
 			else
 				de->inode = 0;
-			dir->i_version++;
 			BUFFER_TRACE(bh, "call ext4_journal_dirty_metadata");
 			ext4_journal_dirty_metadata(handle, bh);
 			return 0;
@@ -2039,7 +2037,6 @@ static int ext4_rmdir (struct inode * dir, struct dentry *dentry)
 		ext4_warning (inode->i_sb, "ext4_rmdir",
 			      "empty directory has nlink!=2 (%d)",
 			      inode->i_nlink);
-	inode->i_version++;
 	clear_nlink(inode);
 	/* There's no need to set i_disksize: the fact that i_nlink is
 	 * zero will ensure that the right thing happens during any
@@ -2284,7 +2281,6 @@ static int ext4_rename (struct inode * old_dir, struct dentry *old_dentry,
 		if (EXT4_HAS_INCOMPAT_FEATURE(new_dir->i_sb,
 					      EXT4_FEATURE_INCOMPAT_FILETYPE))
 			new_de->file_type = old_de->file_type;
-		new_dir->i_version++;
 		BUFFER_TRACE(new_bh, "call ext4_journal_dirty_metadata");
 		ext4_journal_dirty_metadata(handle, new_bh);
 		brelse(new_bh);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 914c273..55b99b3 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2846,7 +2846,6 @@ 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;
 	ext4_mark_inode_dirty(handle, inode);
 	mutex_unlock(&inode->i_mutex);
-- 
1.5.3.rc0-dirty

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

* Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
  2007-07-03  6:55 ` Aneesh Kumar K.V
  2007-07-04  5:26   ` Aneesh Kumar K.V
@ 2007-07-04  5:26   ` Aneesh Kumar K.V
  1 sibling, 0 replies; 12+ messages in thread
From: Aneesh Kumar K.V @ 2007-07-04  5:26 UTC (permalink / raw)
  To: cmm; +Cc: Aneesh Kumar K.V, linux-ext4



Aneesh Kumar K.V wrote:

> 
> If we bump i_version  in ext4_mark_iloc_dity then we should be removing 
> the i_version update
> at other places. A simple grep of ext4 dir shows i_version being updated 
> followed by ext4_mark_inode_dirty.
> 
> 

How about the below patch. I also removed the i_version = 1 from 
ext4_new_inode because we do the same in ext4_alloc_inode. I also added
the From:  line. 

-aneesh

>From ccdfc4a6a8edbdbf5d21d32fd2350f36399a89f8 Mon Sep 17 00:00:00 2001
From: Jean Noel Cordenner <jean-noel.cordenner@bull.net>
Date: Wed, 4 Jul 2007 10:51:50 +0530
Subject: [PATCH] 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: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Mingming Cao <cmm@us.ibm.com>
---
 fs/ext4/inode.c |    1 +
 fs/ext4/namei.c |    4 ----
 fs/ext4/super.c |    1 -
 3 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index cb9f1f4..cd3e886 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3084,6 +3084,7 @@ int ext4_mark_iloc_dirty(handle_t *handle,
 {
 	int err = 0;
 
+	inode->i_version++;
 	/* the do_update_inode consumes one bh->b_count */
 	get_bh(iloc->bh);
 
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 477b6d2..a1dc5ef 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1287,7 +1287,6 @@ static int add_dirent_to_buf(handle_t *handle, struct dentry *dentry,
 	 */
 	dir->i_mtime = dir->i_ctime = ext4_current_time(dir);
 	ext4_update_dx_flag(dir);
-	dir->i_version++;
 	ext4_mark_inode_dirty(handle, dir);
 	BUFFER_TRACE(bh, "call ext4_journal_dirty_metadata");
 	err = ext4_journal_dirty_metadata(handle, bh);
@@ -1606,7 +1605,6 @@ static int ext4_delete_entry (handle_t *handle,
 						    le16_to_cpu(de->rec_len));
 			else
 				de->inode = 0;
-			dir->i_version++;
 			BUFFER_TRACE(bh, "call ext4_journal_dirty_metadata");
 			ext4_journal_dirty_metadata(handle, bh);
 			return 0;
@@ -2039,7 +2037,6 @@ static int ext4_rmdir (struct inode * dir, struct dentry *dentry)
 		ext4_warning (inode->i_sb, "ext4_rmdir",
 			      "empty directory has nlink!=2 (%d)",
 			      inode->i_nlink);
-	inode->i_version++;
 	clear_nlink(inode);
 	/* There's no need to set i_disksize: the fact that i_nlink is
 	 * zero will ensure that the right thing happens during any
@@ -2284,7 +2281,6 @@ static int ext4_rename (struct inode * old_dir, struct dentry *old_dentry,
 		if (EXT4_HAS_INCOMPAT_FEATURE(new_dir->i_sb,
 					      EXT4_FEATURE_INCOMPAT_FILETYPE))
 			new_de->file_type = old_de->file_type;
-		new_dir->i_version++;
 		BUFFER_TRACE(new_bh, "call ext4_journal_dirty_metadata");
 		ext4_journal_dirty_metadata(handle, new_bh);
 		brelse(new_bh);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 914c273..55b99b3 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2846,7 +2846,6 @@ 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;
 	ext4_mark_inode_dirty(handle, inode);
 	mutex_unlock(&inode->i_mutex);
-- 
1.5.3.rc0-dirty

^ permalink raw reply related	[flat|nested] 12+ 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  6:55 ` Aneesh Kumar K.V
@ 2007-07-10 23:31 ` Andrew Morton
  2007-07-11  8:47 ` Christoph Hellwig
  3 siblings, 0 replies; 12+ 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] 12+ 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
                   ` (2 preceding siblings ...)
  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
  3 siblings, 2 replies; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ messages in thread
From: Trond Myklebust @ 2007-07-11 11:54 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, nfsv4, linux-ext4, Mingming Cao, linux-kernel

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] 12+ 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; 12+ 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] 12+ messages in thread

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

Thread overview: 12+ 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-03  6:55 ` Aneesh Kumar K.V
2007-07-04  5:26   ` Aneesh Kumar K.V
2007-07-04  3:31     ` Mingming Cao
2007-07-04  5:26   ` 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 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).