[PATCH] xfs: implement ->update_time

[PATCH] xfs: implement ->update_time

[Christoph Hellwig]

Use this new method to replace our hacky use of ->dirty_inode.  An additional
benefit is that we can now propagate errors up the stack.

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

---
 fs/xfs/xfs_iops.c  |   45 ++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_super.c |   56 -----------------------------------------------------
 fs/xfs/xfs_trace.h |    2 -
 3 files changed, 46 insertions(+), 57 deletions(-)

Index: xfs/fs/xfs/xfs_iops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iops.c	2012-06-04 07:45:36.000000000 -0400
+++ xfs/fs/xfs/xfs_iops.c	2012-06-05 18:41:09.060068319 -0400
@@ -897,6 +897,47 @@ xfs_vn_setattr(
 	return -xfs_setattr_nonsize(XFS_I(dentry->d_inode), iattr, 0);
 }
 
+STATIC int
+xfs_vn_update_time(
+	struct inode		*inode,
+	struct timespec		*now,
+	int			flags)
+{
+	struct xfs_inode	*ip = XFS_I(inode);
+	struct xfs_mount	*mp = ip->i_mount;
+	struct xfs_trans	*tp;
+	int			error;
+
+	trace_xfs_update_time(ip);
+
+	tp = xfs_trans_alloc(mp, XFS_TRANS_FSYNC_TS);
+	error = xfs_trans_reserve(tp, 0, XFS_FSYNC_TS_LOG_RES(mp), 0, 0, 0);
+	if (error) {
+		xfs_trans_cancel(tp, 0);
+		return -error;
+	}
+
+	xfs_ilock(ip, XFS_ILOCK_EXCL);
+	if (flags & S_CTIME) {
+		inode->i_ctime = *now;
+		ip->i_d.di_ctime.t_sec = (__int32_t)now->tv_sec;
+		ip->i_d.di_ctime.t_nsec = (__int32_t)now->tv_nsec;
+	}
+	if (flags & S_MTIME) {
+		inode->i_mtime = *now;
+		ip->i_d.di_mtime.t_sec = (__int32_t)now->tv_sec;
+		ip->i_d.di_mtime.t_nsec = (__int32_t)now->tv_nsec;
+	}
+	if (flags & S_ATIME) {
+		inode->i_atime = *now;
+		ip->i_d.di_atime.t_sec = (__int32_t)now->tv_sec;
+		ip->i_d.di_atime.t_nsec = (__int32_t)now->tv_nsec;
+	}
+	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
+	xfs_trans_log_inode(tp, ip, XFS_ILOG_TIMESTAMP);
+	return -xfs_trans_commit(tp, 0);
+}
+
 #define XFS_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC|FIEMAP_FLAG_XATTR)
 
 /*
@@ -991,6 +1032,7 @@ static const struct inode_operations xfs
 	.removexattr		= generic_removexattr,
 	.listxattr		= xfs_vn_listxattr,
 	.fiemap			= xfs_vn_fiemap,
+	.update_time		= xfs_vn_update_time,
 };
 
 static const struct inode_operations xfs_dir_inode_operations = {
@@ -1016,6 +1058,7 @@ static const struct inode_operations xfs
 	.getxattr		= generic_getxattr,
 	.removexattr		= generic_removexattr,
 	.listxattr		= xfs_vn_listxattr,
+	.update_time		= xfs_vn_update_time,
 };
 
 static const struct inode_operations xfs_dir_ci_inode_operations = {
@@ -1041,6 +1084,7 @@ static const struct inode_operations xfs
 	.getxattr		= generic_getxattr,
 	.removexattr		= generic_removexattr,
 	.listxattr		= xfs_vn_listxattr,
+	.update_time		= xfs_vn_update_time,
 };
 
 static const struct inode_operations xfs_symlink_inode_operations = {
@@ -1054,6 +1098,7 @@ static const struct inode_operations xfs
 	.getxattr		= generic_getxattr,
 	.removexattr		= generic_removexattr,
 	.listxattr		= xfs_vn_listxattr,
+	.update_time		= xfs_vn_update_time,
 };
 
 STATIC void
Index: xfs/fs/xfs/xfs_super.c
===================================================================
--- xfs.orig/fs/xfs/xfs_super.c	2012-06-04 07:45:36.000000000 -0400
+++ xfs/fs/xfs/xfs_super.c	2012-06-05 18:35:03.876058962 -0400
@@ -868,61 +868,6 @@ xfs_fs_inode_init_once(
 		     "xfsino", ip->i_ino);
 }
 
-/*
- * This is called by the VFS when dirtying inode metadata.  This can happen
- * for a few reasons, but we only care about timestamp updates, given that
- * we handled the rest ourselves.  In theory no other calls should happen,
- * but for example generic_write_end() keeps dirtying the inode after
- * updating i_size.  Thus we check that the flags are exactly I_DIRTY_SYNC,
- * and skip this call otherwise.
- *
- * We'll hopefull get a different method just for updating timestamps soon,
- * at which point this hack can go away, and maybe we'll also get real
- * error handling here.
- */
-STATIC void
-xfs_fs_dirty_inode(
-	struct inode		*inode,
-	int			flags)
-{
-	struct xfs_inode	*ip = XFS_I(inode);
-	struct xfs_mount	*mp = ip->i_mount;
-	struct xfs_trans	*tp;
-	int			error;
-
-	if (flags != I_DIRTY_SYNC)
-		return;
-
-	trace_xfs_dirty_inode(ip);
-
-	tp = xfs_trans_alloc(mp, XFS_TRANS_FSYNC_TS);
-	error = xfs_trans_reserve(tp, 0, XFS_FSYNC_TS_LOG_RES(mp), 0, 0, 0);
-	if (error) {
-		xfs_trans_cancel(tp, 0);
-		goto trouble;
-	}
-	xfs_ilock(ip, XFS_ILOCK_EXCL);
-	/*
-	 * Grab all the latest timestamps from the Linux inode.
-	 */
-	ip->i_d.di_atime.t_sec = (__int32_t)inode->i_atime.tv_sec;
-	ip->i_d.di_atime.t_nsec = (__int32_t)inode->i_atime.tv_nsec;
-	ip->i_d.di_ctime.t_sec = (__int32_t)inode->i_ctime.tv_sec;
-	ip->i_d.di_ctime.t_nsec = (__int32_t)inode->i_ctime.tv_nsec;
-	ip->i_d.di_mtime.t_sec = (__int32_t)inode->i_mtime.tv_sec;
-	ip->i_d.di_mtime.t_nsec = (__int32_t)inode->i_mtime.tv_nsec;
-
-	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
-	xfs_trans_log_inode(tp, ip, XFS_ILOG_TIMESTAMP);
-	error = xfs_trans_commit(tp, 0);
-	if (error)
-		goto trouble;
-	return;
-
-trouble:
-	xfs_warn(mp, "failed to update timestamps for inode 0x%llx", ip->i_ino);
-}
-
 STATIC void
 xfs_fs_evict_inode(
 	struct inode		*inode)
@@ -1436,7 +1381,6 @@ xfs_fs_free_cached_objects(
 static const struct super_operations xfs_super_operations = {
 	.alloc_inode		= xfs_fs_alloc_inode,
 	.destroy_inode		= xfs_fs_destroy_inode,
-	.dirty_inode		= xfs_fs_dirty_inode,
 	.evict_inode		= xfs_fs_evict_inode,
 	.drop_inode		= xfs_fs_drop_inode,
 	.put_super		= xfs_fs_put_super,
Index: xfs/fs/xfs/xfs_trace.h
===================================================================
--- xfs.orig/fs/xfs/xfs_trace.h	2012-06-04 07:45:36.000000000 -0400
+++ xfs/fs/xfs/xfs_trace.h	2012-06-05 18:38:32.292064313 -0400
@@ -578,8 +578,8 @@ DEFINE_INODE_EVENT(xfs_ioctl_setattr);
 DEFINE_INODE_EVENT(xfs_dir_fsync);
 DEFINE_INODE_EVENT(xfs_file_fsync);
 DEFINE_INODE_EVENT(xfs_destroy_inode);
-DEFINE_INODE_EVENT(xfs_dirty_inode);
 DEFINE_INODE_EVENT(xfs_evict_inode);
+DEFINE_INODE_EVENT(xfs_update_time);
 
 DEFINE_INODE_EVENT(xfs_dquot_dqalloc);
 DEFINE_INODE_EVENT(xfs_dquot_dqdetach);

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: implement ->update_time

[Christoph Hellwig]

ping?

On Wed, Jun 06, 2012 at 05:01:28PM -0400, Christoph Hellwig wrote:
> Use this new method to replace our hacky use of ->dirty_inode.  An additional
> benefit is that we can now propagate errors up the stack.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> ---
>  fs/xfs/xfs_iops.c  |   45 ++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_super.c |   56 -----------------------------------------------------
>  fs/xfs/xfs_trace.h |    2 -
>  3 files changed, 46 insertions(+), 57 deletions(-)
> 
> Index: xfs/fs/xfs/xfs_iops.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_iops.c	2012-06-04 07:45:36.000000000 -0400
> +++ xfs/fs/xfs/xfs_iops.c	2012-06-05 18:41:09.060068319 -0400
> @@ -897,6 +897,47 @@ xfs_vn_setattr(
>  	return -xfs_setattr_nonsize(XFS_I(dentry->d_inode), iattr, 0);
>  }
>  
> +STATIC int
> +xfs_vn_update_time(
> +	struct inode		*inode,
> +	struct timespec		*now,
> +	int			flags)
> +{
> +	struct xfs_inode	*ip = XFS_I(inode);
> +	struct xfs_mount	*mp = ip->i_mount;
> +	struct xfs_trans	*tp;
> +	int			error;
> +
> +	trace_xfs_update_time(ip);
> +
> +	tp = xfs_trans_alloc(mp, XFS_TRANS_FSYNC_TS);
> +	error = xfs_trans_reserve(tp, 0, XFS_FSYNC_TS_LOG_RES(mp), 0, 0, 0);
> +	if (error) {
> +		xfs_trans_cancel(tp, 0);
> +		return -error;
> +	}
> +
> +	xfs_ilock(ip, XFS_ILOCK_EXCL);
> +	if (flags & S_CTIME) {
> +		inode->i_ctime = *now;
> +		ip->i_d.di_ctime.t_sec = (__int32_t)now->tv_sec;
> +		ip->i_d.di_ctime.t_nsec = (__int32_t)now->tv_nsec;
> +	}
> +	if (flags & S_MTIME) {
> +		inode->i_mtime = *now;
> +		ip->i_d.di_mtime.t_sec = (__int32_t)now->tv_sec;
> +		ip->i_d.di_mtime.t_nsec = (__int32_t)now->tv_nsec;
> +	}
> +	if (flags & S_ATIME) {
> +		inode->i_atime = *now;
> +		ip->i_d.di_atime.t_sec = (__int32_t)now->tv_sec;
> +		ip->i_d.di_atime.t_nsec = (__int32_t)now->tv_nsec;
> +	}
> +	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> +	xfs_trans_log_inode(tp, ip, XFS_ILOG_TIMESTAMP);
> +	return -xfs_trans_commit(tp, 0);
> +}
> +
>  #define XFS_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC|FIEMAP_FLAG_XATTR)
>  
>  /*
> @@ -991,6 +1032,7 @@ static const struct inode_operations xfs
>  	.removexattr		= generic_removexattr,
>  	.listxattr		= xfs_vn_listxattr,
>  	.fiemap			= xfs_vn_fiemap,
> +	.update_time		= xfs_vn_update_time,
>  };
>  
>  static const struct inode_operations xfs_dir_inode_operations = {
> @@ -1016,6 +1058,7 @@ static const struct inode_operations xfs
>  	.getxattr		= generic_getxattr,
>  	.removexattr		= generic_removexattr,
>  	.listxattr		= xfs_vn_listxattr,
> +	.update_time		= xfs_vn_update_time,
>  };
>  
>  static const struct inode_operations xfs_dir_ci_inode_operations = {
> @@ -1041,6 +1084,7 @@ static const struct inode_operations xfs
>  	.getxattr		= generic_getxattr,
>  	.removexattr		= generic_removexattr,
>  	.listxattr		= xfs_vn_listxattr,
> +	.update_time		= xfs_vn_update_time,
>  };
>  
>  static const struct inode_operations xfs_symlink_inode_operations = {
> @@ -1054,6 +1098,7 @@ static const struct inode_operations xfs
>  	.getxattr		= generic_getxattr,
>  	.removexattr		= generic_removexattr,
>  	.listxattr		= xfs_vn_listxattr,
> +	.update_time		= xfs_vn_update_time,
>  };
>  
>  STATIC void
> Index: xfs/fs/xfs/xfs_super.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_super.c	2012-06-04 07:45:36.000000000 -0400
> +++ xfs/fs/xfs/xfs_super.c	2012-06-05 18:35:03.876058962 -0400
> @@ -868,61 +868,6 @@ xfs_fs_inode_init_once(
>  		     "xfsino", ip->i_ino);
>  }
>  
> -/*
> - * This is called by the VFS when dirtying inode metadata.  This can happen
> - * for a few reasons, but we only care about timestamp updates, given that
> - * we handled the rest ourselves.  In theory no other calls should happen,
> - * but for example generic_write_end() keeps dirtying the inode after
> - * updating i_size.  Thus we check that the flags are exactly I_DIRTY_SYNC,
> - * and skip this call otherwise.
> - *
> - * We'll hopefull get a different method just for updating timestamps soon,
> - * at which point this hack can go away, and maybe we'll also get real
> - * error handling here.
> - */
> -STATIC void
> -xfs_fs_dirty_inode(
> -	struct inode		*inode,
> -	int			flags)
> -{
> -	struct xfs_inode	*ip = XFS_I(inode);
> -	struct xfs_mount	*mp = ip->i_mount;
> -	struct xfs_trans	*tp;
> -	int			error;
> -
> -	if (flags != I_DIRTY_SYNC)
> -		return;
> -
> -	trace_xfs_dirty_inode(ip);
> -
> -	tp = xfs_trans_alloc(mp, XFS_TRANS_FSYNC_TS);
> -	error = xfs_trans_reserve(tp, 0, XFS_FSYNC_TS_LOG_RES(mp), 0, 0, 0);
> -	if (error) {
> -		xfs_trans_cancel(tp, 0);
> -		goto trouble;
> -	}
> -	xfs_ilock(ip, XFS_ILOCK_EXCL);
> -	/*
> -	 * Grab all the latest timestamps from the Linux inode.
> -	 */
> -	ip->i_d.di_atime.t_sec = (__int32_t)inode->i_atime.tv_sec;
> -	ip->i_d.di_atime.t_nsec = (__int32_t)inode->i_atime.tv_nsec;
> -	ip->i_d.di_ctime.t_sec = (__int32_t)inode->i_ctime.tv_sec;
> -	ip->i_d.di_ctime.t_nsec = (__int32_t)inode->i_ctime.tv_nsec;
> -	ip->i_d.di_mtime.t_sec = (__int32_t)inode->i_mtime.tv_sec;
> -	ip->i_d.di_mtime.t_nsec = (__int32_t)inode->i_mtime.tv_nsec;
> -
> -	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> -	xfs_trans_log_inode(tp, ip, XFS_ILOG_TIMESTAMP);
> -	error = xfs_trans_commit(tp, 0);
> -	if (error)
> -		goto trouble;
> -	return;
> -
> -trouble:
> -	xfs_warn(mp, "failed to update timestamps for inode 0x%llx", ip->i_ino);
> -}
> -
>  STATIC void
>  xfs_fs_evict_inode(
>  	struct inode		*inode)
> @@ -1436,7 +1381,6 @@ xfs_fs_free_cached_objects(
>  static const struct super_operations xfs_super_operations = {
>  	.alloc_inode		= xfs_fs_alloc_inode,
>  	.destroy_inode		= xfs_fs_destroy_inode,
> -	.dirty_inode		= xfs_fs_dirty_inode,
>  	.evict_inode		= xfs_fs_evict_inode,
>  	.drop_inode		= xfs_fs_drop_inode,
>  	.put_super		= xfs_fs_put_super,
> Index: xfs/fs/xfs/xfs_trace.h
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_trace.h	2012-06-04 07:45:36.000000000 -0400
> +++ xfs/fs/xfs/xfs_trace.h	2012-06-05 18:38:32.292064313 -0400
> @@ -578,8 +578,8 @@ DEFINE_INODE_EVENT(xfs_ioctl_setattr);
>  DEFINE_INODE_EVENT(xfs_dir_fsync);
>  DEFINE_INODE_EVENT(xfs_file_fsync);
>  DEFINE_INODE_EVENT(xfs_destroy_inode);
> -DEFINE_INODE_EVENT(xfs_dirty_inode);
>  DEFINE_INODE_EVENT(xfs_evict_inode);
> +DEFINE_INODE_EVENT(xfs_update_time);
>  
>  DEFINE_INODE_EVENT(xfs_dquot_dqalloc);
>  DEFINE_INODE_EVENT(xfs_dquot_dqdetach);
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: implement ->update_time

[Dave Chinner]

On Wed, Jul 04, 2012 at 02:06:06AM -0400, Christoph Hellwig wrote:
> ping?
> 
> On Wed, Jun 06, 2012 at 05:01:28PM -0400, Christoph Hellwig wrote:
> > Use this new method to replace our hacky use of ->dirty_inode.  An additional
> > benefit is that we can now propagate errors up the stack.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>

I've been testing it and haven't had any problems with it - I just
haven't done a final review of it yet.  I'll try to do it tomorrow
morning....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: implement ->update_time

[Dave Chinner]

On Wed, Jun 06, 2012 at 05:01:28PM -0400, Christoph Hellwig wrote:
> Use this new method to replace our hacky use of ->dirty_inode.  An additional
> benefit is that we can now propagate errors up the stack.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good, all my local qa passes.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs