Gitweb:     
http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=c7e15961115028b99f6142266b5fb08acca0e8dd
Commit:     c7e15961115028b99f6142266b5fb08acca0e8dd
Parent:     4e769b934e7638038e232c05b64f644e7269a90f
Author:     Fabio Olive Leite <[EMAIL PROTECTED]>
AuthorDate: Thu Jul 26 22:59:00 2007 -0300
Committer:  Trond Myklebust <[EMAIL PROTECTED]>
CommitDate: Tue Oct 9 17:15:33 2007 -0400

    Re: [NFS] [PATCH] Attribute timeout handling and wrapping u32 jiffies
    
    I would like to discuss the idea that the current checks for attribute
    timeout using time_after are inadequate for 32bit architectures, since
    time_after works correctly only when the two timestamps being compared
    are within 2^31 jiffies of each other. The signed overflow caused by
    comparing values more than 2^31 jiffies apart will flip the result,
    causing incorrect assumptions of validity.
    
    2^31 jiffies is a fairly large period of time (~25 days) when compared
    to the lifetime of most kernel data structures, but for long lived NFS
    mounts that can sit idle for months (think that for some reason autofs
    cannot be used), it is easy to compare inode attribute timestamps with
    very disparate or even bogus values (as in when jiffies have wrapped
    many times, where the comparison doesn't even make sense).
    
    Currently the code tests for attribute timeout by simply adding the
    desired amount of jiffies to the stored timestamp and comparing that
    with the current timestamp of obtained attribute data with time_after.
    This is incorrect, as it returns true for the desired timeout period
    and another full 2^31 range of jiffies.
    
    In testing with artificial jumps (several small jumps, not one big
    crank) of the jiffies I was able to reproduce a problem found in a
    server with very long lived NFS mounts, where attributes would not be
    refreshed even after touching files and directories in the server:
    
    Initial uptime:
    03:42:01 up 6 min, 0 users, load average: 0.01, 0.12, 0.07
    
    NFS volume is mounted and time is advanced:
    03:38:09 up 25 days, 2 min, 0 users, load average: 1.22, 1.05, 1.08
    
    # ls -l /local/A/foo/bar /nfs/A/foo/bar
    -rw-r--r--  1 root root 0 Dec 17 03:38 /local/A/foo/bar
    -rw-r--r--  1 root root 0 Nov 22 00:36 /nfs/A/foo/bar
    
    # touch /local/A/foo/bar
    
    # ls -l /local/A/foo/bar /nfs/A/foo/bar
    -rw-r--r--  1 root root 0 Dec 17 03:47 /local/A/foo/bar
    -rw-r--r--  1 root root 0 Nov 22 00:36 /nfs/A/foo/bar
    
    We can see the local mtime is updated, but the NFS mount still shows
    the old value. The patch below makes it work:
    
    Initial setup...
    07:11:02 up 25 days, 1 min,  0 users,  load average: 0.15, 0.03, 0.04
    
    # ls -l /local/A/foo/bar /nfs/A/foo/bar
    -rw-r--r--  1 root root 0 Jan 11 07:11 /local/A/foo/bar
    -rw-r--r--  1 root root 0 Jan 11 07:11 /nfs/A/foo/bar
    
    # touch /local/A/foo/bar
    
    # ls -l /local/A/foo/bar /nfs/A/foo/bar
    -rw-r--r--  1 root root 0 Jan 11 07:14 /local/A/foo/bar
    -rw-r--r--  1 root root 0 Jan 11 07:14 /nfs/A/foo/bar
    
    Signed-off-by: Fabio Olive Leite <[EMAIL PROTECTED]>
    Signed-off-by: Trond Myklebust <[EMAIL PROTECTED]>
---
 fs/nfs/dir.c            |    2 +-
 fs/nfs/inode.c          |    4 ++--
 include/linux/jiffies.h |    4 ++++
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index dd02db4..6e0aa04 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1855,7 +1855,7 @@ int nfs_access_get_cached(struct inode *inode, struct 
rpc_cred *cred, struct nfs
        cache = nfs_access_search_rbtree(inode, cred);
        if (cache == NULL)
                goto out;
-       if (time_after(jiffies, cache->jiffies + NFS_ATTRTIMEO(inode)))
+       if (!time_in_range(jiffies, cache->jiffies, cache->jiffies + 
NFS_ATTRTIMEO(inode)))
                goto out_stale;
        res->jiffies = cache->jiffies;
        res->cred = cache->cred;
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 3ad938c..7c8ca17 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -656,7 +656,7 @@ int nfs_attribute_timeout(struct inode *inode)
 
        if (nfs_have_delegation(inode, FMODE_READ))
                return 0;
-       return time_after(jiffies, nfsi->read_cache_jiffies+nfsi->attrtimeo);
+       return !time_in_range(jiffies, nfsi->read_cache_jiffies, 
nfsi->read_cache_jiffies + nfsi->attrtimeo);
 }
 
 /**
@@ -1055,7 +1055,7 @@ static int nfs_update_inode(struct inode *inode, struct 
nfs_fattr *fattr)
                nfs_inc_stats(inode, NFSIOS_ATTRINVALIDATE);
                nfsi->attrtimeo = NFS_MINATTRTIMEO(inode);
                nfsi->attrtimeo_timestamp = now;
-       } else if (time_after(now, nfsi->attrtimeo_timestamp+nfsi->attrtimeo)) {
+       } else if (!time_in_range(now, nfsi->attrtimeo_timestamp, 
nfsi->attrtimeo_timestamp + nfsi->attrtimeo)) {
                if ((nfsi->attrtimeo <<= 1) > NFS_MAXATTRTIMEO(inode))
                        nfsi->attrtimeo = NFS_MAXATTRTIMEO(inode);
                nfsi->attrtimeo_timestamp = now;
diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
index c080f61..f1c87ad 100644
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -115,6 +115,10 @@ static inline u64 get_jiffies_64(void)
         ((long)(a) - (long)(b) >= 0))
 #define time_before_eq(a,b)    time_after_eq(b,a)
 
+#define time_in_range(a,b,c) \
+       (time_after_eq(a,b) && \
+        time_before_eq(a,c))
+
 /* Same as above, but does so with platform independent 64bit types.
  * These must be used when utilizing jiffies_64 (i.e. return value of
  * get_jiffies_64() */
-
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to