Re: [PATCH, RFC] Re: atime not written to disk

[Niv Sardi <xaiki@sgi.com>]

[-- Attachment #1: Type: text/plain, Size: 443 bytes --]

Dave Chinner <david@fromorbit.com> writes:
[...]
> As I mentioned on IRC, Tim, the following patch fixes the above test
> case. It will make XFS behave like other filesystems w.r.t. atime,
> instead of defaulting to relatime-like behaviour. This will have
> performance impact unless ppl now add the relatime mount option.
 ^^^^^^^^^^^^^^^^^^^

I don't really like it, and don't think there is a real justification to
do it. Why not only do:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Mark-core-as-dirty-when-atime-updated-needed-for-XFS.patch --]
[-- Type: text/x-diff, Size: 1443 bytes --]

>From 96be907a11f2cad0d6c8696737bb144b1275ce5a Mon Sep 17 00:00:00 2001
From: Niv Sardi <xaiki@debian.org>
Date: Thu, 23 Oct 2008 13:41:09 +1100
Subject: [PATCH] Mark core as dirty when atime updated needed for XFS inode

Currently we're not writting atime updates back to disk on unmount
if it's the only change that happened. One solution would be to implement
->dirty_inode() but that might be overkill and has a perf hit, instead
just tag as needed for update in reclaim()

Signed-off-by: Niv Sardi <xaiki@debian.org>
---
 fs/xfs/xfs_vnodeops.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index e257f65..828d398 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -2840,6 +2840,7 @@ int
 xfs_reclaim(
 	xfs_inode_t	*ip)
 {
+	struct inode	*inode = VFS_I(ip);
 
 	xfs_itrace_entry(ip);
 
@@ -2856,10 +2857,13 @@ xfs_reclaim(
 	ASSERT(XFS_FORCED_SHUTDOWN(ip->i_mount) || ip->i_delayed_blks == 0);
 
 	/*
-	 * Make sure the atime in the XFS inode is correct before freeing the
-	 * Linux inode.
+	 * Mark dirty and update atime only if different from the linux inode one.
 	 */
-	xfs_synchronize_atime(ip);
+	if (! timespec_equal(&inode->i_atime,
+			     (struct timespec *) &ip->i_d.di_atime)) {
+		xfs_synchronize_atime(ip);
+		ip->i_update_core = 1;
+	}
 
 	/*
 	 * If we have nothing to flush with this inode then complete the
-- 
1.5.6.2


[-- Attachment #3: Type: text/plain, Size: 15 bytes --]


-- 
Niv Sardi