optimize attribute timeouts for "noac" and "actimeo=0"

Hi.

I've been looking at a bugzilla which describes a problem where
a customer was advised to use either the "noac" or "actimeo=0"
mount options to solve a consistency problem that they were
seeing in the file attributes.  It turned out that this solution
did not work reliably for them because sometimes, the local
attribute cache was believed to be valid and not timed out.
(With an attribute cache timeout of 0, the cache should always
appear to be timed out.)

In looking at this situation, it appears to me that the problem
is that the attribute cache timeout code has an off-by-one
error in it.  It is assuming that the cache is valid in the
region, [read_cache_jiffies, read_cache_jiffies + attrtimeo].  The
cache should be considered valid only in the region,
[read_cache_jiffies, read_cache_jiffies + attrtimeo).  With this
change, the options, "noac" and "actimeo=0", work as originally
expected.

This problem was previously addressed by special casing the
attrtimeo == 0 case.  However, since the problem is only an off-
by-one error, the cleaner solution is address the off-by-one
error and thus, not require the special case.

    Thanx...

        ps

Signed-off-by: Peter Staubach <staubach@redhat.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index ed7024c348850adb826aa7d9c0318e91cb7a0fb1..e35c8199f82f108daade505679e6eee22ef3ec34 100644 (file)
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1798,7 +1798,7 @@ static int nfs_access_get_cached(struct inode *inode, struct rpc_cred *cred, str
        if (cache == NULL)
                goto out;
        if (!nfs_have_delegation(inode, FMODE_READ) &&
-           !time_in_range(jiffies, cache->jiffies, cache->jiffies + nfsi->attrtimeo))
+           !time_in_range_open(jiffies, cache->jiffies, cache->jiffies + nfsi->attrtimeo))
                goto out_stale;
        res->jiffies = cache->jiffies;
        res->cred = cache->cred;

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 1cf39202c3ead6f99931a66204264687ee27020e..0c381686171e30cc1eab1f71128bce7837bd22dd 100644 (file)
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -712,14 +712,7 @@ int nfs_attribute_timeout(struct inode *inode)
 
        if (nfs_have_delegation(inode, FMODE_READ))
                return 0;
-       /*
-        * Special case: if the attribute timeout is set to 0, then always
-        *               treat the cache as having expired (unless holding
-        *               a delegation).
-        */
-       if (nfsi->attrtimeo == 0)
-               return 1;
-       return !time_in_range(jiffies, nfsi->read_cache_jiffies, nfsi->read_cache_jiffies + nfsi->attrtimeo);
+       return !time_in_range_open(jiffies, nfsi->read_cache_jiffies, nfsi->read_cache_jiffies + nfsi->attrtimeo);
 }
 
 /**
@@ -1182,7 +1175,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
                nfsi->attrtimeo_timestamp = now;
                nfsi->attr_gencount = nfs_inc_attr_generation_counter();
        } else {
-               if (!time_in_range(now, nfsi->attrtimeo_timestamp, nfsi->attrtimeo_timestamp + nfsi->attrtimeo)) {
+               if (!time_in_range_open(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 abb6ac639e8e19740c8bbd30148db72a1028d031..1a9cf78bfce545fe2495fa5fe248297fc038d6ce 100644 (file)
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -115,10 +115,20 @@ static inline u64 get_jiffies_64(void)
         ((long)(a) - (long)(b) >= 0))
 #define time_before_eq(a,b)    time_after_eq(b,a)
 
+/*
+ * Calculate whether a is in the range of [b, c].
+ */
 #define time_in_range(a,b,c) \
        (time_after_eq(a,b) && \
         time_before_eq(a,c))
 
+/*
+ * Calculate whether a is in the range of [b, c).
+ */
+#define time_in_range_open(a,b,c) \
+       (time_after_eq(a,b) && \
+        time_before(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() */

diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index b8d9c6dd4f63ea0c89eac01f824e65341b94e5f8..db867b04ac3c21b66e39793149e121b4569ee41f 100644 (file)
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -130,7 +130,10 @@ struct nfs_inode {
         *
         * We need to revalidate the cached attrs for this inode if
         *
-        *      jiffies - read_cache_jiffies > attrtimeo
+        *      jiffies - read_cache_jiffies >= attrtimeo
+        *
+        * Please note the comparison is greater than or equal
+        * so that zero timeout values can be specified.
         */
        unsigned long           read_cache_jiffies;
        unsigned long           attrtimeo;

diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index 6e28744b1709715a9fda7a668b340b3f8c6bc49c..050e4e84d9e3c44a562618ca30d7a3eeb5e86535 100644 (file)
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -234,7 +234,7 @@ rpcauth_prune_expired(struct list_head *free, int nr_to_scan)
        list_for_each_entry_safe(cred, next, &cred_unused, cr_lru) {
 
                /* Enforce a 60 second garbage collection moratorium */
-               if (time_in_range(cred->cr_expire, expired, jiffies) &&
+               if (time_in_range_open(cred->cr_expire, expired, jiffies) &&
                    test_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags) != 0)
                        continue;