From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Hellwig Subject: i_version changes Date: Sun, 10 Feb 2008 08:30:41 +0100 Message-ID: <20080210073041.GA23529@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org To: jean-noel.cordenner@bull.net Return-path: Received: from verein.lst.de ([213.95.11.210]:34467 "EHLO verein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755282AbYBJHbT (ORCPT ); Sun, 10 Feb 2008 02:31:19 -0500 Content-Disposition: inline Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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.