Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
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. - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
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 - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
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. - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
On Sun, 01 Jul 2007 03:37:45 -0400 Mingming Cao [EMAIL PROTECTED] 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.c2007-06-13 17:16:28.0 -0700 +++ linux-2.6.22-rc4/fs/ext4/ialloc.c 2007-06-13 17:24:45.0 -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.0 -0700 +++ linux-2.6.22-rc4/fs/ext4/inode.c 2007-06-13 17:24:45.0 -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.0 -0700 +++ linux-2.6.22-rc4/fs/ext4/super.c 2007-06-13 17:24:45.0 -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. - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
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.0 -0700 +++ linux-2.6.22-rc4/fs/ext4/super.c2007-06-13 17:24:45.0 -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 - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
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 [EMAIL PROTECTED] 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 [EMAIL PROTECTED] Signed-off-by: Aneesh Kumar K.V [EMAIL PROTECTED] Signed-off-by: Mingming Cao [EMAIL PROTECTED] --- 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 - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
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 [EMAIL PROTECTED] 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 [EMAIL PROTECTED] Signed-off-by: Aneesh Kumar K.V [EMAIL PROTECTED] Signed-off-by: Mingming Cao [EMAIL PROTECTED] --- 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 - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[EXT4 set 4][PATCH 4/5] i_version:ext4 inode version update
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 [EMAIL PROTECTED] Signed-off-by: Mingming Cao [EMAIL PROTECTED] 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.0 -0700 +++ linux-2.6.22-rc4/fs/ext4/ialloc.c 2007-06-13 17:24:45.0 -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.0 -0700 +++ linux-2.6.22-rc4/fs/ext4/inode.c2007-06-13 17:24:45.0 -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.0 -0700 +++ linux-2.6.22-rc4/fs/ext4/super.c2007-06-13 17:24:45.0 -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; - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html