Re: [PATCH] Access cache forgetting mode bits

[Andreas Gruenbacher <agruen@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2764 bytes --]

Hi,

your idea idea is good, but the patch is incorrect in at least two ways:
(1) it merges the cached results of successful and unsuccessful
requests, (2) [access("file", R_OK) == 0] and [access("file", W_OK) ==
0] does not imply [access("file", R_OK | W_OK) == 0].

Luckily, [access("file", R_OK | W_OK) == 0] does imply both
[access("file", R_OK) == 0] and [access("file", W_OK) == 0] though. We
could take this to our advantage, and optimize for the common case. How
about the attached patch? In the worst case, with "weird" permissions
this will double the number of RPCs needed, but this will not happen on
"real" systems.


The NFS access cache has a problem if mounted file systems are accessed
by multiple users: The results of only a single user are cached. When
another user comes along, cache trashing similar to what Steve has
observed will occur.

Steve, you seem to be looking into the nfsacl extensions as well. ACLs
are not cached on the client side, so operations like `ls -l' (which
need to check which files have ACLs) also produce lots of RPCs. I don't
see this as a significant problem at this point.


On Thu, 2003-08-28 at 18:39, Steve Dickson wrote:
> While debugging something else, I notice that every time
> I did a ls on a nfs mounted directory a wto ACCESS rpc was
> *always* happening (with a  2.4.21 kernel). Taking a closer
> look it turns out the access cache  was dropping previous
> access bits. Here is the scenario...
> 
> nfs_permission is called with mode = MAY_EXEC
> the cached value is Zero so an ACCESS rpc is done.
> after the rpc, the cached value is _set_ to MAY_EXEC
> 
> then nfs_permission is called with mode = MAY_READ
> the cached value is MAY_EXEC, so an ACCESS rpc is done.
> after the rpc, the cache value is _set_ to MAY_READ
> 
> again, nfs_permission is called with mode = MAY_EXEC
> this time the cached value is MAY_READ so _again_ an RPC is done
> after the rpc, the cached value is _set_ to MAY_EXEC
> 
> Its a vicious cycle...
> 
> So this patch changes the setting of the masked value
> to the or-ing of the masked value. This cause the
> previous bits to be remembered thus cutting down on
> the number of otw rpcs...
> 
> 
> SteveD.
> 
> ______________________________________________________________________
> 
> --- linux-2.4.21/fs/nfs/dir.c.diff	2003-08-28 12:13:22.000000000 -0400
> +++ linux-2.4.21/fs/nfs/dir.c	2003-08-28 12:13:47.000000000 -0400
> @@ -1278,7 +1278,7 @@ nfs_permission(struct inode *inode, int 
>  		if (cache->cred)
>  			put_rpccred(cache->cred);
>  		cache->cred = cred;
> -		cache->mask = mask;
> +		cache->mask |= mask;
>  		cache->err = error;
>  		return error;
>  	}


Cheers,
-- 
Andreas Gruenbacher <agruen@suse.de>
SuSE Labs, SuSE Linux AG <http://www.suse.de/>


[-- Attachment #2: nfsd-optimistic-caching.diff --]
[-- Type: text/plain, Size: 961 bytes --]

Index: linux-2.4.21/fs/nfs/dir.c
===================================================================
--- linux-2.4.21.orig/fs/nfs/dir.c	2003-08-28 16:50:33.000000000 +0200
+++ linux-2.4.21/fs/nfs/dir.c	2003-08-28 23:19:27.000000000 +0200
@@ -1266,6 +1266,13 @@ nfs_permission(struct inode *inode, int 
 			/* Is the mask a subset of an accepted mask? */
 			if ((cache->mask & mask) == mask)
 				goto out_cached;
+			/* Try with expanded credentials. */
+			error = NFS_PROTO(inode)->access(inode, cred,
+							 mask | cache->mask);
+			if (!error || !cache->mask) {
+				mask |= cache->mask;
+				goto rpc_done;
+			}
 	       	} else {
 			/* ...or is it a superset of a rejected mask? */
 			if ((cache->mask & mask) == cache->mask)
@@ -1273,6 +1280,7 @@ nfs_permission(struct inode *inode, int 
 		}
 	}
 	error = NFS_PROTO(inode)->access(inode, cred, mask);
+rpc_done:
 	if (!error || error == -EACCES) {
 		cache->jiffies = jiffies;
 		if (cache->cred)