Re: [PATCH 5/8] xfs: log timestamp updates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]    [Yosemite Photos]    [Free Online Dating]     [Linux Kernel]     [Linux SCSI]     [XFree86]

Add to Google Powered by Linux