[PATCH] add some notes on file_update_time

[PATCH] add some notes on file_update_time

[Christoph Hellwig]

add some notes on why this function should only be used in the write
methods, and what NOCTIME is about.


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

Index: linux-2.6/fs/inode.c
===================================================================
--- linux-2.6.orig/fs/inode.c	2005-11-08 12:57:19.000000000 +0100
+++ linux-2.6/fs/inode.c	2005-11-08 12:57:19.000000000 +0100
@@ -1211,7 +1211,12 @@
  *	@file: file accessed
  *
  *	Update the mtime and ctime members of an inode and mark the inode
- *	for writeback.
+ *	for writeback.  Note that this function is meant exclusively for
+ *	usage in the file write path of filesystems, and filesystems may
+ *	chose to explicitly ignore update via this function with the
+ *	S_NOCTIME inode flag, e.g. for network filesystem where these
+ *	timestamps are handled by the server.  All explicit ctime/mtime
+ *	updates must be done via notify_change().
  */
 
 void file_update_time(struct file *file)

Re: [PATCH] add some notes on file_update_time

[Anton Altaparmakov]

On Fri, 11 Nov 2005, Christoph Hellwig wrote:
> add some notes on why this function should only be used in the write
> methods, and what NOCTIME is about.
>
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: linux-2.6/fs/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/inode.c	2005-11-08 12:57:19.000000000 +0100
> +++ linux-2.6/fs/inode.c	2005-11-08 12:57:19.000000000 +0100
> @@ -1211,7 +1211,12 @@
>   *	@file: file accessed
>   *
>   *	Update the mtime and ctime members of an inode and mark the inode
> - *	for writeback.
> + *	for writeback.  Note that this function is meant exclusively for
> + *	usage in the file write path of filesystems, and filesystems may
> + *	chose to explicitly ignore update via this function with the
> + *	S_NOCTIME inode flag, e.g. for network filesystem where these
> + *	timestamps are handled by the server.  All explicit ctime/mtime
> + *	updates must be done via notify_change().
>   */
>  
>  void file_update_time(struct file *file)

Nice explanation except for the last sentence.  Surely you do not expect 
filesystem drivers to call notify_change()?  I thought you told me that I 
should be setting the c/mtime directly, rather than using a function, so 
now you are going back on that?  Or am I simply misunderstanding the 
sentence completely?  I apologize if that is the case...

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] add some notes on file_update_time

[Christoph Hellwig]

> Nice explanation except for the last sentence.  Surely you do not expect 
> filesystem drivers to call notify_change()?  I thought you told me that I 
> should be setting the c/mtime directly, rather than using a function, so 
> now you are going back on that?  Or am I simply misunderstanding the 
> sentence completely?  I apologize if that is the case...

this was meant for non-fs callers.  but to be there's no reason
why non-fs callers should look at this function in the first time.

The updated patch below just drop the sentence.  we might want to write
up rules on c/mtime updates somewhere, but that's really not related to
file_update_time.


Index: linux-2.6/fs/inode.c
===================================================================
--- linux-2.6.orig/fs/inode.c	2005-11-10 16:02:18.000000000 +0100
+++ linux-2.6/fs/inode.c	2005-11-10 16:04:25.000000000 +0100
@@ -1208,7 +1208,11 @@
  *	@file: file accessed
  *
  *	Update the mtime and ctime members of an inode and mark the inode
- *	for writeback.
+ *	for writeback.  Note that this function is meant exclusively for
+ *	usage in the file write path of filesystems, and filesystems may
+ *	chose to explicitly ignore update via this function with the
+ *	S_NOCTIME inode flag, e.g. for network filesystem where these
+ *	timestamps are handled by the server.
  */
 
 void file_update_time(struct file *file)

Re: [PATCH] add some notes on file_update_time

[Christoph Hellwig]

On Fri, Nov 11, 2005 at 12:45:27AM +0100, Christoph Hellwig wrote:
> > Nice explanation except for the last sentence.  Surely you do not expect 
> > filesystem drivers to call notify_change()?  I thought you told me that I 
> > should be setting the c/mtime directly, rather than using a function, so 
> > now you are going back on that?  Or am I simply misunderstanding the 
> > sentence completely?  I apologize if that is the case...
> 
> this was meant for non-fs callers.  but to be there's no reason
> why non-fs callers should look at this function in the first time.
> 
> The updated patch below just drop the sentence.  we might want to write
> up rules on c/mtime updates somewhere, but that's really not related to
> file_update_time.

And here's another update, as Nathan Scott found a small typo:

Index: linux-2.6/fs/inode.c
===================================================================
--- linux-2.6.orig/fs/inode.c	2005-11-10 16:02:18.000000000 +0100
+++ linux-2.6/fs/inode.c	2005-11-10 16:05:55.000000000 +0100
@@ -1208,7 +1208,11 @@
  *	@file: file accessed
  *
  *	Update the mtime and ctime members of an inode and mark the inode
- *	for writeback.
+ *	for writeback.  Note that this function is meant exclusively for
+ *	usage in the file write path of filesystems, and filesystems may
+ *	choose to explicitly ignore update via this function with the
+ *	S_NOCTIME inode flag, e.g. for network filesystem where these
+ *	timestamps are handled by the server.
  */
 
 void file_update_time(struct file *file)