Re: [EXT4 set 4][PATCH 3/5] i_version:ext4 inode version read/store

2007-07-11 Thread Andreas Dilger 
On Jul 10, 2007  16:31 -0700, Andrew Morton wrote:
  This patch adds 64-bit inode version support to ext4. The lower 32 bits
  are stored in the osd1.linux1.l_i_version field while the high 32 bits
  are stored in the i_version_hi field newly created in the ext4_inode.
 
 So reading the code here does serve to answer the question I raised against
 the earlier patch.  A bit.
 
 I'd have thought that this patch and the one which adds i_version_hi should
 be folded into a single diff?

It could be - the original patch to reserve i_version_hi was submitted to
before the patches were ready to avoid that space being used by something
else.

  +   if (EXT4_INODE_SIZE(inode-i_sb)  EXT4_GOOD_OLD_INODE_SIZE) {
  +   if (EXT4_FITS_IN_INODE(raw_inode, ei, i_version_hi))
  +   inode-i_version |=
  +   (__u64)(le32_to_cpu(raw_inode-i_version_hi))  32;
 
 checks the precedence of (type) versus 
 
 OK
 
  +   }
 
 I don't quite see how the above two tests are sufficient to unambiguously
 determine that the i_version_hi field is present on-disk.
 
 I guess we're implicitly assuming that if the on-disk inode is big enough
 then it _must_ have i_version_hi in there?  If so, why is the comparison
 with EXT4_GOOD_OLD_INODE_SIZE needed?

The GOOT_OLD_INODE_SIZE check is needed to know if i_extra_isize is even
present or valid in the on-disk inode.

  @@ -2852,8 +2859,14 @@ static int ext4_do_update_inode(handle_t
  +   raw_inode-i_disk_version = cpu_to_le32(inode-i_version);
  +   if (ei-i_extra_isize) {
  +   if (EXT4_FITS_IN_INODE(raw_inode, ei, i_version_hi)) {
 
 There's no comparison with EXT4_GOOD_OLD_INODE_SIZE here...

Because this is the in-memory version and it is always valid (set to zero
if there is extra space in the on-disk inode).

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 3/5] i_version:ext4 inode version read/store

2007-07-10 Thread Andrew Morton 
On Sun, 01 Jul 2007 03:37:36 -0400
Mingming Cao [EMAIL PROTECTED] wrote:

 This patch adds 64-bit inode version support to ext4. The lower 32 bits
 are stored in the osd1.linux1.l_i_version field while the high 32 bits
 are stored in the i_version_hi field newly created in the ext4_inode.

So reading the code here does serve to answer the question I raised against
the earlier patch.  A bit.

I'd have thought that this patch and the one which adds i_version_hi should
be folded into a single diff?

 
 Index: linux-2.6.21/fs/ext4/inode.c
 ===
 --- linux-2.6.21.orig/fs/ext4/inode.c
 +++ linux-2.6.21/fs/ext4/inode.c
 @@ -2709,6 +2709,13 @@ void ext4_read_inode(struct inode * inod
   EXT4_INODE_GET_XTIME(i_atime, inode, raw_inode);
   EXT4_EINODE_GET_XTIME(i_crtime, ei, raw_inode);
 
 + inode-i_version = le32_to_cpu(raw_inode-i_disk_version);
 + if (EXT4_INODE_SIZE(inode-i_sb)  EXT4_GOOD_OLD_INODE_SIZE) {
 + if (EXT4_FITS_IN_INODE(raw_inode, ei, i_version_hi))
 + inode-i_version |=
 + (__u64)(le32_to_cpu(raw_inode-i_version_hi))  32;

checks the precedence of (type) versus 

OK

 + }

I don't quite see how the above two tests are sufficient to unambiguously
determine that the i_version_hi field is present on-disk.

I guess we're implicitly assuming that if the on-disk inode is big enough
then it _must_ have i_version_hi in there?  If so, why is the comparison
with EXT4_GOOD_OLD_INODE_SIZE needed?

Some description of the overall approach to inode version identification
would be helpful here.

   if (S_ISREG(inode-i_mode)) {
   inode-i_op = ext4_file_inode_operations;
   inode-i_fop = ext4_file_operations;
 @@ -2852,8 +2859,14 @@ static int ext4_do_update_inode(handle_t
   } else for (block = 0; block  EXT4_N_BLOCKS; block++)
   raw_inode-i_block[block] = ei-i_data[block];
 
 - if (ei-i_extra_isize)
 + raw_inode-i_disk_version = cpu_to_le32(inode-i_version);
 + if (ei-i_extra_isize) {
 + if (EXT4_FITS_IN_INODE(raw_inode, ei, i_version_hi)) {

There's no comparison with EXT4_GOOD_OLD_INODE_SIZE here...

 + raw_inode-i_version_hi =
 + cpu_to_le32(inode-i_version  32);
 + }
   raw_inode-i_extra_isize = cpu_to_le16(ei-i_extra_isize);
 + }
 

-
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 3/5] i_version:ext4 inode version read/store

2007-07-01 Thread Mingming Cao 
This patch adds 64-bit inode version support to ext4. The lower 32 bits
are stored in the osd1.linux1.l_i_version field while the high 32 bits
are stored in the i_version_hi field newly created in the ext4_inode.

Signed-off-by: Kalpak Shah [EMAIL PROTECTED]
Signed-off-by: Mingming Cao [EMAIL PROTECTED]

Index: linux-2.6.21/fs/ext4/inode.c
===
--- linux-2.6.21.orig/fs/ext4/inode.c
+++ linux-2.6.21/fs/ext4/inode.c
@@ -2709,6 +2709,13 @@ void ext4_read_inode(struct inode * inod
EXT4_INODE_GET_XTIME(i_atime, inode, raw_inode);
EXT4_EINODE_GET_XTIME(i_crtime, ei, raw_inode);

+   inode-i_version = le32_to_cpu(raw_inode-i_disk_version);
+   if (EXT4_INODE_SIZE(inode-i_sb)  EXT4_GOOD_OLD_INODE_SIZE) {
+   if (EXT4_FITS_IN_INODE(raw_inode, ei, i_version_hi))
+   inode-i_version |=
+   (__u64)(le32_to_cpu(raw_inode-i_version_hi))  32;
+   }
+
if (S_ISREG(inode-i_mode)) {
inode-i_op = ext4_file_inode_operations;
inode-i_fop = ext4_file_operations;
@@ -2852,8 +2859,14 @@ static int ext4_do_update_inode(handle_t
} else for (block = 0; block  EXT4_N_BLOCKS; block++)
raw_inode-i_block[block] = ei-i_data[block];

-   if (ei-i_extra_isize)
+   raw_inode-i_disk_version = cpu_to_le32(inode-i_version);
+   if (ei-i_extra_isize) {
+   if (EXT4_FITS_IN_INODE(raw_inode, ei, i_version_hi)) {
+   raw_inode-i_version_hi =
+   cpu_to_le32(inode-i_version  32);
+   }
raw_inode-i_extra_isize = cpu_to_le16(ei-i_extra_isize);
+   }

BUFFER_TRACE(bh, call ext4_journal_dirty_metadata);
rc = ext4_journal_dirty_metadata(handle, bh);
Index: linux-2.6.21/include/linux/ext4_fs.h
===
--- linux-2.6.21.orig/include/linux/ext4_fs.h
+++ linux-2.6.21/include/linux/ext4_fs.h
@@ -297,7 +297,7 @@ struct ext4_inode {
__le32  i_flags;/* File flags */
union {
struct {
-   __u32  l_i_reserved1;
+   __u32  l_i_version;
} linux1;
struct {
__u32  h_i_translator;
@@ -406,6 +406,8 @@ do {
   \
   raw_inode-xtime ## _extra);\
 } while (0)

+#define i_disk_version osd1.linux1.l_i_version
+
 #if defined(__KERNEL__) || defined(__linux__)
 #define i_reserved1osd1.linux1.l_i_reserved1
 #define i_frag osd2.linux2.l_i_frag


-
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