Re: i_version changes

[Christoph Hellwig <hch@lst.de>]

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.