Re: [PATCH] replace inode_update_time with file_update_time

[Anton Altaparmakov <aia21@cam.ac.uk>]

On Tue, 8 Nov 2005, Christoph Hellwig wrote:
> On Mon, Nov 07, 2005 at 09:40:51PM +0000, Anton Altaparmakov wrote:
> > On Sat, 29 Oct 2005, Christoph Hellwig wrote:
> > > To allow various options to work per-mount instead of per-sb we need
> > 
> > What are those various options?  Please spell them out.  (I mean it!  I 
> > really do not know what you have in mind and I cannot see anything that 
> > would require a vfs mount wrt cmtime updates.)
> 
> The ones I work now are noatime and read-only.

noatime is not dealt with at all in file/inode_update_time().

read-only check should be removed altogether.

Anything else?  (-:

> > I believe your patch is wrong/unnecessary because the only option that is 
> > per superblock (or in your new world per vfsmount) is the IS_RDONLY(inode) 
> > check and if a mount is read-only the VFS should never have allowed 
> > inode/file_update_time() to be called.  The codepath should have been 
> > aborted _much_ earlier than that so it does not make any sense to test for 
> > read-only here thus it is not necessary to do a per mountpoint check.  
> > Instead we should remove the readonly check altogether and if there are 
> > any places where an update of cmtime is attempted on a read-only mount we 
> > should fix those instead...
> > 
> > A more detailed justification for your patch is IMO required before it be 
> > applied to mainline, especially as it breaks existing file systems like 
> > ntfs (as discussed offlist).
> 
> Besides the above usage it's a nice cleanup and already found bugs like
> the API abuse in ntfs ->truncate.

I don't think it is a nice cleanup at all.  It is an inode operation not a 
file operation no matter what you say about it...

And if you don't want this to be used from anywhere other than file write 
& co, you ought to put a comment to that effect on it.  To me it seems the 
exact function one _should_ call rather than doing it oneself because it 
does the optimisations, etc, for free, without having to duplicate the 
whole function like you did to "fix" ntfs.

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/