On Wed, Feb 29, 2012 at 04:53:52AM -0500, Christoph Hellwig wrote:
> Timestamps on regular files are the last metadata that XFS does not update
> transactionally. Now that we use the delaylog mode exclusively and made
> the log scode scale extremly well there is no need to bypass that code for
> timestamp updates. Logging all updates allows to drop a lot of code, and
> will allow for further performance improvements later on.
>
> Note that this patch drops optimized handling of fdatasync - it will be
> added back in a separate commit.
>
> Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
...
> Index: xfs/fs/xfs/xfs_trans_inode.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_trans_inode.c 2012-02-13 13:48:02.127012825 -0800
> +++ xfs/fs/xfs/xfs_trans_inode.c 2012-02-19 13:37:57.829955809 -0800
> @@ -95,10 +95,14 @@ xfs_trans_ichgtime(
> if ((flags & XFS_ICHGTIME_MOD) &&
> !timespec_equal(&inode->i_mtime, &tv)) {
> inode->i_mtime = tv;
> + ip->i_d.di_mtime.t_sec = tv.tv_sec;
> + ip->i_d.di_mtime.t_nsec = tv.tv_nsec;
> }
> if ((flags & XFS_ICHGTIME_CHG) &&
> !timespec_equal(&inode->i_ctime, &tv)) {
> inode->i_ctime = tv;
> + ip->i_d.di_ctime.t_sec = tv.tv_sec;
> + ip->i_d.di_ctime.t_nsec = tv.tv_nsec;
> }
> }
So... you will always want to log the inode after calling
xfs_trans_ichgtime. FWICS this is done in all cases except for
xfs_qm_scall_trunc_qfile. Maybe it doesn't matter in that case.
...
> STATIC void
> xfs_fs_dirty_inode(
> - struct inode *inode,
> - int flags)
> -{
> - barrier();
> - XFS_I(inode)->i_update_core = 1;
> -}
> -
> -STATIC int
> -xfs_fs_write_inode(
> struct inode *inode,
> - struct writeback_control *wbc)
> + int flags)
> {
> struct xfs_inode *ip = XFS_I(inode);
> struct xfs_mount *mp = ip->i_mount;
> - int error = EAGAIN;
> -
> - trace_xfs_write_inode(ip);
> -
> - if (XFS_FORCED_SHUTDOWN(mp))
> - return -XFS_ERROR(EIO);
> -
> - if (wbc->sync_mode == WB_SYNC_ALL || wbc->for_kupdate) {
> - /*
> - * Make sure the inode has made it it into the log. Instead
> - * of forcing it all the way to stable storage using a
> - * synchronous transaction we let the log force inside the
> - * ->sync_fs call do that for thus, which reduces the number
> - * of synchronous log forces dramatically.
> - */
> - error = xfs_log_dirty_inode(ip, NULL, 0);
> - if (error)
> - goto out;
> - return 0;
> - } else {
> - if (!ip->i_update_core)
> - return 0;
> -
> - /*
> - * We make this non-blocking if the inode is contended, return
> - * EAGAIN to indicate to the caller that they did not succeed.
> - * This prevents the flush path from blocking on inodes inside
> - * another operation right now, they get caught later by
> - * xfs_sync.
> - */
> - if (!xfs_ilock_nowait(ip, XFS_ILOCK_SHARED))
> - goto out;
> + struct xfs_trans *tp;
> + int error;
>
> - if (xfs_ipincount(ip) || !xfs_iflock_nowait(ip))
> - goto out_unlock;
> + if (flags != I_DIRTY_SYNC)
> + return;
Ok... so this gets you updates from touch_atime and file_update_time.
> @@ -385,16 +359,6 @@ xfs_quiesce_data(
> {
> int error, error2 = 0;
>
> - /*
> - * Log all pending size and timestamp updates. The vfs writeback
> - * code is supposed to do this, but due to its overagressive
> - * livelock detection it will skip inodes where appending writes
> - * were written out in the first non-blocking sync phase if their
> - * completion took long enough that it happened after taking the
> - * timestamp for the cut-off in the blocking phase.
> - */
> - xfs_inode_ag_iterator(mp, xfs_log_dirty_inode, 0);
> -
You just put that in there!
Looks good!
Reviewed-by: Ben Myers <bpm@xxxxxxx>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
