Put a timeout on -ve name cache entries in NFS

[John Baldwin]

This patch allows one to put an upper time limit on how long the NFS client
should keep negative cache entries around for a given directory.  This is
basically a safety belt as there are certain races with -ve entries that are
not easily fixed (e.g. dealing with the low resolution of the directory
modification timestamps).  However, timing out the entries would put a limit
on how stale the client's cache entries could be.  My main question is do
folks think this value should be tunable as a global sysctl or a per-mount
option.  A sysctl is far easier to implement (and the acccess cache timeout
is a sysctl).  However, the other namecache-related settings are all mount
options, so a mount option might be more consistent.  Current rough patch is
below:

--- //depot/projects/smpng/sys/nfsclient/nfs_vnops.c 2009/10/19 14:27:26
+++ //depot/user/jhb/lock/nfsclient/nfs_vnops.c 2009/10/19 19:53:51
@@ -981,9 +981,11 @@
  * We only accept a negative hit in the cache if the
  * modification time of the parent directory matches
  * our cached copy.  Otherwise, we discard all of the
- * negative cache entries for this directory.
+ * negative cache entries for this directory.  We also
+ * only trust -ve cache entries for up to 5 seconds.
  */
- if (VOP_GETATTR(dvp, &vattr, cnp->cn_cred) == 0 &&
+ if ((u_int)(ticks - np->n_dmtime_ticks) <= 5 &&
+    VOP_GETATTR(dvp, &vattr, cnp->cn_cred) == 0 &&
     vattr.va_mtime.tv_sec == np->n_dmtime) {
  nfsstats.lookupcache_hits++;
  return (ENOENT);
@@ -1157,8 +1159,10 @@
  */
  mtx_lock(&np->n_mtx);
  if (np->n_dmtime <= dmtime) {
- if (np->n_dmtime == 0)
+ if (np->n_dmtime == 0) {
  np->n_dmtime = dmtime;
+ np->n_dmtime_ticks = ticks;
+ }
  mtx_unlock(&np->n_mtx);
  cache_enter(dvp, NULL, cnp);
  } else
--- //depot/projects/smpng/sys/nfsclient/nfsnode.h 2009/05/29 14:35:29
+++ //depot/user/jhb/lock/nfsclient/nfsnode.h 2009/10/19 19:53:51
@@ -119,6 +119,7 @@
  struct vnode *n_vnode; /* associated vnode */
  struct vnode *n_dvp; /* parent vnode */
  int n_error; /* Save write error value */
+ int n_dmtime_ticks; /* When n_dmtime was last set */
  union {
  struct timespec nf_atim; /* Special file times */
  nfsuint64 nd_cookieverf; /* Cookie verifier (dir only) */


--
John Baldwin
_______________________________________________
freebsd-arch@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@..."

[Bruce Evans-4]

On Mon, 19 Oct 2009, John Baldwin wrote:

> This patch allows one to put an upper time limit on how long the NFS client
> should keep negative cache entries around for a given directory.  This is
> basically a safety belt as there are certain races with -ve entries that are
> not easily fixed (e.g. dealing with the low resolution of the directory
> modification timestamps).  However, timing out the entries would put a limit
> on how stale the client's cache entries could be.  My main question is do
> folks think this value should be tunable as a global sysctl or a per-mount
> option.  A sysctl is far easier to implement (and the acccess cache timeout
> is a sysctl).  However, the other namecache-related settings are all mount
> options, so a mount option might be more consistent.  Current rough patch is
> below:

One reason that I never committed my port of NetBSD's implementation of
negative cache entries for nfs is that I thought the timeout should be
a mount option but I didn't want to deal with the portability problems
from that.

Bruce
_______________________________________________
freebsd-arch@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@..."

[Rick Macklem]

On Thu, 22 Oct 2009, Bruce Evans wrote:

>
> One reason that I never committed my port of NetBSD's implementation of
> negative cache entries for nfs is that I thought the timeout should be
> a mount option but I didn't want to deal with the portability problems
> from that.
>
Just to clarify. Are you saying that, given no other system has such
a mount option, you think the sysctl variable is an ok solution or
that you think it should be a mount option and not a sysctl variable?

I'll admit that I'm biased because the sysctl variable is easier to
do (and I don't think many people will need to twiddle it). I suggested
it to John, mostly so that there was a way of disabling the -ve name
caching, for the rare case where it might be causing someone grief.
(That's what I think he meant by "safety belt".)

rick

_______________________________________________
freebsd-arch@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@..."

[Bruce Evans-4]

On Thu, 22 Oct 2009, Rick Macklem wrote:

> On Thu, 22 Oct 2009, Bruce Evans wrote:
>> One reason that I never committed my port of NetBSD's implementation of
>> negative cache entries for nfs is that I thought the timeout should be
>> a mount option but I didn't want to deal with the portability problems
>> from that.
>>
> Just to clarify. Are you saying that, given no other system has such
> a mount option, you think the sysctl variable is an ok solution or
> that you think it should be a mount option and not a sysctl variable?

No; I think it should be a mount option.

> I'll admit that I'm biased because the sysctl variable is easier to
> do (and I don't think many people will need to twiddle it). I suggested
> it to John, mostly so that there was a way of disabling the -ve name
> caching, for the rare case where it might be causing someone grief.
> (That's what I think he meant by "safety belt".)

Mount options are harder to add (especially using nmount) but that is
not intrinsic (in 4.4BSD, sysctls were harder to add, but now they
have more infrastructure so they are easier to add).  It would almost
be easier to abuse sysctls for mount options (they would have to select
the mount instance).

My version actually just uses a hackish sysctl vfs.nfs.negcache (default:
enabled) to enable the negative name cache.  I don't have any timeout
control for it.

Bruce
_______________________________________________
freebsd-arch@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@..."

[Rick Macklem]

On Sat, 24 Oct 2009, Bruce Evans wrote:

> On Thu, 22 Oct 2009, Rick Macklem wrote:
>
>> On Thu, 22 Oct 2009, Bruce Evans wrote:
>>> One reason that I never committed my port of NetBSD's implementation of
>>> negative cache entries for nfs is that I thought the timeout should be
>>> a mount option but I didn't want to deal with the portability problems
>>> from that.
>>>
>> Just to clarify. Are you saying that, given no other system has such
>> a mount option, you think the sysctl variable is an ok solution or
>> that you think it should be a mount option and not a sysctl variable?
>
> No; I think it should be a mount option.
>

Ok, so then what were your concerns w.r.t "portability problems"?
(It's not that much work to make it a mount option and since it looks
like the patch will be delayed until after 8.0 releases, I/or John
can make it a mount option but I didn't understand what you meant
by "portability options".)

[good stuff w.r.t. which is harder, deleted for brevity]
>
> My version actually just uses a hackish sysctl vfs.nfs.negcache (default:
> enabled) to enable the negative name cache.  I don't have any timeout
> control for it.
>
Yea, I saw it mainly as a way to disable -ve caching too. It just
happened that the sysctl variable could specify the timeout as well
as disable it (if set == 0). When it comes to a mount option, an
enable/disable flag is probably easier, but then there might be a
debate along the lines of "An option shouldn't be enabled by default...",
so I'm temped to stick with a setable timeout.

Basically the current code has an "infinite" timeout and no way of
disabling it, so it seemed to me that some sort of optional
timeout/disable would be useful, so I suggested that to John.

If no-one else weighs in on the other side, I head in the "mount option"
direction, rick

_______________________________________________
freebsd-arch@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@..."