Re: i_version changes

2008-02-15 Thread Jean noel Cordenner

Peter Staubach a écrit :

Few month ago, I ran a FFSB test on a 2.6.23 kernel enabling or not 
the i_version flag.

http://bullopensource.org/ext4/20071116/ffsb-write.html


This is good information.

A couple of questions -- what is the -I 256 option used for the ext4
mkfs?



This option force the inode size to 256Bytes instead of 128. The 
requirement is to have a 64bits i_version counter, therefore the ext4 
inode as to be extended to 256Bytes.



What was the variance between the results of the 5 runs?  Is 2%
significant or not?



I didn't keep the raw results, but they were all very close to the 
average (less than 1% of the average).


-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: i_version changes

2008-02-14 Thread Peter Staubach

Jean noel Cordenner wrote:

hi,

Peter Staubach a écrit :


Is the perceived performance hit really going to be as large
as suspected?  We already update the time fields fairly often
and we don't pay a huge penalty for those, or at least not a
penalty that we aren't willing to pay.  Has anyone measured
the cost?


Few month ago, I ran a FFSB test on a 2.6.23 kernel enabling or not 
the i_version flag.

http://bullopensource.org/ext4/20071116/ffsb-write.html


This is good information.

A couple of questions -- what is the -I 256 option used for the ext4
mkfs?

What was the variance between the results of the 5 runs?  Is 2%
significant or not?

   Thanx...

  ps
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: i_version changes

2008-02-13 Thread Andreas Dilger
On Feb 12, 2008  15:06 -0500, J. Bruce Fields wrote:
 On Sun, Feb 10, 2008 at 08:30:41AM +0100, Christoph Hellwig wrote:
  Third using the MS_ flag but then actually having a filesystem
  mount option to enable it is more than confusing.  After all MS_
  options (at least the exported parts) are the mount ABI for common
  options.  Also this option doesn't show up in -show_options,
  which is something Miklos will beat you up for :)
  I'm also not convinced this should be option behaviour, either you
  do update i_version for a given filesystem or you don't - having
  an obscure mount option will only give you confusion.
 
 That does sound likely to be confusing.  Any chance we could just make
 the new behavior mandatory?

One of the reasons NOT to make it mandatory is that it forces updates
of the inode after every write.  On ext3/ext4 this is expensive, as the
ext3_dirty_inode() packs the inode from memory into the buffer each time,
so that it can be journaled.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: i_version changes

2008-02-13 Thread Christoph Hellwig
On Tue, Feb 12, 2008 at 03:06:25PM -0500, J. Bruce Fields wrote:
  First there's a complete lack of documentation on this, which is very
  bad.  Please document what the new semantics for i_version on regular
  files are supposed to be, and how it differes from the existing
  semantics for directories.
  
  Second abusing one of the rather scare superblock mount flags is
  a bad idea.  It would be much better to set this through -setattr
  and an extension of struct iattr.  Especially as we need to convert
  file_update_time to update c and mtime through -setattr anyway.
 
 I don't understand that comment.  (What is this in the second
 sentence, for example?)

i_version.  Instead of hardcoding i_version updates in file_update_time
it would be better to add an ia_verstion to struct iattr and update it
that way.

  Third using the MS_ flag but then actually having a filesystem
  mount option to enable it is more than confusing.  After all MS_
  options (at least the exported parts) are the mount ABI for common
  options.  Also this option doesn't show up in -show_options,
  which is something Miklos will beat you up for :)
  I'm also not convinced this should be option behaviour, either you
  do update i_version for a given filesystem or you don't - having
  an obscure mount option will only give you confusion.
 
 That does sound likely to be confusing.  Any chance we could just make
 the new behavior mandatory?
 
 The one thing we need in nfsd is just an easy (in-kernel) way to check
 whether a given filesystem supports this, so nfsd can decide whether to
 use ctime or i_version as the change attribute.

Probably through export_operations somehow.  Andreas mentioned in the
other reply that he wants it only conditionally due to the overhead
on extN, and enabling this from an export operation called when nfs
exporting a filesystem.

Btw, stupid question:  the commit message for the i_version changes
mentions this is to work around lack of granularity for ctime updates.
But all modern filesystems (and I includ ext4 in that here) have 64bit
timestamps already, so that should be enough.  It would certainly
avoid all this additional code, and especially the additional space
used in struct inode which can hurt quite a lot.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: i_version changes

2008-02-13 Thread Andreas Dilger
On Feb 13, 2008  09:07 -0500, Trond Myklebust wrote:
 On Wed, 2008-02-13 at 13:52 +0100, Christoph Hellwig wrote:
  Btw, stupid question:  the commit message for the i_version changes
  mentions this is to work around lack of granularity for ctime updates.
  But all modern filesystems (and I includ ext4 in that here) have 64bit
  timestamps already, so that should be enough.  It would certainly
  avoid all this additional code, and especially the additional space
  used in struct inode which can hurt quite a lot.
 
 Support for 64-bit on-disk time stamps alone does not suffice. In order
 to label all file/directory changes unambiguously, you would also need
 nanosecond timekeeping support.
 
 An example is XFS, which has had on-disk support for 64-bit time stamps
 since forever, but is in practice limited by the Linux default 250Hz
 internal clock. We've seen plenty of examples of NFS clients missing
 updates on the resulting filesystem due to the fact that they occurred
 within 1/250 sec of each other.

The other issue which unfortunately makes ctime a non-starter is the
ability of ctime to go backward due to clock changes.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: i_version changes

2008-02-12 Thread J. Bruce Fields
On Sun, Feb 10, 2008 at 08:30:41AM +0100, Christoph Hellwig wrote:
 I think the i_version changes that hit mainline about a week ago are
 not as nice as they should be.
 
 First there's a complete lack of documentation on this, which is very
 bad.  Please document what the new semantics for i_version on regular
 files are supposed to be, and how it differes from the existing
 semantics for directories.
 
 Second abusing one of the rather scare superblock mount flags is
 a bad idea.  It would be much better to set this through -setattr
 and an extension of struct iattr.  Especially as we need to convert
 file_update_time to update c and mtime through -setattr anyway.

I don't understand that comment.  (What is this in the second
sentence, for example?)

 
 Third using the MS_ flag but then actually having a filesystem
 mount option to enable it is more than confusing.  After all MS_
 options (at least the exported parts) are the mount ABI for common
 options.  Also this option doesn't show up in -show_options,
 which is something Miklos will beat you up for :)
 I'm also not convinced this should be option behaviour, either you
 do update i_version for a given filesystem or you don't - having
 an obscure mount option will only give you confusion.

That does sound likely to be confusing.  Any chance we could just make
the new behavior mandatory?

The one thing we need in nfsd is just an easy (in-kernel) way to check
whether a given filesystem supports this, so nfsd can decide whether to
use ctime or i_version as the change attribute.

--b.

 Beyond those any good reason for making inode_inc_iversion inline,
 especially after the first patch introduced it properly out of line.
 
 And as a last note please stop pushing these kind of core changes
 through specific filesystem trees.  If this had been in -mm we
 would have caught this a lot earlier, and would have also meant you'd
 get input and possible even implementations from other filesystem
 maintainers.
 -
 To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html