[PATCH] replace inode_update_time with file_update_time

[PATCH] replace inode_update_time with file_update_time

[Christoph Hellwig]

To allow various options to work per-mount instead of per-sb we need
a struct vfsmount when updating ctime and mtime.  This preparation
patch replaces the inode_update_time routine with a file_update_atime
routine so we can easily get at the vfsmount.  (and the file makes more
sense in this context anyway).  Also get rid of the unused second
argument - we always want to update the ctime when calling this routine.


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/fs/inode.c
===================================================================
--- linux-2.6.orig/fs/inode.c	2005-10-29 04:14:44.000000000 +0200
+++ linux-2.6/fs/inode.c	2005-10-29 04:14:47.000000000 +0200
@@ -1203,16 +1203,16 @@
 EXPORT_SYMBOL(update_atime);
 
 /**
- *	inode_update_time	-	update mtime and ctime time
- *	@inode: inode accessed
- *	@ctime_too: update ctime too
+ *	file_update_time	-	update mtime and ctime time
+ *	@file: file accessed
  *
- *	Update the mtime time on an inode and mark it for writeback.
- *	When ctime_too is specified update the ctime too.
+ *	Update the mtime and ctime members of an inode and mark the inode
+ *	for writeback.
  */
 
-void inode_update_time(struct inode *inode, int ctime_too)
+void file_update_time(struct file *file)
 {
+	struct inode *inode = file->f_dentry->d_inode;
 	struct timespec now;
 	int sync_it = 0;
 
@@ -1226,16 +1226,15 @@
 		sync_it = 1;
 	inode->i_mtime = now;
 
-	if (ctime_too) {
-		if (!timespec_equal(&inode->i_ctime, &now))
-			sync_it = 1;
-		inode->i_ctime = now;
-	}
+	if (!timespec_equal(&inode->i_ctime, &now))
+		sync_it = 1;
+	inode->i_ctime = now;
+
 	if (sync_it)
 		mark_inode_dirty_sync(inode);
 }
 
-EXPORT_SYMBOL(inode_update_time);
+EXPORT_SYMBOL(file_update_time);
 
 int inode_needs_sync(struct inode *inode)
 {
Index: linux-2.6/fs/ncpfs/file.c
===================================================================
--- linux-2.6.orig/fs/ncpfs/file.c	2005-10-29 04:14:44.000000000 +0200
+++ linux-2.6/fs/ncpfs/file.c	2005-10-29 04:14:47.000000000 +0200
@@ -262,7 +262,7 @@
 	}
 	vfree(bouncebuffer);
 
-	inode_update_time(inode, 1);
+	file_update_time(file);
 
 	*ppos = pos;
 
Index: linux-2.6/fs/pipe.c
===================================================================
--- linux-2.6.orig/fs/pipe.c	2005-10-29 04:14:44.000000000 +0200
+++ linux-2.6/fs/pipe.c	2005-10-29 04:14:47.000000000 +0200
@@ -347,7 +347,7 @@
 		kill_fasync(PIPE_FASYNC_READERS(*inode), SIGIO, POLL_IN);
 	}
 	if (ret > 0)
-		inode_update_time(inode, 1);	/* mtime and ctime */
+		file_update_time(filp);
 	return ret;
 }
 
Index: linux-2.6/fs/reiserfs/file.c
===================================================================
--- linux-2.6.orig/fs/reiserfs/file.c	2005-10-29 04:14:44.000000000 +0200
+++ linux-2.6/fs/reiserfs/file.c	2005-10-29 04:14:47.000000000 +0200
@@ -1360,7 +1360,7 @@
 	if (res)
 		goto out;
 
-	inode_update_time(inode, 1);	/* Both mtime and ctime */
+	file_update_time(file);
 
 	// Ok, we are done with all the checks.
 
Index: linux-2.6/fs/xfs/linux-2.6/xfs_lrw.c
===================================================================
--- linux-2.6.orig/fs/xfs/linux-2.6/xfs_lrw.c	2005-10-29 04:14:44.000000000 +0200
+++ linux-2.6/fs/xfs/linux-2.6/xfs_lrw.c	2005-10-29 04:14:47.000000000 +0200
@@ -740,7 +740,7 @@
 	 */
 	if (!(ioflags & IO_INVIS)) {
 		xfs_ichgtime(xip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
-		inode_update_time(inode, 1);
+		file_update_time(file);
 	}
 
 	/*
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2005-10-29 04:14:44.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2005-10-29 04:15:01.000000000 +0200
@@ -1651,7 +1651,7 @@
 extern int inode_change_ok(struct inode *, struct iattr *);
 extern int __must_check inode_setattr(struct inode *, struct iattr *);
 
-extern void inode_update_time(struct inode *inode, int ctime_too);
+extern void file_update_time(struct file *file);
 
 static inline ino_t parent_ino(struct dentry *dentry)
 {
Index: linux-2.6/mm/filemap.c
===================================================================
--- linux-2.6.orig/mm/filemap.c	2005-10-29 04:14:44.000000000 +0200
+++ linux-2.6/mm/filemap.c	2005-10-29 04:14:47.000000000 +0200
@@ -2066,7 +2066,7 @@
 	if (err)
 		goto out;
 
-	inode_update_time(inode, 1);
+	file_update_time(file);
 
 	/* coalesce the iovecs and go direct-to-BIO for O_DIRECT */
 	if (unlikely(file->f_flags & O_DIRECT)) {
Index: linux-2.6/mm/filemap_xip.c
===================================================================
--- linux-2.6.orig/mm/filemap_xip.c	2005-10-29 04:14:44.000000000 +0200
+++ linux-2.6/mm/filemap_xip.c	2005-10-29 04:14:47.000000000 +0200
@@ -381,7 +381,7 @@
 	if (ret)
 		goto out_backing;
 
-	inode_update_time(inode, 1);
+	file_update_time(filp);
 
 	ret = __xip_file_write (filp, buf, count, pos, ppos);
 

Re: [PATCH] replace inode_update_time with file_update_time

[Andrew Morton]

Christoph Hellwig <hch@lst.de> wrote:
>
>  	struct timespec now;
> ...
> +	inode->i_ctime = now;

hm, that nonatomic assignment is racy wrt readers and, conceivably, other
writers.

Re: [PATCH] replace inode_update_time with file_update_time

[Christoph Hellwig]

On Mon, Oct 31, 2005 at 02:54:19PM -0800, Andrew Morton wrote:
> Christoph Hellwig <hch@lst.de> wrote:
> >
> >  	struct timespec now;
> > ...
> > +	inode->i_ctime = now;
> 
> hm, that nonatomic assignment is racy wrt readers and, conceivably, other
> writers.

this function is always called under i_sem

Re: [PATCH] replace inode_update_time with file_update_time

[Anton Altaparmakov]

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.)

> a struct vfsmount when updating ctime and mtime.  This preparation
> patch replaces the inode_update_time routine with a file_update_atime
> routine so we can easily get at the vfsmount.  (and the file makes more
> sense in this context anyway).  Also get rid of the unused second
> argument - we always want to update the ctime when calling this routine.

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).

Best regards,

	Anton

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: linux-2.6/fs/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/inode.c	2005-10-29 04:14:44.000000000 +0200
> +++ linux-2.6/fs/inode.c	2005-10-29 04:14:47.000000000 +0200
> @@ -1203,16 +1203,16 @@
>  EXPORT_SYMBOL(update_atime);
>  
>  /**
> - *	inode_update_time	-	update mtime and ctime time
> - *	@inode: inode accessed
> - *	@ctime_too: update ctime too
> + *	file_update_time	-	update mtime and ctime time
> + *	@file: file accessed
>   *
> - *	Update the mtime time on an inode and mark it for writeback.
> - *	When ctime_too is specified update the ctime too.
> + *	Update the mtime and ctime members of an inode and mark the inode
> + *	for writeback.
>   */
>  
> -void inode_update_time(struct inode *inode, int ctime_too)
> +void file_update_time(struct file *file)
>  {
> +	struct inode *inode = file->f_dentry->d_inode;
>  	struct timespec now;
>  	int sync_it = 0;
>  
> @@ -1226,16 +1226,15 @@
>  		sync_it = 1;
>  	inode->i_mtime = now;
>  
> -	if (ctime_too) {
> -		if (!timespec_equal(&inode->i_ctime, &now))
> -			sync_it = 1;
> -		inode->i_ctime = now;
> -	}
> +	if (!timespec_equal(&inode->i_ctime, &now))
> +		sync_it = 1;
> +	inode->i_ctime = now;
> +
>  	if (sync_it)
>  		mark_inode_dirty_sync(inode);
>  }
>  
> -EXPORT_SYMBOL(inode_update_time);
> +EXPORT_SYMBOL(file_update_time);
>  
>  int inode_needs_sync(struct inode *inode)
>  {
> Index: linux-2.6/fs/ncpfs/file.c
> ===================================================================
> --- linux-2.6.orig/fs/ncpfs/file.c	2005-10-29 04:14:44.000000000 +0200
> +++ linux-2.6/fs/ncpfs/file.c	2005-10-29 04:14:47.000000000 +0200
> @@ -262,7 +262,7 @@
>  	}
>  	vfree(bouncebuffer);
>  
> -	inode_update_time(inode, 1);
> +	file_update_time(file);
>  
>  	*ppos = pos;
>  
> Index: linux-2.6/fs/pipe.c
> ===================================================================
> --- linux-2.6.orig/fs/pipe.c	2005-10-29 04:14:44.000000000 +0200
> +++ linux-2.6/fs/pipe.c	2005-10-29 04:14:47.000000000 +0200
> @@ -347,7 +347,7 @@
>  		kill_fasync(PIPE_FASYNC_READERS(*inode), SIGIO, POLL_IN);
>  	}
>  	if (ret > 0)
> -		inode_update_time(inode, 1);	/* mtime and ctime */
> +		file_update_time(filp);
>  	return ret;
>  }
>  
> Index: linux-2.6/fs/reiserfs/file.c
> ===================================================================
> --- linux-2.6.orig/fs/reiserfs/file.c	2005-10-29 04:14:44.000000000 +0200
> +++ linux-2.6/fs/reiserfs/file.c	2005-10-29 04:14:47.000000000 +0200
> @@ -1360,7 +1360,7 @@
>  	if (res)
>  		goto out;
>  
> -	inode_update_time(inode, 1);	/* Both mtime and ctime */
> +	file_update_time(file);
>  
>  	// Ok, we are done with all the checks.
>  
> Index: linux-2.6/fs/xfs/linux-2.6/xfs_lrw.c
> ===================================================================
> --- linux-2.6.orig/fs/xfs/linux-2.6/xfs_lrw.c	2005-10-29 04:14:44.000000000 +0200
> +++ linux-2.6/fs/xfs/linux-2.6/xfs_lrw.c	2005-10-29 04:14:47.000000000 +0200
> @@ -740,7 +740,7 @@
>  	 */
>  	if (!(ioflags & IO_INVIS)) {
>  		xfs_ichgtime(xip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
> -		inode_update_time(inode, 1);
> +		file_update_time(file);
>  	}
>  
>  	/*
> Index: linux-2.6/include/linux/fs.h
> ===================================================================
> --- linux-2.6.orig/include/linux/fs.h	2005-10-29 04:14:44.000000000 +0200
> +++ linux-2.6/include/linux/fs.h	2005-10-29 04:15:01.000000000 +0200
> @@ -1651,7 +1651,7 @@
>  extern int inode_change_ok(struct inode *, struct iattr *);
>  extern int __must_check inode_setattr(struct inode *, struct iattr *);
>  
> -extern void inode_update_time(struct inode *inode, int ctime_too);
> +extern void file_update_time(struct file *file);
>  
>  static inline ino_t parent_ino(struct dentry *dentry)
>  {
> Index: linux-2.6/mm/filemap.c
> ===================================================================
> --- linux-2.6.orig/mm/filemap.c	2005-10-29 04:14:44.000000000 +0200
> +++ linux-2.6/mm/filemap.c	2005-10-29 04:14:47.000000000 +0200
> @@ -2066,7 +2066,7 @@
>  	if (err)
>  		goto out;
>  
> -	inode_update_time(inode, 1);
> +	file_update_time(file);
>  
>  	/* coalesce the iovecs and go direct-to-BIO for O_DIRECT */
>  	if (unlikely(file->f_flags & O_DIRECT)) {
> Index: linux-2.6/mm/filemap_xip.c
> ===================================================================
> --- linux-2.6.orig/mm/filemap_xip.c	2005-10-29 04:14:44.000000000 +0200
> +++ linux-2.6/mm/filemap_xip.c	2005-10-29 04:14:47.000000000 +0200
> @@ -381,7 +381,7 @@
>  	if (ret)
>  		goto out_backing;
>  
> -	inode_update_time(inode, 1);
> +	file_update_time(filp);
>  
>  	ret = __xip_file_write (filp, buf, count, pos, ppos);
>  
> -
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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/

Re: [PATCH] replace inode_update_time with file_update_time

[Shaya Potter]

On Mon, 2005-11-07 at 21:40 +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.)

I'm thinking you should think of things like read only bind mounts,
where even meta data wont be updated.

Re: [PATCH] replace inode_update_time with file_update_time

[Anton Altaparmakov]

On Mon, 7 Nov 2005, Shaya Potter wrote:
> On Mon, 2005-11-07 at 21:40 +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.)
> 
> I'm thinking you should think of things like read only bind mounts,
> where even meta data wont be updated.

But that is my point!  A read-only bind mount is just like any other 
read-only mount and should never even try to update metadata.

Which codepaths cause inode/file_update_time() to be called for a 
read-only mount?

I do not believe there are any!

And assuming that I am correct this makes the IS_RDONLY() check 
pointless and this in turn makes the whole patch pointless and given it 
breaks existing file systems it should be sent to the nirvana of useless 
patches.

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/

Re: [PATCH] replace inode_update_time with file_update_time

[Shaya Potter]

On Mon, 2005-11-07 at 22:02 +0000, Anton Altaparmakov wrote:
> On Mon, 7 Nov 2005, Shaya Potter wrote:
> > On Mon, 2005-11-07 at 21:40 +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.)
> > 
> > I'm thinking you should think of things like read only bind mounts,
> > where even meta data wont be updated.
> 
> But that is my point!  A read-only bind mount is just like any other 
> read-only mount and should never even try to update metadata.

there's no such thing as a read only bind mount right now, I believe
this is to enable support of such a thing, but I could be wrong.

Re: [PATCH] replace inode_update_time with file_update_time

[Anton Altaparmakov]

On Mon, 7 Nov 2005, Shaya Potter wrote:
> On Mon, 2005-11-07 at 22:02 +0000, Anton Altaparmakov wrote:
> > On Mon, 7 Nov 2005, Shaya Potter wrote:
> > > On Mon, 2005-11-07 at 21:40 +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.)
> > > 
> > > I'm thinking you should think of things like read only bind mounts,
> > > where even meta data wont be updated.
> > 
> > But that is my point!  A read-only bind mount is just like any other 
> > read-only mount and should never even try to update metadata.
> 
> there's no such thing as a read only bind mount right now, I believe
> this is to enable support of such a thing, but I could be wrong.

Correct and I am countering that to support a read only bind mount, you do 
_not_ need this patch at all.  So I am asking whether it is needed for 
other options than read only and if so what are those.

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/

Re: [PATCH] replace inode_update_time with file_update_time

[Christoph Hellwig]

On Mon, Nov 07, 2005 at 10:02:01PM +0000, Anton Altaparmakov wrote:
> But that is my point!  A read-only bind mount is just like any other 
> read-only mount and should never even try to update metadata.
> 
> Which codepaths cause inode/file_update_time() to be called for a 
> read-only mount?

The callers right now are the write methods of the various filesystems,
so you are right indeed that we shouldn't need it.

> I do not believe there are any!
> 
> And assuming that I am correct this makes the IS_RDONLY() check 
> pointless and this in turn makes the whole patch pointless

Yes, I think you're right here that we can removed it.

> and given it 
> breaks existing file systems it should be sent to the nirvana of useless 
> patches.

and here I disagree. Given that the only intended use of this function
is to be used in the ->write methods the file is a useful argument.  The
only other use that sneaked in recently is ntfs ->truncate that
shouldn't use and has been fixed by a patch I sent offlist.

Re: [PATCH] replace inode_update_time with file_update_time

[Anton Altaparmakov]

On Tue, 8 Nov 2005, Christoph Hellwig wrote:
> On Mon, Nov 07, 2005 at 10:02:01PM +0000, Anton Altaparmakov wrote:
> > But that is my point!  A read-only bind mount is just like any other 
> > read-only mount and should never even try to update metadata.
> > 
> > Which codepaths cause inode/file_update_time() to be called for a 
> > read-only mount?
> 
> The callers right now are the write methods of the various filesystems,
> so you are right indeed that we shouldn't need it.
>
> > I do not believe there are any!
> > 
> > And assuming that I am correct this makes the IS_RDONLY() check 
> > pointless and this in turn makes the whole patch pointless
> 
> Yes, I think you're right here that we can removed it.

Thank you.  So lets drop it then.
 
> > and given it 
> > breaks existing file systems it should be sent to the nirvana of useless 
> > patches.
> 
> and here I disagree. Given that the only intended use of this function
> is to be used in the ->write methods the file is a useful argument.  The

It does not say so anywhere though...

> only other use that sneaked in recently is ntfs ->truncate that
> shouldn't use and has been fixed by a patch I sent offlist.

Yes, and a nice fix that was, too...  But hey, ntfs is already forced to 
duplicate half of the vfs, a bit more or less will not make any 
difference...

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/

Re: [PATCH] replace inode_update_time with file_update_time

[Christoph Hellwig]

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.

> 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.

Re: [PATCH] replace inode_update_time with file_update_time

[Anton Altaparmakov]

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/