Commit f9581b14 authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Alex Elder
Browse files

xfs: implement ->dirty_inode to fix timestamp handling 



This is picking up on Felix's repost of Dave's patch to implement a
.dirty_inode method.  We really need this notification because
the VFS keeps writing directly into the inode structure instead
of going through methods to update this state.  In addition to
the long-known atime issue we now also have a caller in VM code
that updates c/mtime that way for shared writeable mmaps.  And
I found another one that no one has noticed in practice in the FIFO
code.

So implement ->dirty_inode to set i_update_core whenever the
inode gets externally dirtied, and switch the c/mtime handling to
the same scheme we already use for atime (always picking up
the value from the Linux inode).

Note that this patch also removes the xfs_synchronize_atime call
in xfs_reclaim it was superflous as we already synchronize the time
when writing the inode via the log (xfs_inode_item_format) or the
normal buffers (xfs_iflush_int).

In addition also remove the I_CLEAR check before copying the Linux
timestamps - now that we always have the Linux inode available
we can always use the timestamps in it.

Also switch to just using file_update_time for regular reads/writes -
that will get us all optimization done to it for free and make
sure we notice early when it breaks.

Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarFelix Blyakher <felixb@sgi.com>
Reviewed-by: default avatarAlex Elder <aelder@sgi.com>
Signed-off-by: default avatarAlex Elder <aelder@sgi.com>
parent 9ef96da6

fs/xfs/linux-2.6/xfs_aops.c

+0 −1
Original line number Diff line number Diff line
@@ -215,7 +215,6 @@ xfs_setfilesize( 


	if (ip->i_d.di_size < isize) {

		ip->i_d.di_size = isize;

		ip->i_update_core = 1;

		xfs_mark_inode_dirty_sync(ip);

	}

fs/xfs/linux-2.6/xfs_iops.c

+15 −26
Original line number Diff line number Diff line
@@ -57,19 +57,22 @@ 
#include <linux/fiemap.h>



/*

 * Bring the atime in the XFS inode uptodate.

 * Used before logging the inode to disk or when the Linux inode goes away.

 * Bring the timestamps in the XFS inode uptodate.

 *

 * Used before writing the inode to disk.

 */

void

xfs_synchronize_atime(

xfs_synchronize_times(

	xfs_inode_t	*ip)

{

	struct inode	*inode = VFS_I(ip);



	if (!(inode->i_state & I_CLEAR)) {

	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;

}



/*
@@ -106,33 +109,21 @@ xfs_ichgtime( 
	if ((flags & XFS_ICHGTIME_MOD) &&

	    !timespec_equal(&inode->i_mtime, &tv)) {

		inode->i_mtime = tv;

		ip->i_d.di_mtime.t_sec = (__int32_t)tv.tv_sec;

		ip->i_d.di_mtime.t_nsec = (__int32_t)tv.tv_nsec;

		sync_it = 1;

	}

	if ((flags & XFS_ICHGTIME_CHG) &&

	    !timespec_equal(&inode->i_ctime, &tv)) {

		inode->i_ctime = tv;

		ip->i_d.di_ctime.t_sec = (__int32_t)tv.tv_sec;

		ip->i_d.di_ctime.t_nsec = (__int32_t)tv.tv_nsec;

		sync_it = 1;

	}



	/*

	 * We update the i_update_core field _after_ changing

	 * the timestamps in order to coordinate properly with

	 * xfs_iflush() so that we don't lose timestamp updates.

	 * This keeps us from having to hold the inode lock

	 * while doing this.  We use the SYNCHRONIZE macro to

	 * ensure that the compiler does not reorder the update

	 * of i_update_core above the timestamp updates above.

	 * Update complete - now make sure everyone knows that the inode

	 * is dirty.

	 */

	if (sync_it) {

		SYNCHRONIZE();

		ip->i_update_core = 1;

	if (sync_it)

		xfs_mark_inode_dirty_sync(ip);

}

}



/*

 * Hook in SELinux.  This is not quite correct yet, what we really need
@@ -514,10 +505,8 @@ xfs_vn_getattr( 
	stat->gid = ip->i_d.di_gid;

	stat->ino = ip->i_ino;

	stat->atime = inode->i_atime;

	stat->mtime.tv_sec = ip->i_d.di_mtime.t_sec;

	stat->mtime.tv_nsec = ip->i_d.di_mtime.t_nsec;

	stat->ctime.tv_sec = ip->i_d.di_ctime.t_sec;

	stat->ctime.tv_nsec = ip->i_d.di_ctime.t_nsec;

	stat->mtime = inode->i_mtime;

	stat->ctime = inode->i_ctime;

	stat->blocks =

		XFS_FSB_TO_BB(mp, ip->i_d.di_nblocks + ip->i_delayed_blks);

fs/xfs/linux-2.6/xfs_lrw.c

+1 −1
Original line number Diff line number Diff line
@@ -667,7 +667,7 @@ start: 
		xip->i_new_size = new_size;



	if (likely(!(ioflags & IO_INVIS)))

		xfs_ichgtime(xip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);

		file_update_time(file);



	/*

	 * If the offset is beyond the size of the file, we have a couple

fs/xfs/linux-2.6/xfs_super.c

+23 −0
Original line number Diff line number Diff line
@@ -976,6 +976,28 @@ xfs_fs_inode_init_once( 
	mrlock_init(&ip->i_iolock, MRLOCK_BARRIER, "xfsio", ip->i_ino);

}



/*

 * Dirty the XFS inode when mark_inode_dirty_sync() is called so that

 * we catch unlogged VFS level updates to the inode. Care must be taken

 * here - the transaction code calls mark_inode_dirty_sync() to mark the

 * VFS inode dirty in a transaction and clears the i_update_core field;

 * it must clear the field after calling mark_inode_dirty_sync() to

 * correctly indicate that the dirty state has been propagated into the

 * inode log item.

 *

 * We need the barrier() to maintain correct ordering between unlogged

 * updates and the transaction commit code that clears the i_update_core

 * field. This requires all updates to be completed before marking the

 * inode dirty.

 */

STATIC void

xfs_fs_dirty_inode(

	struct inode	*inode)

{

	barrier();

	XFS_I(inode)->i_update_core = 1;

}



/*

 * Attempt to flush the inode, this will actually fail

 * if the inode is pinned, but we dirty the inode again
@@ -1539,6 +1561,7 @@ xfs_fs_get_sb( 
static struct super_operations xfs_super_operations = {

	.alloc_inode		= xfs_fs_alloc_inode,

	.destroy_inode		= xfs_fs_destroy_inode,

	.dirty_inode		= xfs_fs_dirty_inode,

	.write_inode		= xfs_fs_write_inode,

	.clear_inode		= xfs_fs_clear_inode,

	.put_super		= xfs_fs_put_super,

fs/xfs/xfs_dfrag.c

+4 −4
Original line number Diff line number Diff line
@@ -206,10 +206,10 @@ xfs_swap_extents( 
	 * process that the file was not changed out from

	 * under it.

	 */

	if ((sbp->bs_ctime.tv_sec != ip->i_d.di_ctime.t_sec) ||

	    (sbp->bs_ctime.tv_nsec != ip->i_d.di_ctime.t_nsec) ||

	    (sbp->bs_mtime.tv_sec != ip->i_d.di_mtime.t_sec) ||

	    (sbp->bs_mtime.tv_nsec != ip->i_d.di_mtime.t_nsec)) {

	if ((sbp->bs_ctime.tv_sec != VFS_I(ip)->i_ctime.tv_sec) ||

	    (sbp->bs_ctime.tv_nsec != VFS_I(ip)->i_ctime.tv_nsec) ||

	    (sbp->bs_mtime.tv_sec != VFS_I(ip)->i_mtime.tv_sec) ||

	    (sbp->bs_mtime.tv_nsec != VFS_I(ip)->i_mtime.tv_nsec)) {

		error = XFS_ERROR(EBUSY);

		goto out_unlock;

	}

CRA Git | Maintained and supported by SUSTech CRA and CCSE