[PATCH 0/2] sunrpc: use monotonic time for expiry times

[PATCH 0/2] sunrpc: use monotonic time for expiry times

[NeilBrown]

I've always known I would pay for this some day ....

The authentication cache uses wall-clock time for expiry times and
related time stamps.

This can cause problems if the wall-clock time changes suddenly.
e.g. if you set the time forward one hour, the export cache is
immediately flushed.  This isn't an enormous problem, but it should be
fixed.

Currently, it can cause clients to have to resend requests which can
cause a delay of several timeouts.  Once we get the improvements in
which allow a thread to wait for a while rather than dropping a
request, this will be less of an issue.  But I think it is worth
fixing anyway.

So: two patches.  One moves come common code to an inline, the other
makes the change.

comments?

Thanks,
NeilBrown

---

NeilBrown (2):
      sunrpc: extract some common sunrpc_cache code from nfsd
      sunrpc: use monotonic time in expiry cache


 fs/nfs/dns_resolve.c         |    6 +++---
 fs/nfsd/export.c             |    9 +++------
 fs/nfsd/nfs4idmap.c          |    2 +-
 include/linux/sunrpc/cache.h |   23 ++++++++++++++++++++++-
 net/sunrpc/cache.c           |   33 ++++++++++++++++++---------------
 5 files changed, 47 insertions(+), 26 deletions(-)

[PATCH 1/2] sunrpc: extract some common sunrpc_cache code from nfsd

[NeilBrown]

Rather can duplicating this idiom twice, put it in an inline function.
This reduces the usage of 'expiry_time' out side the sunrpc/cache.c
code and thus the impact of a change that is about to be made to that
field.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 fs/nfsd/export.c             |    9 +++------
 include/linux/sunrpc/cache.h |    6 ++++++
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index a0c4016..d548da4 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -928,10 +928,9 @@ static void exp_fsid_unhash(struct svc_export *exp)
 
 	ek = exp_get_fsid_key(exp->ex_client, exp->ex_fsid);
 	if (!IS_ERR(ek)) {
-		ek->h.expiry_time = get_seconds()-1;
+		sunrpc_invalidate(&ek->h, &svc_expkey_cache);
 		cache_put(&ek->h, &svc_expkey_cache);
 	}
-	svc_expkey_cache.nextcheck = get_seconds();
 }
 
 static int exp_fsid_hash(svc_client *clp, struct svc_export *exp)
@@ -966,10 +965,9 @@ static void exp_unhash(struct svc_export *exp)
 
 	ek = exp_get_key(exp->ex_client, inode->i_sb->s_dev, inode->i_ino);
 	if (!IS_ERR(ek)) {
-		ek->h.expiry_time = get_seconds()-1;
+		sunrpc_invalidate(&ek->h, &svc_expkey_cache);
 		cache_put(&ek->h, &svc_expkey_cache);
 	}
-	svc_expkey_cache.nextcheck = get_seconds();
 }
 	
 /*
@@ -1090,8 +1088,7 @@ out:
 static void
 exp_do_unexport(svc_export *unexp)
 {
-	unexp->h.expiry_time = get_seconds()-1;
-	svc_export_cache.nextcheck = get_seconds();
+	sunrpc_invalidate(&unexp->h, &svc_export_cache);
 	exp_unhash(unexp);
 	exp_fsid_unhash(unexp);
 }
diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index 6f52b4d..e41ee6d 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -227,4 +227,10 @@ static inline time_t get_expiry(char **bpp)
 	return rv;
 }
 
+static inline void sunrpc_invalidate(struct cache_head *h,
+				     struct cache_detail *detail)
+{
+	h->expiry_time = get_seconds() - 1;
+	detail->nextcheck = get_seconds();
+}
 #endif /*  _LINUX_SUNRPC_CACHE_H_ */

[PATCH 2/2] sunrpc: use monotonic time in expiry cache

[NeilBrown]

this protects us from confusion when the wallclock time changes.

We convert to and from wallclock when  setting or reading expiry
times.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 fs/nfs/dns_resolve.c         |    6 +++---
 fs/nfsd/nfs4idmap.c          |    2 +-
 include/linux/sunrpc/cache.h |   21 ++++++++++++++++++---
 net/sunrpc/cache.c           |   33 ++++++++++++++++++---------------
 4 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c
index 95e1ca7..412e6a0 100644
--- a/fs/nfs/dns_resolve.c
+++ b/fs/nfs/dns_resolve.c
@@ -131,7 +131,7 @@ static int nfs_dns_show(struct seq_file *m, struct cache_detail *cd,
 		return 0;
 	}
 	item = container_of(h, struct nfs_dns_ent, h);
-	ttl = (long)item->h.expiry_time - (long)get_seconds();
+	ttl = (long)item->h.expiry_time - (long)monotonic_seconds();
 	if (ttl < 0)
 		ttl = 0;
 
@@ -203,7 +203,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen)
 	ttl = get_expiry(&buf);
 	if (ttl == 0)
 		goto out;
-	key.h.expiry_time = ttl + get_seconds();
+	key.h.expiry_time = ttl + monotonic_seconds();
 
 	ret = -ENOMEM;
 	item = nfs_dns_lookup(cd, &key);
@@ -265,7 +265,7 @@ static int do_cache_lookup_nowait(struct cache_detail *cd,
 		goto out_err;
 	ret = -ETIMEDOUT;
 	if (!test_bit(CACHE_VALID, &(*item)->h.flags)
-			|| (*item)->h.expiry_time < get_seconds()
+			|| (*item)->h.expiry_time < monotonic_seconds()
 			|| cd->flush_time > (*item)->h.last_refresh)
 		goto out_put;
 	ret = -ENOENT;
diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index 6e2983b..8f5cc77 100644
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -549,7 +549,7 @@ do_idmap_lookup_nowait(struct ent *(*lookup_fn)(struct ent *),
 		goto out_err;
 	ret = -ETIMEDOUT;
 	if (!test_bit(CACHE_VALID, &(*item)->h.flags)
-			|| (*item)->h.expiry_time < get_seconds()
+			|| (*item)->h.expiry_time < monotonic_seconds()
 			|| detail->flush_time > (*item)->h.last_refresh)
 		goto out_put;
 	ret = -ENOENT;
diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index e41ee6d..153aae8 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -217,20 +217,35 @@ static inline int get_int(char **bpp, int *anint)
 	return 0;
 }
 
+/*
+ * timestamps kept in the cache are expressed in seconds
+ * since boot.  This is the best for measuring differences in
+ * real time.
+ */
+static inline unsigned long monotonic_seconds(void)
+{
+	struct timespec boot;
+	getboottime(&boot);
+	return get_seconds() - boot.tv_sec;
+}
+
 static inline time_t get_expiry(char **bpp)
 {
 	int rv;
+	struct timespec boot;
+
 	if (get_int(bpp, &rv))
 		return 0;
 	if (rv < 0)
 		return 0;
-	return rv;
+	getboottime(&boot);
+	return rv - boot.tv_sec;
 }
 
 static inline void sunrpc_invalidate(struct cache_head *h,
 				     struct cache_detail *detail)
 {
-	h->expiry_time = get_seconds() - 1;
-	detail->nextcheck = get_seconds();
+	h->expiry_time = monotonic_seconds() - 1;
+	detail->nextcheck = monotonic_seconds();
 }
 #endif /*  _LINUX_SUNRPC_CACHE_H_ */
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 39bddba..379e2bf 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -41,7 +41,7 @@ static void cache_revisit_request(struct cache_head *item);
 
 static void cache_init(struct cache_head *h)
 {
-	time_t now = get_seconds();
+	time_t now = monotonic_seconds();
 	h->next = NULL;
 	h->flags = 0;
 	kref_init(&h->ref);
@@ -108,7 +108,7 @@ static void cache_dequeue(struct cache_detail *detail, struct cache_head *ch);
 static void cache_fresh_locked(struct cache_head *head, time_t expiry)
 {
 	head->expiry_time = expiry;
-	head->last_refresh = get_seconds();
+	head->last_refresh = monotonic_seconds();
 	set_bit(CACHE_VALID, &head->flags);
 }
 
@@ -184,7 +184,7 @@ static int cache_make_upcall(struct cache_detail *cd, struct cache_head *h)
 static inline int cache_is_valid(struct cache_detail *detail, struct cache_head *h)
 {
 	if (!test_bit(CACHE_VALID, &h->flags) ||
-	    h->expiry_time < get_seconds())
+	    h->expiry_time < monotonic_seconds())
 		return -EAGAIN;
 	else if (detail->flush_time > h->last_refresh)
 		return -EAGAIN;
@@ -222,7 +222,7 @@ int cache_check(struct cache_detail *detail,
 
 	/* now see if we want to start an upcall */
 	refresh_age = (h->expiry_time - h->last_refresh);
-	age = get_seconds() - h->last_refresh;
+	age = monotonic_seconds() - h->last_refresh;
 
 	if (rqstp == NULL) {
 		if (rv == -EAGAIN)
@@ -372,11 +372,11 @@ static int cache_clean(void)
 			return -1;
 		}
 		current_detail = list_entry(next, struct cache_detail, others);
-		if (current_detail->nextcheck > get_seconds())
+		if (current_detail->nextcheck > monotonic_seconds())
 			current_index = current_detail->hash_size;
 		else {
 			current_index = 0;
-			current_detail->nextcheck = get_seconds()+30*60;
+			current_detail->nextcheck = monotonic_seconds()+30*60;
 		}
 	}
 
@@ -401,7 +401,7 @@ static int cache_clean(void)
 		for (; ch; cp= & ch->next, ch= *cp) {
 			if (current_detail->nextcheck > ch->expiry_time)
 				current_detail->nextcheck = ch->expiry_time+1;
-			if (ch->expiry_time >= get_seconds() &&
+			if (ch->expiry_time >= monotonic_seconds() &&
 			    ch->last_refresh >= current_detail->flush_time)
 				continue;
 			if (test_and_clear_bit(CACHE_PENDING, &ch->flags))
@@ -465,7 +465,7 @@ EXPORT_SYMBOL_GPL(cache_flush);
 void cache_purge(struct cache_detail *detail)
 {
 	detail->flush_time = LONG_MAX;
-	detail->nextcheck = get_seconds();
+	detail->nextcheck = monotonic_seconds();
 	cache_flush();
 	detail->flush_time = 1;
 }
@@ -1207,7 +1207,8 @@ static int c_show(struct seq_file *m, void *p)
 
 	ifdebug(CACHE)
 		seq_printf(m, "# expiry=%ld refcnt=%d flags=%lx\n",
-			   cp->expiry_time, atomic_read(&cp->ref.refcount), cp->flags);
+			   cp->expiry_time - monotonic_seconds() + get_seconds(),
+			   atomic_read(&cp->ref.refcount), cp->flags);
 	cache_get(cp);
 	if (cache_check(cd, cp, NULL))
 		/* cache_check does a cache_put on failure */
@@ -1271,7 +1272,8 @@ static ssize_t read_flush(struct file *file, char __user *buf,
 	unsigned long p = *ppos;
 	size_t len;
 
-	sprintf(tbuf, "%lu\n", cd->flush_time);
+	sprintf(tbuf, "%lu\n", (cd->flush_time - monotonic_seconds()
+				+ get_seconds()));
 	len = strlen(tbuf);
 	if (p >= len)
 		return 0;
@@ -1289,19 +1291,20 @@ static ssize_t write_flush(struct file *file, const char __user *buf,
 			   struct cache_detail *cd)
 {
 	char tbuf[20];
-	char *ep;
-	long flushtime;
+	char *bp, *ep;
+
 	if (*ppos || count > sizeof(tbuf)-1)
 		return -EINVAL;
 	if (copy_from_user(tbuf, buf, count))
 		return -EFAULT;
 	tbuf[count] = 0;
-	flushtime = simple_strtoul(tbuf, &ep, 0);
+	simple_strtoul(tbuf, &ep, 0);
 	if (*ep && *ep != '\n')
 		return -EINVAL;
 
-	cd->flush_time = flushtime;
-	cd->nextcheck = get_seconds();
+	bp = tbuf;
+	cd->flush_time = get_expiry(&bp);
+	cd->nextcheck = monotonic_seconds();
 	cache_flush();
 
 	*ppos += count;

Re: [PATCH 2/2] sunrpc: use monotonic time in expiry cache

[Chuck Lever]

Hi Neil-

On 02/17/2010 01:47 AM, NeilBrown wrote:
> this protects us from confusion when the wallclock time changes.
>
> We convert to and from wallclock when  setting or reading expiry
> times.
>
> Signed-off-by: NeilBrown<neilb@suse.de>
> ---
>   fs/nfs/dns_resolve.c         |    6 +++---
>   fs/nfsd/nfs4idmap.c          |    2 +-
>   include/linux/sunrpc/cache.h |   21 ++++++++++++++++++---
>   net/sunrpc/cache.c           |   33 ++++++++++++++++++---------------
>   4 files changed, 40 insertions(+), 22 deletions(-)
>
> diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c
> index 95e1ca7..412e6a0 100644
> --- a/fs/nfs/dns_resolve.c
> +++ b/fs/nfs/dns_resolve.c
> @@ -131,7 +131,7 @@ static int nfs_dns_show(struct seq_file *m, struct cache_detail *cd,
>   		return 0;
>   	}
>   	item = container_of(h, struct nfs_dns_ent, h);
> -	ttl = (long)item->h.expiry_time - (long)get_seconds();
> +	ttl = (long)item->h.expiry_time - (long)monotonic_seconds();
>   	if (ttl<  0)
>   		ttl = 0;

> @@ -203,7 +203,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen)
>   	ttl = get_expiry(&buf);
>   	if (ttl == 0)
>   		goto out;
> -	key.h.expiry_time = ttl + get_seconds();
> +	key.h.expiry_time = ttl + monotonic_seconds();
>
>   	ret = -ENOMEM;
>   	item = nfs_dns_lookup(cd,&key);
> @@ -265,7 +265,7 @@ static int do_cache_lookup_nowait(struct cache_detail *cd,
>   		goto out_err;
>   	ret = -ETIMEDOUT;
>   	if (!test_bit(CACHE_VALID,&(*item)->h.flags)
> -			|| (*item)->h.expiry_time<  get_seconds()
> +			|| (*item)->h.expiry_time<  monotonic_seconds()
>   			|| cd->flush_time>  (*item)->h.last_refresh)
>   		goto out_put;
>   	ret = -ENOENT;
> diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
> index 6e2983b..8f5cc77 100644
> --- a/fs/nfsd/nfs4idmap.c
> +++ b/fs/nfsd/nfs4idmap.c
> @@ -549,7 +549,7 @@ do_idmap_lookup_nowait(struct ent *(*lookup_fn)(struct ent *),
>   		goto out_err;
>   	ret = -ETIMEDOUT;
>   	if (!test_bit(CACHE_VALID,&(*item)->h.flags)
> -			|| (*item)->h.expiry_time<  get_seconds()
> +			|| (*item)->h.expiry_time<  monotonic_seconds()
>   			|| detail->flush_time>  (*item)->h.last_refresh)
>   		goto out_put;
>   	ret = -ENOENT;
> diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
> index e41ee6d..153aae8 100644
> --- a/include/linux/sunrpc/cache.h
> +++ b/include/linux/sunrpc/cache.h
> @@ -217,20 +217,35 @@ static inline int get_int(char **bpp, int *anint)
>   	return 0;
>   }
>
> +/*
> + * timestamps kept in the cache are expressed in seconds
> + * since boot.  This is the best for measuring differences in
> + * real time.
> + */
> +static inline unsigned long monotonic_seconds(void)

I find the alternate use of long, unsigned long, and time_t for fields, 
variables, and return values that represent time in seconds to be 
confusing.  Would it be nicer if we stuck with one type, say, time_t, 
for all of these?

That would at least avoid mixed sign comparisons where the return value 
of get_seconds or monotonic_seconds is directly compared to fields in 
the cache_head structure.  It might also simplify the ttl logic above in 
the DNS lookup cache.

I've always wondered if the expiry math behaves correctly when seconds 
wrap.  It hurts my head when I try to prove it is correct.

> +{
> +	struct timespec boot;
> +	getboottime(&boot);
> +	return get_seconds() - boot.tv_sec;
> +}
> +
>   static inline time_t get_expiry(char **bpp)
>   {
>   	int rv;
> +	struct timespec boot;
> +
>   	if (get_int(bpp,&rv))
>   		return 0;
>   	if (rv<  0)
>   		return 0;
> -	return rv;
> +	getboottime(&boot);
> +	return rv - boot.tv_sec;
>   }
>
>   static inline void sunrpc_invalidate(struct cache_head *h,
>   				     struct cache_detail *detail)
>   {
> -	h->expiry_time = get_seconds() - 1;
> -	detail->nextcheck = get_seconds();
> +	h->expiry_time = monotonic_seconds() - 1;
> +	detail->nextcheck = monotonic_seconds();
>   }
>   #endif /*  _LINUX_SUNRPC_CACHE_H_ */
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index 39bddba..379e2bf 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -41,7 +41,7 @@ static void cache_revisit_request(struct cache_head *item);
>
>   static void cache_init(struct cache_head *h)
>   {
> -	time_t now = get_seconds();
> +	time_t now = monotonic_seconds();
>   	h->next = NULL;
>   	h->flags = 0;
>   	kref_init(&h->ref);
> @@ -108,7 +108,7 @@ static void cache_dequeue(struct cache_detail *detail, struct cache_head *ch);
>   static void cache_fresh_locked(struct cache_head *head, time_t expiry)
>   {
>   	head->expiry_time = expiry;
> -	head->last_refresh = get_seconds();
> +	head->last_refresh = monotonic_seconds();
>   	set_bit(CACHE_VALID,&head->flags);
>   }
>
> @@ -184,7 +184,7 @@ static int cache_make_upcall(struct cache_detail *cd, struct cache_head *h)
>   static inline int cache_is_valid(struct cache_detail *detail, struct cache_head *h)
>   {
>   	if (!test_bit(CACHE_VALID,&h->flags) ||
> -	    h->expiry_time<  get_seconds())
> +	    h->expiry_time<  monotonic_seconds())
>   		return -EAGAIN;
>   	else if (detail->flush_time>  h->last_refresh)
>   		return -EAGAIN;
> @@ -222,7 +222,7 @@ int cache_check(struct cache_detail *detail,
>
>   	/* now see if we want to start an upcall */
>   	refresh_age = (h->expiry_time - h->last_refresh);
> -	age = get_seconds() - h->last_refresh;
> +	age = monotonic_seconds() - h->last_refresh;
>
>   	if (rqstp == NULL) {
>   		if (rv == -EAGAIN)
> @@ -372,11 +372,11 @@ static int cache_clean(void)
>   			return -1;
>   		}
>   		current_detail = list_entry(next, struct cache_detail, others);
> -		if (current_detail->nextcheck>  get_seconds())
> +		if (current_detail->nextcheck>  monotonic_seconds())
>   			current_index = current_detail->hash_size;
>   		else {
>   			current_index = 0;
> -			current_detail->nextcheck = get_seconds()+30*60;
> +			current_detail->nextcheck = monotonic_seconds()+30*60;
>   		}
>   	}
>
> @@ -401,7 +401,7 @@ static int cache_clean(void)
>   		for (; ch; cp=&  ch->next, ch= *cp) {
>   			if (current_detail->nextcheck>  ch->expiry_time)
>   				current_detail->nextcheck = ch->expiry_time+1;
> -			if (ch->expiry_time>= get_seconds()&&
> +			if (ch->expiry_time>= monotonic_seconds()&&
>   			ch->last_refresh>= current_detail->flush_time)
>   				continue;
>   			if (test_and_clear_bit(CACHE_PENDING,&ch->flags))
> @@ -465,7 +465,7 @@ EXPORT_SYMBOL_GPL(cache_flush);
>   void cache_purge(struct cache_detail *detail)
>   {
>   	detail->flush_time = LONG_MAX;
> -	detail->nextcheck = get_seconds();
> +	detail->nextcheck = monotonic_seconds();
>   	cache_flush();
>   	detail->flush_time = 1;
>   }
> @@ -1207,7 +1207,8 @@ static int c_show(struct seq_file *m, void *p)
>
>   	ifdebug(CACHE)
>   		seq_printf(m, "# expiry=%ld refcnt=%d flags=%lx\n",
> -			   cp->expiry_time, atomic_read(&cp->ref.refcount), cp->flags);
> +			   cp->expiry_time - monotonic_seconds() + get_seconds(),

I see this calculation in at least one other spot.  Maybe it would be 
more clear if a helper was used.

> +			   atomic_read(&cp->ref.refcount), cp->flags);
>   	cache_get(cp);
>   	if (cache_check(cd, cp, NULL))
>   		/* cache_check does a cache_put on failure */
> @@ -1271,7 +1272,8 @@ static ssize_t read_flush(struct file *file, char __user *buf,
>   	unsigned long p = *ppos;
>   	size_t len;
>
> -	sprintf(tbuf, "%lu\n", cd->flush_time);
> +	sprintf(tbuf, "%lu\n", (cd->flush_time - monotonic_seconds()
> +				+ get_seconds()));
>   	len = strlen(tbuf);
>   	if (p>= len)
>   		return 0;
> @@ -1289,19 +1291,20 @@ static ssize_t write_flush(struct file *file, const char __user *buf,
>   			   struct cache_detail *cd)
>   {
>   	char tbuf[20];
> -	char *ep;
> -	long flushtime;
> +	char *bp, *ep;
> +
>   	if (*ppos || count>  sizeof(tbuf)-1)
>   		return -EINVAL;
>   	if (copy_from_user(tbuf, buf, count))
>   		return -EFAULT;
>   	tbuf[count] = 0;
> -	flushtime = simple_strtoul(tbuf,&ep, 0);
> +	simple_strtoul(tbuf,&ep, 0);
>   	if (*ep&&  *ep != '\n')
>   		return -EINVAL;
>
> -	cd->flush_time = flushtime;
> -	cd->nextcheck = get_seconds();
> +	bp = tbuf;
> +	cd->flush_time = get_expiry(&bp);
> +	cd->nextcheck = monotonic_seconds();
>   	cache_flush();
>
>   	*ppos += count;

cache_check() still has a call to get_seconds() at about line 240; maybe 
that should be monotonic_seconds() instead.

-- 
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 2/2] sunrpc: use monotonic time in expiry cache

[Neil Brown]

On Wed, 17 Feb 2010 17:00:33 -0500
Chuck Lever <chuck.lever@oracle.com> wrote:


> > +/*
> > + * timestamps kept in the cache are expressed in seconds
> > + * since boot.  This is the best for measuring differences in
> > + * real time.
> > + */
> > +static inline unsigned long monotonic_seconds(void)
> 
> I find the alternate use of long, unsigned long, and time_t for fields, 
> variables, and return values that represent time in seconds to be 
> confusing.  Would it be nicer if we stuck with one type, say, time_t, 
> for all of these?

Probably it would...
I think arch's can change it, but the default is that __kernel_time_t is
'long', and so is 'time_t'.
But "get_seconds()" is 'unsigned long'.  No idea why.

You could propose a patch to linux-kernel??

I'll change monotonic_seconds to return time_t, so we can get ride
of the casts in  dns_resolve.c, but I'm not up to trying to push
a broader fix.


> 
> That would at least avoid mixed sign comparisons where the return value 
> of get_seconds or monotonic_seconds is directly compared to fields in 
> the cache_head structure.  It might also simplify the ttl logic above in 
> the DNS lookup cache.
> 
> I've always wondered if the expiry math behaves correctly when seconds 
> wrap.  It hurts my head when I try to prove it is correct.

Seconds shouldn't wrap - or I suspect many more things will go wrong than
just expiry.
Any 32-bit hosts still running in 2038 might have problems, but I wouldn't
be surprised if even phones are 64-bit by then (making it easier to handle
IPv6 :-)



> > @@ -1207,7 +1207,8 @@ static int c_show(struct seq_file *m, void *p)
> >
> >   	ifdebug(CACHE)
> >   		seq_printf(m, "# expiry=%ld refcnt=%d flags=%lx\n",
> > -			   cp->expiry_time, atomic_read(&cp->ref.refcount), cp->flags);
> > +			   cp->expiry_time - monotonic_seconds() + get_seconds(),
> 
> I see this calculation in at least one other spot.  Maybe it would be 
> more clear if a helper was used.

Maybe .. there are only two places, and I don't think a helper would really
simplify it that much... sometimes I prefer things to be open coded so that I
can see exactly what is happening when I read the code.


> cache_check() still has a call to get_seconds() at about line 240; maybe 
> that should be monotonic_seconds() instead.
> 

Yes, you are right.  There is a reason why that one was left unchanged, but
it was only relevant in a previous iteration of the patch.

Thanks!
Revised version follows.

NeilBrown



>From 2e736c816e68580f4555a974508c258b82796873 Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@suse.de>
Date: Wed, 17 Feb 2010 16:35:36 +1100
Subject: [PATCH] sunrpc: use monotonic time in expiry cache

this protects us from confusion when the wallclock time changes.

We convert to and from wallclock when  setting or reading expiry
times.

Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c
index 95e1ca7..9a0b7ad 100644
--- a/fs/nfs/dns_resolve.c
+++ b/fs/nfs/dns_resolve.c
@@ -131,7 +131,7 @@ static int nfs_dns_show(struct seq_file *m, struct cache_detail *cd,
 		return 0;
 	}
 	item = container_of(h, struct nfs_dns_ent, h);
-	ttl = (long)item->h.expiry_time - (long)get_seconds();
+	ttl = item->h.expiry_time - monotonic_seconds();
 	if (ttl < 0)
 		ttl = 0;
 
@@ -203,7 +203,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen)
 	ttl = get_expiry(&buf);
 	if (ttl == 0)
 		goto out;
-	key.h.expiry_time = ttl + get_seconds();
+	key.h.expiry_time = ttl + monotonic_seconds();
 
 	ret = -ENOMEM;
 	item = nfs_dns_lookup(cd, &key);
@@ -265,7 +265,7 @@ static int do_cache_lookup_nowait(struct cache_detail *cd,
 		goto out_err;
 	ret = -ETIMEDOUT;
 	if (!test_bit(CACHE_VALID, &(*item)->h.flags)
-			|| (*item)->h.expiry_time < get_seconds()
+			|| (*item)->h.expiry_time < monotonic_seconds()
 			|| cd->flush_time > (*item)->h.last_refresh)
 		goto out_put;
 	ret = -ENOENT;
diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index 6e2983b..8f5cc77 100644
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -549,7 +549,7 @@ do_idmap_lookup_nowait(struct ent *(*lookup_fn)(struct ent *),
 		goto out_err;
 	ret = -ETIMEDOUT;
 	if (!test_bit(CACHE_VALID, &(*item)->h.flags)
-			|| (*item)->h.expiry_time < get_seconds()
+			|| (*item)->h.expiry_time < monotonic_seconds()
 			|| detail->flush_time > (*item)->h.last_refresh)
 		goto out_put;
 	ret = -ENOENT;
diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index e41ee6d..dbd11e0 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -217,20 +217,35 @@ static inline int get_int(char **bpp, int *anint)
 	return 0;
 }
 
+/*
+ * timestamps kept in the cache are expressed in seconds
+ * since boot.  This is the best for measuring differences in
+ * real time.
+ */
+static inline time_t monotonic_seconds(void)
+{
+	struct timespec boot;
+	getboottime(&boot);
+	return get_seconds() - boot.tv_sec;
+}
+
 static inline time_t get_expiry(char **bpp)
 {
 	int rv;
+	struct timespec boot;
+
 	if (get_int(bpp, &rv))
 		return 0;
 	if (rv < 0)
 		return 0;
-	return rv;
+	getboottime(&boot);
+	return rv - boot.tv_sec;
 }
 
 static inline void sunrpc_invalidate(struct cache_head *h,
 				     struct cache_detail *detail)
 {
-	h->expiry_time = get_seconds() - 1;
-	detail->nextcheck = get_seconds();
+	h->expiry_time = monotonic_seconds() - 1;
+	detail->nextcheck = monotonic_seconds();
 }
 #endif /*  _LINUX_SUNRPC_CACHE_H_ */
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 39bddba..6818dc3 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -41,7 +41,7 @@ static void cache_revisit_request(struct cache_head *item);
 
 static void cache_init(struct cache_head *h)
 {
-	time_t now = get_seconds();
+	time_t now = monotonic_seconds();
 	h->next = NULL;
 	h->flags = 0;
 	kref_init(&h->ref);
@@ -108,7 +108,7 @@ static void cache_dequeue(struct cache_detail *detail, struct cache_head *ch);
 static void cache_fresh_locked(struct cache_head *head, time_t expiry)
 {
 	head->expiry_time = expiry;
-	head->last_refresh = get_seconds();
+	head->last_refresh = monotonic_seconds();
 	set_bit(CACHE_VALID, &head->flags);
 }
 
@@ -184,7 +184,7 @@ static int cache_make_upcall(struct cache_detail *cd, struct cache_head *h)
 static inline int cache_is_valid(struct cache_detail *detail, struct cache_head *h)
 {
 	if (!test_bit(CACHE_VALID, &h->flags) ||
-	    h->expiry_time < get_seconds())
+	    h->expiry_time < monotonic_seconds())
 		return -EAGAIN;
 	else if (detail->flush_time > h->last_refresh)
 		return -EAGAIN;
@@ -222,7 +222,7 @@ int cache_check(struct cache_detail *detail,
 
 	/* now see if we want to start an upcall */
 	refresh_age = (h->expiry_time - h->last_refresh);
-	age = get_seconds() - h->last_refresh;
+	age = monotonic_seconds() - h->last_refresh;
 
 	if (rqstp == NULL) {
 		if (rv == -EAGAIN)
@@ -237,7 +237,7 @@ int cache_check(struct cache_detail *detail,
 				cache_revisit_request(h);
 				if (rv == -EAGAIN) {
 					set_bit(CACHE_NEGATIVE, &h->flags);
-					cache_fresh_locked(h, get_seconds()+CACHE_NEW_EXPIRY);
+					cache_fresh_locked(h, monotonic_seconds()+CACHE_NEW_EXPIRY);
 					cache_fresh_unlocked(h, detail);
 					rv = -ENOENT;
 				}
@@ -372,11 +372,11 @@ static int cache_clean(void)
 			return -1;
 		}
 		current_detail = list_entry(next, struct cache_detail, others);
-		if (current_detail->nextcheck > get_seconds())
+		if (current_detail->nextcheck > monotonic_seconds())
 			current_index = current_detail->hash_size;
 		else {
 			current_index = 0;
-			current_detail->nextcheck = get_seconds()+30*60;
+			current_detail->nextcheck = monotonic_seconds()+30*60;
 		}
 	}
 
@@ -401,7 +401,7 @@ static int cache_clean(void)
 		for (; ch; cp= & ch->next, ch= *cp) {
 			if (current_detail->nextcheck > ch->expiry_time)
 				current_detail->nextcheck = ch->expiry_time+1;
-			if (ch->expiry_time >= get_seconds() &&
+			if (ch->expiry_time >= monotonic_seconds() &&
 			    ch->last_refresh >= current_detail->flush_time)
 				continue;
 			if (test_and_clear_bit(CACHE_PENDING, &ch->flags))
@@ -465,7 +465,7 @@ EXPORT_SYMBOL_GPL(cache_flush);
 void cache_purge(struct cache_detail *detail)
 {
 	detail->flush_time = LONG_MAX;
-	detail->nextcheck = get_seconds();
+	detail->nextcheck = monotonic_seconds();
 	cache_flush();
 	detail->flush_time = 1;
 }
@@ -1207,7 +1207,8 @@ static int c_show(struct seq_file *m, void *p)
 
 	ifdebug(CACHE)
 		seq_printf(m, "# expiry=%ld refcnt=%d flags=%lx\n",
-			   cp->expiry_time, atomic_read(&cp->ref.refcount), cp->flags);
+			   cp->expiry_time - monotonic_seconds() + get_seconds(),
+			   atomic_read(&cp->ref.refcount), cp->flags);
 	cache_get(cp);
 	if (cache_check(cd, cp, NULL))
 		/* cache_check does a cache_put on failure */
@@ -1271,7 +1272,8 @@ static ssize_t read_flush(struct file *file, char __user *buf,
 	unsigned long p = *ppos;
 	size_t len;
 
-	sprintf(tbuf, "%lu\n", cd->flush_time);
+	sprintf(tbuf, "%lu\n", (cd->flush_time - monotonic_seconds()
+				+ get_seconds()));
 	len = strlen(tbuf);
 	if (p >= len)
 		return 0;
@@ -1289,19 +1291,20 @@ static ssize_t write_flush(struct file *file, const char __user *buf,
 			   struct cache_detail *cd)
 {
 	char tbuf[20];
-	char *ep;
-	long flushtime;
+	char *bp, *ep;
+
 	if (*ppos || count > sizeof(tbuf)-1)
 		return -EINVAL;
 	if (copy_from_user(tbuf, buf, count))
 		return -EFAULT;
 	tbuf[count] = 0;
-	flushtime = simple_strtoul(tbuf, &ep, 0);
+	simple_strtoul(tbuf, &ep, 0);
 	if (*ep && *ep != '\n')
 		return -EINVAL;
 
-	cd->flush_time = flushtime;
-	cd->nextcheck = get_seconds();
+	bp = tbuf;
+	cd->flush_time = get_expiry(&bp);
+	cd->nextcheck = monotonic_seconds();
 	cache_flush();
 
 	*ppos += count;

[PATCH 0/2] Use monotonic time stamps in sunrpc auth cache.

[NeilBrown]

A 6 month release cycles seems to be popular these days, and it is six
months since I last posted these, so I guess it is time to post them
again (yes, I've been busy and forgot all about them - but I still
think they are important).

If you set your system time backwards, nfsd can end up caching things
much longer than it should.  And if you set if very far forward,
caches will get flushed too early.

So change all time keeping to use a monotonic time based on
getboottime.

A few issues were raised last time I posted these:
   http://www.spinics.net/lists/linux-nfs/msg11515.html
I think they have all been addressed now.

Thanks,
NeilBrown

---

NeilBrown (2):
      sunrpc: extract some common sunrpc_cache code from nfsd
      sunrpc: use monotonic time in expiry cache


 fs/nfs/dns_resolve.c         |    6 +++---
 fs/nfsd/export.c             |    9 +++------
 fs/nfsd/nfs4idmap.c          |    2 +-
 include/linux/sunrpc/cache.h |   23 ++++++++++++++++++++++-
 net/sunrpc/cache.c           |   37 ++++++++++++++++++++-----------------
 5 files changed, 49 insertions(+), 28 deletions(-)

-- 

[PATCH 2/2] sunrpc: use monotonic time in expiry cache

[NeilBrown]

this protects us from confusion when the wallclock time changes.

We convert to and from wallclock when  setting or reading expiry
times.

Also use monotonic_seconds for last_clost time.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 fs/nfs/dns_resolve.c         |    6 +++---
 fs/nfsd/nfs4idmap.c          |    2 +-
 include/linux/sunrpc/cache.h |   21 ++++++++++++++++++---
 net/sunrpc/cache.c           |   37 ++++++++++++++++++++-----------------
 4 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c
index 76fd235..3e6aab2 100644
--- a/fs/nfs/dns_resolve.c
+++ b/fs/nfs/dns_resolve.c
@@ -144,7 +144,7 @@ static int nfs_dns_show(struct seq_file *m, struct cache_detail *cd,
 		return 0;
 	}
 	item = container_of(h, struct nfs_dns_ent, h);
-	ttl = (long)item->h.expiry_time - (long)get_seconds();
+	ttl = item->h.expiry_time - monotonic_seconds();
 	if (ttl < 0)
 		ttl = 0;
 
@@ -216,7 +216,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen)
 	ttl = get_expiry(&buf);
 	if (ttl == 0)
 		goto out;
-	key.h.expiry_time = ttl + get_seconds();
+	key.h.expiry_time = ttl + monotonic_seconds();
 
 	ret = -ENOMEM;
 	item = nfs_dns_lookup(cd, &key);
@@ -278,7 +278,7 @@ static int do_cache_lookup_nowait(struct cache_detail *cd,
 		goto out_err;
 	ret = -ETIMEDOUT;
 	if (!test_bit(CACHE_VALID, &(*item)->h.flags)
-			|| (*item)->h.expiry_time < get_seconds()
+			|| (*item)->h.expiry_time < monotonic_seconds()
 			|| cd->flush_time > (*item)->h.last_refresh)
 		goto out_put;
 	ret = -ENOENT;
diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index c78dbf4..6031a90 100644
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -550,7 +550,7 @@ do_idmap_lookup_nowait(struct ent *(*lookup_fn)(struct ent *),
 		goto out_err;
 	ret = -ETIMEDOUT;
 	if (!test_bit(CACHE_VALID, &(*item)->h.flags)
-			|| (*item)->h.expiry_time < get_seconds()
+			|| (*item)->h.expiry_time < monotonic_seconds()
 			|| detail->flush_time > (*item)->h.last_refresh)
 		goto out_put;
 	ret = -ENOENT;
diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index 0e1febf..df7c19b 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -218,20 +218,35 @@ static inline int get_int(char **bpp, int *anint)
 	return 0;
 }
 
+/*
+ * timestamps kept in the cache are expressed in seconds
+ * since boot.  This is the best for measuring differences in
+ * real time.
+ */
+static inline time_t monotonic_seconds(void)
+{
+	struct timespec boot;
+	getboottime(&boot);
+	return get_seconds() - boot.tv_sec;
+}
+
 static inline time_t get_expiry(char **bpp)
 {
 	int rv;
+	struct timespec boot;
+
 	if (get_int(bpp, &rv))
 		return 0;
 	if (rv < 0)
 		return 0;
-	return rv;
+	getboottime(&boot);
+	return rv - boot.tv_sec;
 }
 
 static inline void sunrpc_invalidate(struct cache_head *h,
 				     struct cache_detail *detail)
 {
-	h->expiry_time = get_seconds() - 1;
-	detail->nextcheck = get_seconds();
+	h->expiry_time = monotonic_seconds() - 1;
+	detail->nextcheck = monotonic_seconds();
 }
 #endif /*  _LINUX_SUNRPC_CACHE_H_ */
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 2b06410..99d852e 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -42,7 +42,7 @@ static void cache_revisit_request(struct cache_head *item);
 
 static void cache_init(struct cache_head *h)
 {
-	time_t now = get_seconds();
+	time_t now = monotonic_seconds();
 	h->next = NULL;
 	h->flags = 0;
 	kref_init(&h->ref);
@@ -52,7 +52,7 @@ static void cache_init(struct cache_head *h)
 
 static inline int cache_is_expired(struct cache_detail *detail, struct cache_head *h)
 {
-	return  (h->expiry_time < get_seconds()) ||
+	return  (h->expiry_time < monotonic_seconds()) ||
 		(detail->flush_time > h->last_refresh);
 }
 
@@ -127,7 +127,7 @@ static void cache_dequeue(struct cache_detail *detail, struct cache_head *ch);
 static void cache_fresh_locked(struct cache_head *head, time_t expiry)
 {
 	head->expiry_time = expiry;
-	head->last_refresh = get_seconds();
+	head->last_refresh = monotonic_seconds();
 	set_bit(CACHE_VALID, &head->flags);
 }
 
@@ -238,7 +238,7 @@ int cache_check(struct cache_detail *detail,
 
 	/* now see if we want to start an upcall */
 	refresh_age = (h->expiry_time - h->last_refresh);
-	age = get_seconds() - h->last_refresh;
+	age = monotonic_seconds() - h->last_refresh;
 
 	if (rqstp == NULL) {
 		if (rv == -EAGAIN)
@@ -253,7 +253,7 @@ int cache_check(struct cache_detail *detail,
 				cache_revisit_request(h);
 				if (rv == -EAGAIN) {
 					set_bit(CACHE_NEGATIVE, &h->flags);
-					cache_fresh_locked(h, get_seconds()+CACHE_NEW_EXPIRY);
+					cache_fresh_locked(h, monotonic_seconds()+CACHE_NEW_EXPIRY);
 					cache_fresh_unlocked(h, detail);
 					rv = -ENOENT;
 				}
@@ -388,11 +388,11 @@ static int cache_clean(void)
 			return -1;
 		}
 		current_detail = list_entry(next, struct cache_detail, others);
-		if (current_detail->nextcheck > get_seconds())
+		if (current_detail->nextcheck > monotonic_seconds())
 			current_index = current_detail->hash_size;
 		else {
 			current_index = 0;
-			current_detail->nextcheck = get_seconds()+30*60;
+			current_detail->nextcheck = monotonic_seconds()+30*60;
 		}
 	}
 
@@ -477,7 +477,7 @@ EXPORT_SYMBOL_GPL(cache_flush);
 void cache_purge(struct cache_detail *detail)
 {
 	detail->flush_time = LONG_MAX;
-	detail->nextcheck = get_seconds();
+	detail->nextcheck = monotonic_seconds();
 	cache_flush();
 	detail->flush_time = 1;
 }
@@ -902,7 +902,7 @@ static int cache_release(struct inode *inode, struct file *filp,
 		filp->private_data = NULL;
 		kfree(rp);
 
-		cd->last_close = get_seconds();
+		cd->last_close = monotonic_seconds();
 		atomic_dec(&cd->readers);
 	}
 	module_put(cd->owner);
@@ -1034,7 +1034,7 @@ int sunrpc_cache_pipe_upcall(struct cache_detail *detail, struct cache_head *h,
 	int len;
 
 	if (atomic_read(&detail->readers) == 0 &&
-	    detail->last_close < get_seconds() - 30) {
+	    detail->last_close < monotonic_seconds() - 30) {
 			warn_no_listener(detail);
 			return -EINVAL;
 	}
@@ -1219,7 +1219,8 @@ static int c_show(struct seq_file *m, void *p)
 
 	ifdebug(CACHE)
 		seq_printf(m, "# expiry=%ld refcnt=%d flags=%lx\n",
-			   cp->expiry_time, atomic_read(&cp->ref.refcount), cp->flags);
+			   cp->expiry_time - monotonic_seconds() + get_seconds(),
+			   atomic_read(&cp->ref.refcount), cp->flags);
 	cache_get(cp);
 	if (cache_check(cd, cp, NULL))
 		/* cache_check does a cache_put on failure */
@@ -1285,7 +1286,8 @@ static ssize_t read_flush(struct file *file, char __user *buf,
 	unsigned long p = *ppos;
 	size_t len;
 
-	sprintf(tbuf, "%lu\n", cd->flush_time);
+	sprintf(tbuf, "%lu\n", (cd->flush_time - monotonic_seconds()
+				+ get_seconds()));
 	len = strlen(tbuf);
 	if (p >= len)
 		return 0;
@@ -1303,19 +1305,20 @@ static ssize_t write_flush(struct file *file, const char __user *buf,
 			   struct cache_detail *cd)
 {
 	char tbuf[20];
-	char *ep;
-	long flushtime;
+	char *bp, *ep;
+
 	if (*ppos || count > sizeof(tbuf)-1)
 		return -EINVAL;
 	if (copy_from_user(tbuf, buf, count))
 		return -EFAULT;
 	tbuf[count] = 0;
-	flushtime = simple_strtoul(tbuf, &ep, 0);
+	simple_strtoul(tbuf, &ep, 0);
 	if (*ep && *ep != '\n')
 		return -EINVAL;
 
-	cd->flush_time = flushtime;
-	cd->nextcheck = get_seconds();
+	bp = tbuf;
+	cd->flush_time = get_expiry(&bp);
+	cd->nextcheck = monotonic_seconds();
 	cache_flush();
 
 	*ppos += count;

Re: [PATCH 2/2] sunrpc: use monotonic time in expiry cache

[J. Bruce Fields]

On Thu, Aug 12, 2010 at 04:55:22PM +1000, NeilBrown wrote:
> this protects us from confusion when the wallclock time changes.
> 
> We convert to and from wallclock when  setting or reading expiry
> times.
> 
> Also use monotonic_seconds for last_clost time.

Looks good to me, thanks--applying for 2.6.37.

(Apologies for the delay--partly due to my getting fed up with not
understanding time, and feeling I should go read some code.  Resulting
notes at http://fieldses.org/~bfields/kernel/time.txt.

The only thing I noticed was that the timekeeping code consistently uses
the word "monotonic" on functions that return something slightly
different; seconds_since_boot() might be a better name.

Oh, and

	get_seconds() - monotonic_seconds()

isn't the most intuitive way possible to say "boot time in seconds".  If
you want to fix either of those, fine, otherwise no big deal.)

--b.

> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  fs/nfs/dns_resolve.c         |    6 +++---
>  fs/nfsd/nfs4idmap.c          |    2 +-
>  include/linux/sunrpc/cache.h |   21 ++++++++++++++++++---
>  net/sunrpc/cache.c           |   37 ++++++++++++++++++++-----------------
>  4 files changed, 42 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c
> index 76fd235..3e6aab2 100644
> --- a/fs/nfs/dns_resolve.c
> +++ b/fs/nfs/dns_resolve.c
> @@ -144,7 +144,7 @@ static int nfs_dns_show(struct seq_file *m, struct cache_detail *cd,
>  		return 0;
>  	}
>  	item = container_of(h, struct nfs_dns_ent, h);
> -	ttl = (long)item->h.expiry_time - (long)get_seconds();
> +	ttl = item->h.expiry_time - monotonic_seconds();
>  	if (ttl < 0)
>  		ttl = 0;
>  
> @@ -216,7 +216,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen)
>  	ttl = get_expiry(&buf);
>  	if (ttl == 0)
>  		goto out;
> -	key.h.expiry_time = ttl + get_seconds();
> +	key.h.expiry_time = ttl + monotonic_seconds();
>  
>  	ret = -ENOMEM;
>  	item = nfs_dns_lookup(cd, &key);
> @@ -278,7 +278,7 @@ static int do_cache_lookup_nowait(struct cache_detail *cd,
>  		goto out_err;
>  	ret = -ETIMEDOUT;
>  	if (!test_bit(CACHE_VALID, &(*item)->h.flags)
> -			|| (*item)->h.expiry_time < get_seconds()
> +			|| (*item)->h.expiry_time < monotonic_seconds()
>  			|| cd->flush_time > (*item)->h.last_refresh)
>  		goto out_put;
>  	ret = -ENOENT;
> diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
> index c78dbf4..6031a90 100644
> --- a/fs/nfsd/nfs4idmap.c
> +++ b/fs/nfsd/nfs4idmap.c
> @@ -550,7 +550,7 @@ do_idmap_lookup_nowait(struct ent *(*lookup_fn)(struct ent *),
>  		goto out_err;
>  	ret = -ETIMEDOUT;
>  	if (!test_bit(CACHE_VALID, &(*item)->h.flags)
> -			|| (*item)->h.expiry_time < get_seconds()
> +			|| (*item)->h.expiry_time < monotonic_seconds()
>  			|| detail->flush_time > (*item)->h.last_refresh)
>  		goto out_put;
>  	ret = -ENOENT;
> diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
> index 0e1febf..df7c19b 100644
> --- a/include/linux/sunrpc/cache.h
> +++ b/include/linux/sunrpc/cache.h
> @@ -218,20 +218,35 @@ static inline int get_int(char **bpp, int *anint)
>  	return 0;
>  }
>  
> +/*
> + * timestamps kept in the cache are expressed in seconds
> + * since boot.  This is the best for measuring differences in
> + * real time.
> + */
> +static inline time_t monotonic_seconds(void)
> +{
> +	struct timespec boot;
> +	getboottime(&boot);
> +	return get_seconds() - boot.tv_sec;
> +}
> +
>  static inline time_t get_expiry(char **bpp)
>  {
>  	int rv;
> +	struct timespec boot;
> +
>  	if (get_int(bpp, &rv))
>  		return 0;
>  	if (rv < 0)
>  		return 0;
> -	return rv;
> +	getboottime(&boot);
> +	return rv - boot.tv_sec;
>  }
>  
>  static inline void sunrpc_invalidate(struct cache_head *h,
>  				     struct cache_detail *detail)
>  {
> -	h->expiry_time = get_seconds() - 1;
> -	detail->nextcheck = get_seconds();
> +	h->expiry_time = monotonic_seconds() - 1;
> +	detail->nextcheck = monotonic_seconds();
>  }
>  #endif /*  _LINUX_SUNRPC_CACHE_H_ */
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index 2b06410..99d852e 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -42,7 +42,7 @@ static void cache_revisit_request(struct cache_head *item);
>  
>  static void cache_init(struct cache_head *h)
>  {
> -	time_t now = get_seconds();
> +	time_t now = monotonic_seconds();
>  	h->next = NULL;
>  	h->flags = 0;
>  	kref_init(&h->ref);
> @@ -52,7 +52,7 @@ static void cache_init(struct cache_head *h)
>  
>  static inline int cache_is_expired(struct cache_detail *detail, struct cache_head *h)
>  {
> -	return  (h->expiry_time < get_seconds()) ||
> +	return  (h->expiry_time < monotonic_seconds()) ||
>  		(detail->flush_time > h->last_refresh);
>  }
>  
> @@ -127,7 +127,7 @@ static void cache_dequeue(struct cache_detail *detail, struct cache_head *ch);
>  static void cache_fresh_locked(struct cache_head *head, time_t expiry)
>  {
>  	head->expiry_time = expiry;
> -	head->last_refresh = get_seconds();
> +	head->last_refresh = monotonic_seconds();
>  	set_bit(CACHE_VALID, &head->flags);
>  }
>  
> @@ -238,7 +238,7 @@ int cache_check(struct cache_detail *detail,
>  
>  	/* now see if we want to start an upcall */
>  	refresh_age = (h->expiry_time - h->last_refresh);
> -	age = get_seconds() - h->last_refresh;
> +	age = monotonic_seconds() - h->last_refresh;
>  
>  	if (rqstp == NULL) {
>  		if (rv == -EAGAIN)
> @@ -253,7 +253,7 @@ int cache_check(struct cache_detail *detail,
>  				cache_revisit_request(h);
>  				if (rv == -EAGAIN) {
>  					set_bit(CACHE_NEGATIVE, &h->flags);
> -					cache_fresh_locked(h, get_seconds()+CACHE_NEW_EXPIRY);
> +					cache_fresh_locked(h, monotonic_seconds()+CACHE_NEW_EXPIRY);
>  					cache_fresh_unlocked(h, detail);
>  					rv = -ENOENT;
>  				}
> @@ -388,11 +388,11 @@ static int cache_clean(void)
>  			return -1;
>  		}
>  		current_detail = list_entry(next, struct cache_detail, others);
> -		if (current_detail->nextcheck > get_seconds())
> +		if (current_detail->nextcheck > monotonic_seconds())
>  			current_index = current_detail->hash_size;
>  		else {
>  			current_index = 0;
> -			current_detail->nextcheck = get_seconds()+30*60;
> +			current_detail->nextcheck = monotonic_seconds()+30*60;
>  		}
>  	}
>  
> @@ -477,7 +477,7 @@ EXPORT_SYMBOL_GPL(cache_flush);
>  void cache_purge(struct cache_detail *detail)
>  {
>  	detail->flush_time = LONG_MAX;
> -	detail->nextcheck = get_seconds();
> +	detail->nextcheck = monotonic_seconds();
>  	cache_flush();
>  	detail->flush_time = 1;
>  }
> @@ -902,7 +902,7 @@ static int cache_release(struct inode *inode, struct file *filp,
>  		filp->private_data = NULL;
>  		kfree(rp);
>  
> -		cd->last_close = get_seconds();
> +		cd->last_close = monotonic_seconds();
>  		atomic_dec(&cd->readers);
>  	}
>  	module_put(cd->owner);
> @@ -1034,7 +1034,7 @@ int sunrpc_cache_pipe_upcall(struct cache_detail *detail, struct cache_head *h,
>  	int len;
>  
>  	if (atomic_read(&detail->readers) == 0 &&
> -	    detail->last_close < get_seconds() - 30) {
> +	    detail->last_close < monotonic_seconds() - 30) {
>  			warn_no_listener(detail);
>  			return -EINVAL;
>  	}
> @@ -1219,7 +1219,8 @@ static int c_show(struct seq_file *m, void *p)
>  
>  	ifdebug(CACHE)
>  		seq_printf(m, "# expiry=%ld refcnt=%d flags=%lx\n",
> -			   cp->expiry_time, atomic_read(&cp->ref.refcount), cp->flags);
> +			   cp->expiry_time - monotonic_seconds() + get_seconds(),
> +			   atomic_read(&cp->ref.refcount), cp->flags);
>  	cache_get(cp);
>  	if (cache_check(cd, cp, NULL))
>  		/* cache_check does a cache_put on failure */
> @@ -1285,7 +1286,8 @@ static ssize_t read_flush(struct file *file, char __user *buf,
>  	unsigned long p = *ppos;
>  	size_t len;
>  
> -	sprintf(tbuf, "%lu\n", cd->flush_time);
> +	sprintf(tbuf, "%lu\n", (cd->flush_time - monotonic_seconds()
> +				+ get_seconds()));
>  	len = strlen(tbuf);
>  	if (p >= len)
>  		return 0;
> @@ -1303,19 +1305,20 @@ static ssize_t write_flush(struct file *file, const char __user *buf,
>  			   struct cache_detail *cd)
>  {
>  	char tbuf[20];
> -	char *ep;
> -	long flushtime;
> +	char *bp, *ep;
> +
>  	if (*ppos || count > sizeof(tbuf)-1)
>  		return -EINVAL;
>  	if (copy_from_user(tbuf, buf, count))
>  		return -EFAULT;
>  	tbuf[count] = 0;
> -	flushtime = simple_strtoul(tbuf, &ep, 0);
> +	simple_strtoul(tbuf, &ep, 0);
>  	if (*ep && *ep != '\n')
>  		return -EINVAL;
>  
> -	cd->flush_time = flushtime;
> -	cd->nextcheck = get_seconds();
> +	bp = tbuf;
> +	cd->flush_time = get_expiry(&bp);
> +	cd->nextcheck = monotonic_seconds();
>  	cache_flush();
>  
>  	*ppos += count;
> 
> 

Re: [PATCH 2/2] sunrpc: use monotonic time in expiry cache

[Neil Brown]

On Tue, 24 Aug 2010 14:15:31 -0400
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Thu, Aug 12, 2010 at 04:55:22PM +1000, NeilBrown wrote:
> > this protects us from confusion when the wallclock time changes.
> > 
> > We convert to and from wallclock when  setting or reading expiry
> > times.
> > 
> > Also use monotonic_seconds for last_clost time.
> 
> Looks good to me, thanks--applying for 2.6.37.
> 
> (Apologies for the delay--partly due to my getting fed up with not
> understanding time, and feeling I should go read some code.  Resulting
> notes at http://fieldses.org/~bfields/kernel/time.txt.
> 
> The only thing I noticed was that the timekeeping code consistently uses
> the word "monotonic" on functions that return something slightly
> different; seconds_since_boot() might be a better name.

Yes.... I think I developed this against 2.6.16 which has different naming
conventions, and then forward-parted to -current without giving too much
thought to the names.   seconds_since_boot() does sound better.

> 
> Oh, and
> 
> 	get_seconds() - monotonic_seconds()
> 
> isn't the most intuitive way possible to say "boot time in seconds".  If
> you want to fix either of those, fine, otherwise no big deal.)

To be fair, I don't use that exactly.  I use

      some_monotonic_based_value - monotonic_seconds() + get_seconds()

to turn a monotonic_based value to a wallclock based value.
This makes sense to me - subtract the base I don't want, and add the base
that I do.

I guess you could wrap that in convert_to_wallclock and use getboottime
directly:

static inline time_t convert_to_wallclock(time_t sinceboot)
{
	struct timespec boot;
	getboottime(&boot);
	return sinceboot + boot.tv_sec;
}

Following is only compile tested.

Thanks,
NeilBrown

-------

Rename monotonic_seconds to seconds_since_boot

monotonic has a meaning in Linux timekeeping slight different to what we
use in sunrpc_cache, so change the name.

Also make the conversion from second_since_boot times to wallclock times less
obscure.

Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c
index 3e6aab2..97c03ca 100644
--- a/fs/nfs/dns_resolve.c
+++ b/fs/nfs/dns_resolve.c
@@ -144,7 +144,7 @@ static int nfs_dns_show(struct seq_file *m, struct cache_detail *cd,
 		return 0;
 	}
 	item = container_of(h, struct nfs_dns_ent, h);
-	ttl = item->h.expiry_time - monotonic_seconds();
+	ttl = item->h.expiry_time - seconds_since_boot();
 	if (ttl < 0)
 		ttl = 0;
 
@@ -216,7 +216,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen)
 	ttl = get_expiry(&buf);
 	if (ttl == 0)
 		goto out;
-	key.h.expiry_time = ttl + monotonic_seconds();
+	key.h.expiry_time = ttl + seconds_since_boot();
 
 	ret = -ENOMEM;
 	item = nfs_dns_lookup(cd, &key);
@@ -278,7 +278,7 @@ static int do_cache_lookup_nowait(struct cache_detail *cd,
 		goto out_err;
 	ret = -ETIMEDOUT;
 	if (!test_bit(CACHE_VALID, &(*item)->h.flags)
-			|| (*item)->h.expiry_time < monotonic_seconds()
+			|| (*item)->h.expiry_time < seconds_since_boot()
 			|| cd->flush_time > (*item)->h.last_refresh)
 		goto out_put;
 	ret = -ENOENT;
diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index 6031a90..808b33a 100644
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -550,7 +550,7 @@ do_idmap_lookup_nowait(struct ent *(*lookup_fn)(struct ent *),
 		goto out_err;
 	ret = -ETIMEDOUT;
 	if (!test_bit(CACHE_VALID, &(*item)->h.flags)
-			|| (*item)->h.expiry_time < monotonic_seconds()
+			|| (*item)->h.expiry_time < seconds_since_boot()
 			|| detail->flush_time > (*item)->h.last_refresh)
 		goto out_put;
 	ret = -ENOENT;
diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index df7c19b..ece432b 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -223,13 +223,20 @@ static inline int get_int(char **bpp, int *anint)
  * since boot.  This is the best for measuring differences in
  * real time.
  */
-static inline time_t monotonic_seconds(void)
+static inline time_t seconds_since_boot(void)
 {
 	struct timespec boot;
 	getboottime(&boot);
 	return get_seconds() - boot.tv_sec;
 }
 
+static inline time_t convert_to_wallclock(time_t sinceboot)
+{
+	struct timespec boot;
+	getboottime(&boot);
+	return boot.tv_sec + sinceboot;
+}
+
 static inline time_t get_expiry(char **bpp)
 {
 	int rv;
@@ -246,7 +253,7 @@ static inline time_t get_expiry(char **bpp)
 static inline void sunrpc_invalidate(struct cache_head *h,
 				     struct cache_detail *detail)
 {
-	h->expiry_time = monotonic_seconds() - 1;
-	detail->nextcheck = monotonic_seconds();
+	h->expiry_time = seconds_since_boot() - 1;
+	detail->nextcheck = seconds_since_boot();
 }
 #endif /*  _LINUX_SUNRPC_CACHE_H_ */
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 99d852e..8dc1219 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -42,7 +42,7 @@ static void cache_revisit_request(struct cache_head *item);
 
 static void cache_init(struct cache_head *h)
 {
-	time_t now = monotonic_seconds();
+	time_t now = seconds_since_boot();
 	h->next = NULL;
 	h->flags = 0;
 	kref_init(&h->ref);
@@ -52,7 +52,7 @@ static void cache_init(struct cache_head *h)
 
 static inline int cache_is_expired(struct cache_detail *detail, struct cache_head *h)
 {
-	return  (h->expiry_time < monotonic_seconds()) ||
+	return  (h->expiry_time < seconds_since_boot()) ||
 		(detail->flush_time > h->last_refresh);
 }
 
@@ -127,7 +127,7 @@ static void cache_dequeue(struct cache_detail *detail, struct cache_head *ch);
 static void cache_fresh_locked(struct cache_head *head, time_t expiry)
 {
 	head->expiry_time = expiry;
-	head->last_refresh = monotonic_seconds();
+	head->last_refresh = seconds_since_boot();
 	set_bit(CACHE_VALID, &head->flags);
 }
 
@@ -238,7 +238,7 @@ int cache_check(struct cache_detail *detail,
 
 	/* now see if we want to start an upcall */
 	refresh_age = (h->expiry_time - h->last_refresh);
-	age = monotonic_seconds() - h->last_refresh;
+	age = seconds_since_boot() - h->last_refresh;
 
 	if (rqstp == NULL) {
 		if (rv == -EAGAIN)
@@ -253,7 +253,7 @@ int cache_check(struct cache_detail *detail,
 				cache_revisit_request(h);
 				if (rv == -EAGAIN) {
 					set_bit(CACHE_NEGATIVE, &h->flags);
-					cache_fresh_locked(h, monotonic_seconds()+CACHE_NEW_EXPIRY);
+					cache_fresh_locked(h, seconds_since_boot()+CACHE_NEW_EXPIRY);
 					cache_fresh_unlocked(h, detail);
 					rv = -ENOENT;
 				}
@@ -388,11 +388,11 @@ static int cache_clean(void)
 			return -1;
 		}
 		current_detail = list_entry(next, struct cache_detail, others);
-		if (current_detail->nextcheck > monotonic_seconds())
+		if (current_detail->nextcheck > seconds_since_boot())
 			current_index = current_detail->hash_size;
 		else {
 			current_index = 0;
-			current_detail->nextcheck = monotonic_seconds()+30*60;
+			current_detail->nextcheck = seconds_since_boot()+30*60;
 		}
 	}
 
@@ -477,7 +477,7 @@ EXPORT_SYMBOL_GPL(cache_flush);
 void cache_purge(struct cache_detail *detail)
 {
 	detail->flush_time = LONG_MAX;
-	detail->nextcheck = monotonic_seconds();
+	detail->nextcheck = seconds_since_boot();
 	cache_flush();
 	detail->flush_time = 1;
 }
@@ -902,7 +902,7 @@ static int cache_release(struct inode *inode, struct file *filp,
 		filp->private_data = NULL;
 		kfree(rp);
 
-		cd->last_close = monotonic_seconds();
+		cd->last_close = seconds_since_boot();
 		atomic_dec(&cd->readers);
 	}
 	module_put(cd->owner);
@@ -1034,7 +1034,7 @@ int sunrpc_cache_pipe_upcall(struct cache_detail *detail, struct cache_head *h,
 	int len;
 
 	if (atomic_read(&detail->readers) == 0 &&
-	    detail->last_close < monotonic_seconds() - 30) {
+	    detail->last_close < seconds_since_boot() - 30) {
 			warn_no_listener(detail);
 			return -EINVAL;
 	}
@@ -1219,7 +1219,7 @@ static int c_show(struct seq_file *m, void *p)
 
 	ifdebug(CACHE)
 		seq_printf(m, "# expiry=%ld refcnt=%d flags=%lx\n",
-			   cp->expiry_time - monotonic_seconds() + get_seconds(),
+			   convert_to_wallclock(cp->expiry_time),
 			   atomic_read(&cp->ref.refcount), cp->flags);
 	cache_get(cp);
 	if (cache_check(cd, cp, NULL))
@@ -1286,8 +1286,7 @@ static ssize_t read_flush(struct file *file, char __user *buf,
 	unsigned long p = *ppos;
 	size_t len;
 
-	sprintf(tbuf, "%lu\n", (cd->flush_time - monotonic_seconds()
-				+ get_seconds()));
+	sprintf(tbuf, "%lu\n", convert_to_wallclock(cd->flush_time));
 	len = strlen(tbuf);
 	if (p >= len)
 		return 0;
@@ -1318,7 +1317,7 @@ static ssize_t write_flush(struct file *file, const char __user *buf,
 
 	bp = tbuf;
 	cd->flush_time = get_expiry(&bp);
-	cd->nextcheck = monotonic_seconds();
+	cd->nextcheck = seconds_since_boot();
 	cache_flush();
 
 	*ppos += count;

Re: [PATCH 2/2] sunrpc: use monotonic time in expiry cache

[J. Bruce Fields]

On Wed, Aug 25, 2010 at 09:12:26AM +1000, Neil Brown wrote:
> Yes.... I think I developed this against 2.6.16 which has different naming
> conventions, and then forward-parted to -current without giving too much
> thought to the names.   seconds_since_boot() does sound better.
> 
> > 
> > Oh, and
> > 
> > 	get_seconds() - monotonic_seconds()
> > 
> > isn't the most intuitive way possible to say "boot time in seconds".  If
> > you want to fix either of those, fine, otherwise no big deal.)
> 
> To be fair, I don't use that exactly.  I use
> 
>       some_monotonic_based_value - monotonic_seconds() + get_seconds()
> 
> to turn a monotonic_based value to a wallclock based value.
> This makes sense to me - subtract the base I don't want, and add the base
> that I do.

Sure....

> I guess you could wrap that in convert_to_wallclock and use getboottime
> directly:
> 
> static inline time_t convert_to_wallclock(time_t sinceboot)
> {
> 	struct timespec boot;
> 	getboottime(&boot);
> 	return sinceboot + boot.tv_sec;
> }

....  But I think that makes the calculation truly, blindingly obvious.

> Following is only compile tested.

Merged into the previous patch (not pushed out yet pending some
testing).  Thanks!

--b.

> 
> Thanks,
> NeilBrown
> 
> -------
> 
> Rename monotonic_seconds to seconds_since_boot
> 
> monotonic has a meaning in Linux timekeeping slight different to what we
> use in sunrpc_cache, so change the name.
> 
> Also make the conversion from second_since_boot times to wallclock times less
> obscure.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> 
> diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c
> index 3e6aab2..97c03ca 100644
> --- a/fs/nfs/dns_resolve.c
> +++ b/fs/nfs/dns_resolve.c
> @@ -144,7 +144,7 @@ static int nfs_dns_show(struct seq_file *m, struct cache_detail *cd,
>  		return 0;
>  	}
>  	item = container_of(h, struct nfs_dns_ent, h);
> -	ttl = item->h.expiry_time - monotonic_seconds();
> +	ttl = item->h.expiry_time - seconds_since_boot();
>  	if (ttl < 0)
>  		ttl = 0;
>  
> @@ -216,7 +216,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen)
>  	ttl = get_expiry(&buf);
>  	if (ttl == 0)
>  		goto out;
> -	key.h.expiry_time = ttl + monotonic_seconds();
> +	key.h.expiry_time = ttl + seconds_since_boot();
>  
>  	ret = -ENOMEM;
>  	item = nfs_dns_lookup(cd, &key);
> @@ -278,7 +278,7 @@ static int do_cache_lookup_nowait(struct cache_detail *cd,
>  		goto out_err;
>  	ret = -ETIMEDOUT;
>  	if (!test_bit(CACHE_VALID, &(*item)->h.flags)
> -			|| (*item)->h.expiry_time < monotonic_seconds()
> +			|| (*item)->h.expiry_time < seconds_since_boot()
>  			|| cd->flush_time > (*item)->h.last_refresh)
>  		goto out_put;
>  	ret = -ENOENT;
> diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
> index 6031a90..808b33a 100644
> --- a/fs/nfsd/nfs4idmap.c
> +++ b/fs/nfsd/nfs4idmap.c
> @@ -550,7 +550,7 @@ do_idmap_lookup_nowait(struct ent *(*lookup_fn)(struct ent *),
>  		goto out_err;
>  	ret = -ETIMEDOUT;
>  	if (!test_bit(CACHE_VALID, &(*item)->h.flags)
> -			|| (*item)->h.expiry_time < monotonic_seconds()
> +			|| (*item)->h.expiry_time < seconds_since_boot()
>  			|| detail->flush_time > (*item)->h.last_refresh)
>  		goto out_put;
>  	ret = -ENOENT;
> diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
> index df7c19b..ece432b 100644
> --- a/include/linux/sunrpc/cache.h
> +++ b/include/linux/sunrpc/cache.h
> @@ -223,13 +223,20 @@ static inline int get_int(char **bpp, int *anint)
>   * since boot.  This is the best for measuring differences in
>   * real time.
>   */
> -static inline time_t monotonic_seconds(void)
> +static inline time_t seconds_since_boot(void)
>  {
>  	struct timespec boot;
>  	getboottime(&boot);
>  	return get_seconds() - boot.tv_sec;
>  }
>  
> +static inline time_t convert_to_wallclock(time_t sinceboot)
> +{
> +	struct timespec boot;
> +	getboottime(&boot);
> +	return boot.tv_sec + sinceboot;
> +}
> +
>  static inline time_t get_expiry(char **bpp)
>  {
>  	int rv;
> @@ -246,7 +253,7 @@ static inline time_t get_expiry(char **bpp)
>  static inline void sunrpc_invalidate(struct cache_head *h,
>  				     struct cache_detail *detail)
>  {
> -	h->expiry_time = monotonic_seconds() - 1;
> -	detail->nextcheck = monotonic_seconds();
> +	h->expiry_time = seconds_since_boot() - 1;
> +	detail->nextcheck = seconds_since_boot();
>  }
>  #endif /*  _LINUX_SUNRPC_CACHE_H_ */
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index 99d852e..8dc1219 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -42,7 +42,7 @@ static void cache_revisit_request(struct cache_head *item);
>  
>  static void cache_init(struct cache_head *h)
>  {
> -	time_t now = monotonic_seconds();
> +	time_t now = seconds_since_boot();
>  	h->next = NULL;
>  	h->flags = 0;
>  	kref_init(&h->ref);
> @@ -52,7 +52,7 @@ static void cache_init(struct cache_head *h)
>  
>  static inline int cache_is_expired(struct cache_detail *detail, struct cache_head *h)
>  {
> -	return  (h->expiry_time < monotonic_seconds()) ||
> +	return  (h->expiry_time < seconds_since_boot()) ||
>  		(detail->flush_time > h->last_refresh);
>  }
>  
> @@ -127,7 +127,7 @@ static void cache_dequeue(struct cache_detail *detail, struct cache_head *ch);
>  static void cache_fresh_locked(struct cache_head *head, time_t expiry)
>  {
>  	head->expiry_time = expiry;
> -	head->last_refresh = monotonic_seconds();
> +	head->last_refresh = seconds_since_boot();
>  	set_bit(CACHE_VALID, &head->flags);
>  }
>  
> @@ -238,7 +238,7 @@ int cache_check(struct cache_detail *detail,
>  
>  	/* now see if we want to start an upcall */
>  	refresh_age = (h->expiry_time - h->last_refresh);
> -	age = monotonic_seconds() - h->last_refresh;
> +	age = seconds_since_boot() - h->last_refresh;
>  
>  	if (rqstp == NULL) {
>  		if (rv == -EAGAIN)
> @@ -253,7 +253,7 @@ int cache_check(struct cache_detail *detail,
>  				cache_revisit_request(h);
>  				if (rv == -EAGAIN) {
>  					set_bit(CACHE_NEGATIVE, &h->flags);
> -					cache_fresh_locked(h, monotonic_seconds()+CACHE_NEW_EXPIRY);
> +					cache_fresh_locked(h, seconds_since_boot()+CACHE_NEW_EXPIRY);
>  					cache_fresh_unlocked(h, detail);
>  					rv = -ENOENT;
>  				}
> @@ -388,11 +388,11 @@ static int cache_clean(void)
>  			return -1;
>  		}
>  		current_detail = list_entry(next, struct cache_detail, others);
> -		if (current_detail->nextcheck > monotonic_seconds())
> +		if (current_detail->nextcheck > seconds_since_boot())
>  			current_index = current_detail->hash_size;
>  		else {
>  			current_index = 0;
> -			current_detail->nextcheck = monotonic_seconds()+30*60;
> +			current_detail->nextcheck = seconds_since_boot()+30*60;
>  		}
>  	}
>  
> @@ -477,7 +477,7 @@ EXPORT_SYMBOL_GPL(cache_flush);
>  void cache_purge(struct cache_detail *detail)
>  {
>  	detail->flush_time = LONG_MAX;
> -	detail->nextcheck = monotonic_seconds();
> +	detail->nextcheck = seconds_since_boot();
>  	cache_flush();
>  	detail->flush_time = 1;
>  }
> @@ -902,7 +902,7 @@ static int cache_release(struct inode *inode, struct file *filp,
>  		filp->private_data = NULL;
>  		kfree(rp);
>  
> -		cd->last_close = monotonic_seconds();
> +		cd->last_close = seconds_since_boot();
>  		atomic_dec(&cd->readers);
>  	}
>  	module_put(cd->owner);
> @@ -1034,7 +1034,7 @@ int sunrpc_cache_pipe_upcall(struct cache_detail *detail, struct cache_head *h,
>  	int len;
>  
>  	if (atomic_read(&detail->readers) == 0 &&
> -	    detail->last_close < monotonic_seconds() - 30) {
> +	    detail->last_close < seconds_since_boot() - 30) {
>  			warn_no_listener(detail);
>  			return -EINVAL;
>  	}
> @@ -1219,7 +1219,7 @@ static int c_show(struct seq_file *m, void *p)
>  
>  	ifdebug(CACHE)
>  		seq_printf(m, "# expiry=%ld refcnt=%d flags=%lx\n",
> -			   cp->expiry_time - monotonic_seconds() + get_seconds(),
> +			   convert_to_wallclock(cp->expiry_time),
>  			   atomic_read(&cp->ref.refcount), cp->flags);
>  	cache_get(cp);
>  	if (cache_check(cd, cp, NULL))
> @@ -1286,8 +1286,7 @@ static ssize_t read_flush(struct file *file, char __user *buf,
>  	unsigned long p = *ppos;
>  	size_t len;
>  
> -	sprintf(tbuf, "%lu\n", (cd->flush_time - monotonic_seconds()
> -				+ get_seconds()));
> +	sprintf(tbuf, "%lu\n", convert_to_wallclock(cd->flush_time));
>  	len = strlen(tbuf);
>  	if (p >= len)
>  		return 0;
> @@ -1318,7 +1317,7 @@ static ssize_t write_flush(struct file *file, const char __user *buf,
>  
>  	bp = tbuf;
>  	cd->flush_time = get_expiry(&bp);
> -	cd->nextcheck = monotonic_seconds();
> +	cd->nextcheck = seconds_since_boot();
>  	cache_flush();
>  
>  	*ppos += count;