xfs
[Top] [All Lists]

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

To: Utako Kusaka <u-kusaka@xxxxxxxxxxxxx>
Subject: Re: [PATCH, RFC] Re: atime not written to disk
From: Niv Sardi <xaiki@xxxxxxx>
Date: Thu, 23 Oct 2008 13:53:23 +1100
Cc: Timothy Shimmin <tes@xxxxxxx>, xfs <xfs@xxxxxxxxxxx>
In-reply-to: <20081022081710.GL18495@disturbed> (Dave Chinner's message of "Wed, 22 Oct 2008 19:17:10 +1100")
References: <48FD74CC.907@xxxxxxx> <48FD7B69.3090600@xxxxxxxxxxxxx> <20081022081710.GL18495@disturbed>
User-agent: Gnus/5.110011 (No Gnus v0.11) Emacs/23.0.60 (x86_64-pc-linux-gnu)
Dave Chinner <david@xxxxxxxxxxxxx> 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:

>From 96be907a11f2cad0d6c8696737bb144b1275ce5a Mon Sep 17 00:00:00 2001
From: Niv Sardi <xaiki@xxxxxxxxxx>
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@xxxxxxxxxx>
---
 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

-- 
Niv Sardi
<Prev in Thread] Current Thread [Next in Thread>