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.

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.

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

Reply via email to