rfc: [patch] change attribute for ext3

hello,

here is a small patch that adds the "change attribute" for ext3 file-systems;

the change attribute is a simple counter that is reset to zero on inode creation and that is incremented every time the inode data is modified (similarly to the "ctime" time-stamp).

Quoted 8 lines · Alexandre Ratchov

hello,

here is a small patch that adds the "change attribute" for ext3 file-systems;

the change attribute is a simple counter that is reset to zero on inode creation and that is incremented every time the inode data is modified (similarly to the "ctime" time-stamp).

I would really have preferred a full-blown 64-bit counter as per RFC3530, but I suppose we could always combine this change attribute with the high word from ctime in order to make up the NFSv4 change attribute. That should keep us safe until someone develops a ramdisk with < 1 nsecond access time.

Quoted 10 lines · Trond Myklebust

Quoted 8 lines · Alexandre Ratchov

hello,

here is a small patch that adds the "change attribute" for ext3 file-systems;

the change attribute is a simple counter that is reset to zero on inode creation and that is incremented every time the inode data is modified (similarly to the "ctime" time-stamp).

Quoted 6 lines

I would really have preferred a full-blown 64-bit counter as per RFC3530, but I suppose we could always combine this change attribute with the high word from ctime in order to make up the NFSv4 change attribute. That should keep us safe until someone develops a ramdisk with < 1 nsecond access time.

do you mean something like "(ctime.tv_sec << 32) | change_attribute"? this would allow 2^32 inode changes per second.

For ext3 it's hard to find unused bits in the on-disk inode structure, but ext4 inode may become larger in the future, allowing a 64bit counter.

Quoted 18 lines · Alexandre Ratchov

Quoted 10 lines · Trond Myklebust

Quoted 8 lines · Alexandre Ratchov

hello,

here is a small patch that adds the "change attribute" for ext3 file-systems;

the change attribute is a simple counter that is reset to zero on inode creation and that is incremented every time the inode data is modified (similarly to the "ctime" time-stamp).

Quoted 6 lines

I would really have preferred a full-blown 64-bit counter as per RFC3530, but I suppose we could always combine this change attribute with the high word from ctime in order to make up the NFSv4 change attribute. That should keep us safe until someone develops a ramdisk with < 1 nsecond access time.

Quoted 2 lines

do you mean something like "(ctime.tv_sec << 32) | change_attribute"? this would allow 2^32 inode changes per second.

Yes. As I said, that probably ought to suffice for now.

Quoted 2 lines

For ext3 it's hard to find unused bits in the on-disk inode structure, but ext4 inode may become larger in the future, allowing a 64bit counter.

In anticipation of that event, could you please change the field in 'struct stat' and 'struct stat64' to be an 'unsigned long long' instead of the current 'unsigned long'?

All the other fields are internal to the kernel, so a future change of their size should not matter.

Quoted 5 lines · Trond Myklebust

Quoted 3 lines · Alexandre Ratchov

the change attribute is a simple counter that is reset to zero on inode creation and that is incremented every time the inode data is modified (similarly to the "ctime" time-stamp).

Quoted 5 lines

I would really have preferred a full-blown 64-bit counter as per RFC3530, but I suppose we could always combine this change attribute with the high word from ctime in order to make up the NFSv4 change attribute. That should keep us safe until someone develops a ramdisk with < 1 nsecond access time.

do you mean something like "(ctime.tv_sec << 32) | change_attribute"? this would allow 2^32 inode changes per second.

Quoted 6 lines · Alexandre Ratchov

Quoted 5 lines · Trond Myklebust

Quoted 3 lines · Alexandre Ratchov

the change attribute is a simple counter that is reset to zero on inode creation and that is incremented every time the inode data is modified (similarly to the "ctime" time-stamp).

Quoted 8 lines

Quoted 5 lines

I would really have preferred a full-blown 64-bit counter as per RFC3530, but I suppose we could always combine this change attribute with the high word from ctime in order to make up the NFSv4 change attribute. That should keep us safe until someone develops a ramdisk with < 1 nsecond access time.

do you mean something like "(ctime.tv_sec << 32) | change_attribute"? this would allow 2^32 inode changes per second.

It might be preferrable, since we are depending on the ctime here anyways, is to combine this with the nsec-resolution ctime, and kill two birds with one field in the inode.

The implementation would be to update the ctime+nsec field as normal, but in the unlikely case that both the second+nsec ctime is the same as before the nsec value would be incremented by 1. This could happen in case of low-resolution kernel timers, and would also handle the future case where the inode is modified more than once in the same nanosecond.

The other benefit is that it allows comparisons between two different inodes to be more meaningful, instead of just using the seconds + random version number.

It would be possible/desirable to make the nsec ctime field be part of the small inode (using the proposed reserved field) instead of the large inode, since that is a requirement for working with existing ext3 filesystems. The previous nsec timestamp patch would only need trivial modifications to make this work, just #define i_ctime_extra to be l_i_reserved1 I believe.

Quoted 5 lines · Trond Myklebust

I would really have preferred a full-blown 64-bit counter as per RFC3530, but I suppose we could always combine this change attribute with the high word from ctime in order to make up the NFSv4 change attribute. That should keep us safe until someone develops a ramdisk with < 1 nsecond access time.

do you mean something like "(ctime.tv_sec << 32) | change_attribute"? this would allow 2^32 inode changes per second.

Quoted 9 lines · Alexandre Ratchov

Quoted 5 lines · Trond Myklebust

I would really have preferred a full-blown 64-bit counter as per RFC3530, but I suppose we could always combine this change attribute with the high word from ctime in order to make up the NFSv4 change attribute. That should keep us safe until someone develops a ramdisk with < 1 nsecond access time.

do you mean something like "(ctime.tv_sec << 32) | change_attribute"? this would allow 2^32 inode changes per second.

I've been giving this further thought, and it may be that a full 64-bit counter per inode is the only bulletproof solution.

One reason that ctime+nsec as the version number isn't so great is that if there is some reason to set the clock backward (i.e. it was incorrectly set into the future at some point) the inode ctime may jump backward. This could cause either misordering of events, or collisions between version numbers. The problem could be mitigated by having the ctime+nsec value only increment the nsec component by 1 for each new version (like a counter) until real time catches up with the bad ctime, but it might leave files with a bad ctime for a long time.

The main drawback of a 64-bit counter is the space in the inode that it consumes... I don't think we can find 64 bits of free space in the core inode, so this would relegate the solution to new filesystems that are formatted with large inodes.

Quoted 12 lines · Andreas Dilger

I've been giving this further thought, and it may be that a full 64-bit counter per inode is the only bulletproof solution.

One reason that ctime+nsec as the version number isn't so great is that if there is some reason to set the clock backward (i.e. it was incorrectly set into the future at some point) the inode ctime may jump backward. This could cause either misordering of events, or collisions between version numbers. The problem could be mitigated by having the ctime+nsec value only increment the nsec component by 1 for each new version (like a counter) until real time catches up with the bad ctime, but it might leave files with a bad ctime for a long time.

Quoted 4 lines

The main drawback of a 64-bit counter is the space in the inode that it consumes... I don't think we can find 64 bits of free space in the core inode, so this would relegate the solution to new filesystems that are formatted with large inodes.

Alexandre, Trond,
what do you think about using a 32-bit in-inode version (sufficient for causal uses of NFSv4),
and put the 32-bit MSB of the version into the
large part of the inode (say after cr_time)?

That allows use of the version for existing ext3 filesystems, and with large inodes (Lustre, ext4) it also meets the specs of RFC 3530 and any intended NFSv4 future use?

Quoted 18 lines · Andreas Dilger

Quoted 12 lines · Andreas Dilger

I've been giving this further thought, and it may be that a full 64-bit counter per inode is the only bulletproof solution.

One reason that ctime+nsec as the version number isn't so great is that if there is some reason to set the clock backward (i.e. it was incorrectly set into the future at some point) the inode ctime may jump backward. This could cause either misordering of events, or collisions between version numbers. The problem could be mitigated by having the ctime+nsec value only increment the nsec component by 1 for each new version (like a counter) until real time catches up with the bad ctime, but it might leave files with a bad ctime for a long time.

Quoted 4 lines

The main drawback of a 64-bit counter is the space in the inode that it consumes... I don't think we can find 64 bits of free space in the core inode, so this would relegate the solution to new filesystems that are formatted with large inodes.

Quoted 5 lines

Alexandre, Trond,
what do you think about using a 32-bit in-inode version (sufficient for causal uses of NFSv4),
and put the 32-bit MSB of the version into the
large part of the inode (say after cr_time)?

So does that mean that the MSB of the change attribute would only be available on some filesystems, or that it would be available on all of them but be slower on those with smaller inodes? And how does the user (e.g. the nfsd code) distinguish the two cases?

Quoted 3 lines

That allows use of the version for existing ext3 filesystems, and with large inodes (Lustre, ext4) it also meets the specs of RFC 3530 and any intended NFSv4 future use?

Quoted 9 lines · Alexandre Ratchov

hello,

here is a small patch that adds the "change attribute" for ext3 file-systems;

the change attribute is a simple counter that is reset to zero on inode creation and that is incremented every time the inode data is modified (similarly to the "ctime" time-stamp).

I would really have preferred a full-blown 64-bit counter as per RFC3530, but I suppose we could always combine this change attribute with the high word from ctime in order to make up the NFSv4 change attribute. That should keep us safe until someone develops a ramdisk with < 1 nsecond access time.

Wouldn't the generation count work better than ctime to differentiate between
instances of files using the same inode number? That way, there wouldn't be a clock resolution issue.

Quoted 12 lines · Trond Myklebust

Quoted 9 lines · Alexandre Ratchov

hello,

here is a small patch that adds the "change attribute" for ext3 file-systems;

the change attribute is a simple counter that is reset to zero on inode creation and that is incremented every time the inode data is modified (similarly to the "ctime" time-stamp).

Quoted 5 lines

I would really have preferred a full-blown 64-bit counter as per RFC3530, but I suppose we could always combine this change attribute with the high word from ctime in order to make up the NFSv4 change attribute. That should keep us safe until someone develops a ramdisk with < 1 nsecond access time.

Wouldn't the generation count work better than ctime to differentiate between
instances of files using the same inode number? That way, there wouldn't be a clock resolution issue.

Quoted 6 lines · Alexandre Ratchov

here is a small patch that adds the "change attribute" for ext3 file-systems;

the change attribute is a simple counter that is reset to zero on inode creation and that is incremented every time the inode data is modified (similarly to the "ctime" time-stamp).

I would really have preferred a full-blown 64-bit counter as per RFC3530, but I suppose we could always combine this change attribute with the high word from ctime in order to make up the NFSv4 change attribute. That should keep us safe until someone develops a ramdisk with < 1 nsecond access time.

Quoted 5 lines · Trond Myklebust

I would really have preferred a full-blown 64-bit counter as per RFC3530, but I suppose we could always combine this change attribute with the high word from ctime in order to make up the NFSv4 change attribute. That should keep us safe until someone develops a ramdisk with < 1 nsecond access time.

Trond, can you please elaborate on the need for a 64-bit version counter for NFSv4?

What kind of requirements does NFSv4 place on the version? Monotonic is probably a good bet.

Does it need to be global for the filesystem

or is a per-inode version sufficient?

What functionality of NFSv4 needs the version?

Quoted 2 lines

What kind of requirements does NFSv4 place on the version? Monotonic is probably a good bet.

The only requirement is that it be unique (assuming a file is never modified 2^64 times). Clients can't compare them except for equality.

Quoted 1 line

Does it need to be global for the filesystem

Nope.

Quoted 1 line

or is a per-inode version sufficient?

Yes.

Quoted 1 line

What functionality of NFSv4 needs the version?

Clients use it to revalidate their caches.

Quoted 2 lines

The only requirement is that it be unique (assuming a file is never modified 2^64 times). Clients can't compare them except for equality.

The other requirement is that they be updated in more or less any situation where you would normally see a 'ctime' update. In other words any time when the file metadata or data changes, and any time when the ACL changes.

Yup. It is used to detect changes made on the NFS server itself (possibly by other NFS clients, possibly by local processes on the server), so that the client can flush out any stale cached data.

the change attribute is a simple counter that is reset to zero on inode creation and that is incremented every time the inode data is modified (similarly to the "ctime" time-stamp).

The patch also adds a new ``st_change_attribute'' field in the stat structure, and modifies the stat(2) syscall accordingly. Currently the change is only visible on i386 and x86_64 archs.

inode.c +1 -2

+++ fs/inode.c	13 Sep 2006 18:15:43 -0000	1.1.1.3.2.1

@@ -1232,16 +1232,10 @@ void file_update_time(struct file *file)
 		return;
 
 	now = current_fs_time(inode->i_sb);
-	if (!timespec_equal(&inode->i_mtime, &now))
-		sync_it = 1;

Index: include/linux/ext3_fs.h
===================================================================
RCS file: /home/ratchova/cvs/linux/include/linux/ext3_fs.h,v retrieving revision 1.1.1.3
retrieving revision 1.1.1.3.2.1
diff -u -p -r1.1.1.3 -r1.1.1.3.2.1

Quoted 3 lines · Alexandre Ratchov

The patch also adds a new ``st_change_attribute'' field in the stat structure, and modifies the stat(2) syscall accordingly. Currently the change is only visible on i386 and x86_64 archs.

Is this really necessary for knfsd?

Quoted 3 lines · Alexandre Ratchov

the change attribute is a simple counter that is reset to zero on inode creation and that is incremented every time the inode data is modified (similarly to the "ctime" time-stamp).

To start, I'm supportive of this concept, my comments are only to get the most efficient implementation.

This appears to be very similar to the i_version field that is already in the inode (which is also modified only by ext3), so instead of increasing the size of the inode further we could use that field and just make it persistent on disk. The i_version field is incremented in mkdir/rmdir/create/rename. For Lustre it would also be desirable to modify the version field for regular files when they are modified (e.g. setattr, write), and it appears NFS v4 also wants the same (assumed from use of file_update_time()). The question is whether this should be handled internal to the filesystem (as ext3 does now) or if it should be part of the VFS.

Quoted 3 lines

The patch also adds a new ``st_change_attribute'' field in the stat structure, and modifies the stat(2) syscall accordingly. Currently the change is only visible on i386 and x86_64 archs.

Is this really necessary for knfsd?

binfmt_misc.c static struct inode *bm_get_inode(struct · line 514

Quoted 5 lines

binfmt_misc.c +1 -0

@@ -511,6 +511,7 @@ static struct inode *bm_get_inode(struct
 		inode->i_blocks = 0;
 		inode->i_atime = inode->i_mtime = inode->i_ctime =
 			current_fs_time(inode->i_sb);
+		inode->i_change_attribute = 0;

Initializing to zero is more dangerous than any non-zero number, since this is the most likely outcome of corruption... The current ext3 code initializes i_version to a random number, and we can use comparisons similar to jiffies as long as we don't expect > 2^31 changes between comparisons.

Quoted 7 lines

inode.c +1 -2

+++ fs/inode.c	13 Sep 2006 18:15:43 -0000	1.1.1.3.2.1

@@ -1232,16 +1232,10 @@ void file_update_time(struct file *file)
 		return;
 
 	now = current_fs_time(inode->i_sb);
-	if (!timespec_equal(&inode->i_mtime, &now))
-		sync_it = 1;

inode.c void file_update_time(struct file *file) · line 1238

Quoted 10 lines

inode.c +2 -6

 	inode->i_mtime = now;
-
-	if (!timespec_equal(&inode->i_ctime, &now))
-		sync_it = 1;
 	inode->i_ctime = now;
-
-	if (sync_it)
-		mark_inode_dirty_sync(inode);
+	inode->i_change_attribute++;
+	mark_inode_dirty_sync(inode);

Ugh, this would definitely hurt performance, because ext3_dirty_inode() packs-for-disk the whole inode each time. I believe Stephen had patches at one time to do the inode packing at transaction commit time instead of when it is changed, so we only do the packing once. Having a generic mechanism to do pre-commit callbacks from jbd (i.e. to pack a dirty inode to the buffer) would also be useful for other things like doing the inode or group descriptor checksum only once per transaction...

Quoted 13 lines

Index: include/linux/ext3_fs.h
===================================================================
RCS file: /home/ratchova/cvs/linux/include/linux/ext3_fs.h,v retrieving revision 1.1.1.3
retrieving revision 1.1.1.3.2.1
diff -u -p -r1.1.1.3 -r1.1.1.3.2.1

linux/ext3_fs.h +0 -1

@@ -286,7 +286,7 @@ struct ext3_inode {
 			__u16	i_pad1;
 			__le16	l_i_uid_high;	/* these 2 fields    */
 			__le16	l_i_gid_high;	/* were reserved2[0] */
-			__u32	l_i_reserved2;

linux/ext3_fs.h struct ext3_inode { · line 289

Quoted 1 line

linux/ext3_fs.h +1 -0

+			__le32	l_i_change_attribute;

There was some other use of the reserved fields for ext4 64-bit-blocks support. One was for i_file_acl_hi (I think this is using the i_pad1 above), one was for i_blocks_hi (I believe this was proposed to use the i_frag and i_fsize bytes). Is this conflicting with anything else? There were a lot of proposals for these fields, and I don't recall which ones are still out there.

linux/ext3_fs.h typedef void (dio_iodone_t)(struct kiocb · line 280

Quoted 3 lines

linux/ext3_fs.h +1 -0

@@ -280,6 +280,7 @@ typedef void (dio_iodone_t)(struct kiocb
+#define ATTR_CHANGE_ATTRIBUTE  16384

Do you need a setattr interface for this, or is it sufficient to use the i_version field from the inode, and let the filesystem manage i_version updates as it is doing now?

IMHO, the natural place to do this stuff is the VFS, because it can be common to all file-systems supporting this feature. Currently it's the same with ctime, mtime and atime. These are in the VFS even if there are file-systems that don't support all of them.

Quoted 7 lines

Ugh, this would definitely hurt performance, because ext3_dirty_inode() packs-for-disk the whole inode each time. I believe Stephen had patches at one time to do the inode packing at transaction commit time instead of when it is changed, so we only do the packing once. Having a generic mechanism to do pre-commit callbacks from jbd (i.e. to pack a dirty inode to the buffer) would also be useful for other things like doing the inode or group descriptor checksum only once per transaction...

afaik, for an open file, there is always a copy of the inode in memory, because there is a reference to it in the file structure. So, in principle, we shouldn't need to make dirty the inode. I don't know if this is feasable perhaps am i missing something here.

it's not strictly necessary; it's not more necessary that the interface to ctime or other attributes. It's here for completness, in my opinion the change attribute is the same as the ctime time-stamp

Quoted 4 lines · Alexandre Ratchov

IMHO, the natural place to do this stuff is the VFS, because it can be common to all file-systems supporting this feature. Currently it's the same with ctime, mtime and atime. These are in the VFS even if there are file-systems that don't support all of them.

Well, that is only partly true. I see lots of places in ext3 that are setting i_ctime and i_mtime...

Quoted 8 lines

Quoted 7 lines

Ugh, this would definitely hurt performance, because ext3_dirty_inode() packs-for-disk the whole inode each time. I believe Stephen had patches at one time to do the inode packing at transaction commit time instead of when it is changed, so we only do the packing once. Having a generic mechanism to do pre-commit callbacks from jbd (i.e. to pack a dirty inode to the buffer) would also be useful for other things like doing the inode or group descriptor checksum only once per transaction...

Quoted 4 lines

afaik, for an open file, there is always a copy of the inode in memory, because there is a reference to it in the file structure. So, in principle, we shouldn't need to make dirty the inode. I don't know if this is feasable perhaps am i missing something here.

The in-memory inode needs to be copied into the buffer so that it is part of the transaction being committed to disk, or updates are lost. This was a common bug with early ext3 - marking the inode dirty and then changing a field in the in-core inode - which would not be saved to disk. In other filesystems this is only a few-cycle race, but in ext3 the in-core inode is not written to disk unless the inode is again marked dirty.

The potential benefit of making this a callback from the JBD layer is it avoids copying the inode for EVERY dirty, and only doing it once per transaction. Add a list of callbacks hooked onto the transaction to be called before it is committed, and the callback data is the inode pointer which does a single ext3_do_update_inode() call if the inode is still marked dirty.

Quoted 3 lines

it's not strictly necessary; it's not more necessary that the interface to ctime or other attributes. It's here for completness, in my opinion the change attribute is the same as the ctime time-stamp

Then makes sense to just improve the ctime mechanism instead of adding new code and interfaces...

BTW, note that when we'll add support for nanosecond time-stamps, we still have the same problem, because with a very high clock and time-stamp resolutions, we'll have to update the inode on every change.