Access cache forgetting mode bits

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.

@@ -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;
 	}

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.

Andreas Gruenbacher <[email protected]>
SuSE Labs, SuSE Linux AG <http://www.suse.de/>

Index: linux-2.4.21/fs/nfs/dir.c
===================================================================
@@ -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)

Good point!

Yes... your patch does seem to work with "normal" cases.... which is probably the best we can hope for...

True... its too bad the cred cache doesn't use uids and gids....

Caches are always a good idea wrt to NFS... but I guess only time will tell wrt nfsacls...

SteveD.

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

Hello,

I'm thinking there might even be a better way to cache these bits...

It seems the OS calls nfs_permission() to test each permission bit which in turn causes an otw access rpc for every call (assuming the same person and same bit is *not* asked for ) and even with Andreas patch's there are at least two rpcs for one access call....

Now what if on the first access call, we get *all* of the access bits from the server and cache them locally (i.e. in cache->mask). That way when nfs_permission() is called again, all if the needed info is there... The attached patch does just that and it really seems to cut down on the amount access rpc needed...

Thoughts?

SteveD.

@@ -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;
 	}

Individually you mean? That's not at all the case in general.

No, two different, consecutice permission checks will result in two RPCs.

It is possible to optimize another common case: Directory accesses are seldom denied, and users are usually granted both r and x at once. The attached patch optimizes that; it's the same pattern as in my first patch. I'm in doubt whether this optimization is worth it, though.

For non-directories, I don't think a common enough case exists that would deserve optimization.

You have again attached your first, buggy patch.

Cheers,

Andreas Gruenbacher <[email protected]>
SuSE Labs, SuSE Linux AG <http://www.suse.de/>

Index: linux-2.4.21/fs/nfs/dir.c
===================================================================
@@ -1278,6 +1278,13 @@ nfs_permission(struct inode *inode, int 
 			if ((cache->mask & mask) == cache->mask)
 				goto out_cached;
 		}
+	} else if (S_ISDIR(mode)) {
+		int try_mask = mask | S_IROTH | S_IXOTH;
+		error = NFS_PROTO(inode)->access(inode, cred, try_mask);
+		if (!error || mask == try_mask) {
+			mask = try_mask;
+			goto rpc_done;
+		}
 	}
 	error = NFS_PROTO(inode)->access(inode, cred, mask);
 rpc_done:

Yes... So at times there will be more than one bit set in the mask? I know that's how it works in other OS but I was not sure with linux...

Oops... I hate when that happens....

SteveD.

@@ -1259,17 +1259,21 @@ nfs_permission(struct inode *inode, int 
 
 	if ((server->flags & NFS_MOUNT_NOACL) || !NFS_PROTO(inode)->access)
 		goto out_notsup;
+	/*
+	 * See if we can use the cache
+	 */
 	cred = rpcauth_lookupcred(NFS_CLIENT(inode)->cl_auth, 0);
 	if (cache->cred == cred
 	    && time_before(jiffies, cache->jiffies + NFS_ATTRTIMEO(inode))) {
-		if (!cache->err) {
-			/* Is the mask a subset of an accepted mask? */
-			if ((cache->mask & mask) == mask)
-				goto out_cached;
-	       	} else {
-			/* ...or is it a superset of a rejected mask? */
-			if ((cache->mask & mask) == cache->mask)
-				goto out_cached;
+		/* Is the mask a subset of an accepted mask? */
+		if ((cache->mask & mask) == mask) {
+			cache->err  = 0;
+			goto out_cached;
+		} 
+		/* ...or is it a superset of a rejected mask? */
+		if ((~cache->mask & mask) == mask) {
+			cache->err  = -EACCES;
+			goto out_cached;
 		}
 	}
 	error = NFS_PROTO(inode)->access(inode, cred, mask);

@@ -1278,8 +1282,6 @@ nfs_permission(struct inode *inode, int 
 		if (cache->cred)
 			put_rpccred(cache->cred);
 		cache->cred = cred;
-		cache->mask = mask;
-		cache->err = error;
 		return error;
 	}
 	put_rpccred(cred);

@@ -121,10 +121,13 @@ static int
 nfs3_proc_access(struct inode *inode, struct rpc_cred *cred, int mode)
 {
 	struct nfs_fattr	fattr;
+	struct nfs_access_cache *cache = &NFS_I(inode)->cache_access;
 	struct nfs3_accessargs	arg = { NFS_FH(inode), 0 };
 	struct nfs3_accessres	res = { &fattr, 0 };
 	struct rpc_message msg = { NFS3PROC_ACCESS, &arg, &res, cred };
 	int	status;
+	static int may_write, may_exec;
+	__u32 access;
 
 	dprintk("NFS call  access\n");
 	fattr.valid = 0;

@@ -134,20 +137,45 @@ nfs3_proc_access(struct inode *inode, st
 	if (S_ISDIR(inode->i_mode)) {
 		if (mode & MAY_WRITE)
 			arg.access |= NFS3_ACCESS_MODIFY | NFS3_ACCESS_EXTEND | NFS3_ACCESS_DELETE;
+		may_write = NFS3_ACCESS_MODIFY| NFS3_ACCESS_EXTEND | NFS3_ACCESS_DELETE;
+
 		if (mode & MAY_EXEC)
 			arg.access |= NFS3_ACCESS_LOOKUP;
+		may_exec = NFS3_ACCESS_LOOKUP;
+
 	} else {
 		if (mode & MAY_WRITE)
 			arg.access |= NFS3_ACCESS_MODIFY | NFS3_ACCESS_EXTEND;
+		may_write = NFS3_ACCESS_MODIFY | NFS3_ACCESS_EXTEND;
+
 		if (mode & MAY_EXEC)
 			arg.access |= NFS3_ACCESS_EXECUTE;
+		may_exec = NFS3_ACCESS_EXECUTE;
 	}
+	/*
+	 * Save off whats needed and then ask for all of the mode bits
+	 * which will be cached
+	 */
+	access = arg.access;
+	arg.access = (NFS3_ACCESS_EXECUTE|NFS3_ACCESS_READ|NFS3_ACCESS_MODIFY| 
+		NFS3_ACCESS_EXTEND|NFS3_ACCESS_DELETE|NFS3_ACCESS_LOOKUP);
+
 	status = rpc_call_sync(NFS_CLIENT(inode), &msg, 0);
 	nfs_refresh_inode(inode, &fattr);
 	dprintk("NFS reply access\n");
 
-	if (status == 0 && (arg.access & res.access) != arg.access)
+	if (status == 0 && (access & res.access) != access)
 		status = -EACCES;
+
+	if ((!status || status == -EACCES)) {
+		cache->mask = 0;
+		if (res.access & NFS3_ACCESS_READ)
+			cache->mask |= MAY_READ;
+		if (res.access & may_write)
+			cache->mask |= MAY_WRITE;
+		if (res.access & may_exec)
+			cache->mask |= MAY_EXEC;
+	}
 	return status;
 }
 

What are you using to measure performance here? It would be nice to see some figures to demonstrate the improvement.

Cheers,
Trond

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

Hi Trond,

Good point... If figured the Connectathon test suite (cthon03) would be good since it has a large variety of tests and easy to run... But if there are other tests that would be good just let me know...

Running cthon03 three times and using nfsstat to count the total number of rpc and access here are the numbers...

Run 1 total calls access calls

Original Code:   30230       1755    5%
Andreas's Patch: 30181       1676    5%

Latest Patch: 29671 1141 3%

Run 2 total calls access calls

Original Code:   30064       1755    5%
Andreas's Patch: 30176       1676    5%

Latest Patch: 29978 1141 3%

Run 3 total calls access calls

Original Code:   30286      1755    5%
Andreas's Patch: 29978      1676    5%

Latest Patch: 29410 1141 3%

Note: It is not a cut/paste error that each test run had the same amount of access calls... the only number that really fluctuated was the number of getattrs.... that's why I only ran the tests three times... not much seem to be changing...

I've attached the raw data pruned of the v2 stats...

SteveD.

Client rpc stats:

calls      retrans    authrefrsh
30230      15         0       

Client nfs v3:
null getattr setattr lookup access readlink
0 0% 1587 5% 1199 3% 2428 8% 1755 5% 250 0%
read write create mkdir symlink mknod
3976 13% 12304 40% 890 2% 175 0% 250 0% 0 0%
remove rmdir rename link readdir readdirplus
1389 4% 175 0% 407 1% 250 0% 0 0% 1121 3%
fsstat fsinfo pathconf commit
1500 4% 1 0% 0 0% 573 1%

Client rpc stats:

calls      retrans    authrefrsh
30064      28         0       

Client nfs v3:
null getattr setattr lookup access readlink
0 0% 1493 4% 1199 3% 2362 7% 1755 5% 250 0%
read write create mkdir symlink mknod
3976 13% 12304 40% 890 2% 175 0% 250 0% 0 0%
remove rmdir rename link readdir readdirplus
1389 4% 175 0% 404 1% 250 0% 0 0% 1121 3%
fsstat fsinfo pathconf commit
1500 4% 1 0% 0 0% 570 1%

Client rpc stats:

calls      retrans    authrefrsh
30286      24         0       

Client nfs v3:
null getattr setattr lookup access readlink
0 0% 1520 5% 1199 3% 2554 8% 1755 5% 250 0%
read write create mkdir symlink mknod
3976 13% 12304 40% 890 2% 175 0% 250 0% 0 0%
remove rmdir rename link readdir readdirplus
1389 4% 175 0% 406 1% 250 0% 0 0% 1121 3%
fsstat fsinfo pathconf commit
1500 4% 1 0% 0 0% 571 1%

Client rpc stats:

calls      retrans    authrefrsh
30181      24         0       

Client nfs v3:
null getattr setattr lookup access readlink
0 0% 1478 4% 1199 3% 2567 8% 1676 5% 250 0%
read write create mkdir symlink mknod
3977 13% 12305 40% 890 2% 175 0% 250 0% 0 0%
remove rmdir rename link readdir readdirplus
1389 4% 175 0% 405 1% 250 0% 0 0% 1121 3%
fsstat fsinfo pathconf commit
1500 4% 1 0% 0 0% 573 1%

Client rpc stats:

calls      retrans    authrefrsh
30176      24         0       

Client nfs v3:
null getattr setattr lookup access readlink
0 0% 1574 5% 1199 3% 2477 8% 1676 5% 250 0%
read write create mkdir symlink mknod
3976 13% 12304 40% 890 2% 175 0% 250 0% 0 0%
remove rmdir rename link readdir readdirplus
1389 4% 175 0% 401 1% 250 0% 0 0% 1121 3%
fsstat fsinfo pathconf commit
1500 4% 1 0% 0 0% 568 1%

Client rpc stats:

calls      retrans    authrefrsh
29978      8          0       

Client nfs v3:
null getattr setattr lookup access readlink
0 0% 1515 5% 1199 3% 2335 7% 1676 5% 250 0%
read write create mkdir symlink mknod
3976 13% 12304 41% 890 2% 175 0% 250 0% 0 0%
remove rmdir rename link readdir readdirplus
1389 4% 175 0% 403 1% 250 0% 0 0% 1121 3%
fsstat fsinfo pathconf commit
1500 5% 1 0% 0 0% 569 1%

Client rpc stats:

calls      retrans    authrefrsh
29671      9          0       

Client nfs v3:
null getattr setattr lookup access readlink
0 0% 1644 5% 1199 4% 2432 8% 1141 3% 250 0%
read write create mkdir symlink mknod
3976 13% 12304 41% 890 2% 175 0% 250 0% 0 0%
remove rmdir rename link readdir readdirplus
1389 4% 175 0% 403 1% 250 0% 0 0% 1121 3%
fsstat fsinfo pathconf commit
1500 5% 1 0% 0 0% 571 1%

Client rpc stats:

calls      retrans    authrefrsh
29761      10         0       

Client nfs v3:
null getattr setattr lookup access readlink
0 0% 1682 5% 1199 4% 2484 8% 1141 3% 250 0%
read write create mkdir symlink mknod
3976 13% 12303 41% 890 2% 175 0% 250 0% 0 0%
remove rmdir rename link readdir readdirplus
1389 4% 175 0% 405 1% 250 0% 0 0% 1121 3%
fsstat fsinfo pathconf commit
1500 5% 1 0% 0 0% 570 1%

Client rpc stats:

calls      retrans    authrefrsh
29410      23         0       

Client nfs v3:
null getattr setattr lookup access readlink
0 0% 1492 5% 1199 4% 2318 7% 1141 3% 250 0%
read write create mkdir symlink mknod
3976 13% 12304 41% 890 3% 175 0% 250 0% 0 0%
remove rmdir rename link readdir readdirplus
1389 4% 175 0% 403 1% 250 0% 0 0% 1121 3%
fsstat fsinfo pathconf commit
1500 5% 1 0% 0 0% 576 1%