[PATCH] NFSv3: cached permissions subset enhancement, v2

[Joe Korty]

On Thu, Aug 28, 2008 at 12:03:47PM -0400, Myklebust, Trond wrote:

> This isn't a good solution. The correct thing to do here is to resend
> the request for just those permissions that the VFS requested.

Is there any chance that the cached permissions are shared amoung
multiple open(2)'s of the same file?  If sharing happens then we
really need to cache all of MAY_EXEC | MAY_WRITE | MAY_READ, rather
than just that subset asked for by the open(2) that happens to
be the one filling in the cache.

In any case, I've rewritten the patch per the above suggestion, and
in doing so noted an embarrasing bug in the original posting, one that
happened to touch all the values that it needed to, by accident:

-        for (i = 0; i <= MAY_READ; i++) {
-            cache.mask = 1 << i;
+	 for (i = 1; i <= MAY_READ; i *= 2) {
+            cache.mask = i;

     ---------- cut here ---------

[NFSv3] cached permissions subset enhancement.

NFSv3 allows file permissions to be cached on the client
side.  The Linux client, when fetching these permissions,
makes the request for all permissions (rwx) in a single
operation.  However, for some NFSv3 server implementations
(the only one currently known is PowerMAX OS), an incoming
request for all rwx permissions in a single request,
when all are not actually set in the file, returns an
NFSERR_ACCES failure code to the client, rather than
the subset of permissions actually available for that file.

This patch modifies the Linux client side code to
individually fetch the r, w, and x permissions (combining
these for storage into the cache), if the original
single-request method fails.

This slower method will not affect performance of those
client/server pairs for which the original single-request
method works.  In particular there is no performance
penalty for linux/linux NFSv3 connections.

Author: Linda Dunaphant <ldunaphant@xxxxxxxxxxx>
Reworked-by: Joe Korty <joe.korty@xxxxxxxx>
Signed-off-by: Joe Korty <joe.korty@xxxxxxxx>

Index: a/fs/nfs/dir.c
===================================================================
--- a.orig/fs/nfs/dir.c	2008-08-27 11:31:43.000000000 -0400
+++ a/fs/nfs/dir.c	2008-08-28 14:12:52.000000000 -0400
@@ -1902,8 +1902,34 @@
 	cache.cred = cred;
 	cache.jiffies = jiffies;
 	status = NFS_PROTO(inode)->access(inode, &cache);
-	if (status != 0)
+	if (status == -NFSERR_ACCES) {
+		/*
+		 * Try again - one mode at a time & combine at the end
+		 */
+		int cmask = 0;
+
+		if (mask & MAY_EXEC) {
+			cache.mask = MAY_EXEC;
+			if (!NFS_PROTO(inode)->access(inode, &cache))
+				cmask |= cache.mask;
+		}
+		if (mask & MAY_WRITE) {
+			cache.mask = MAY_WRITE;
+			if (!NFS_PROTO(inode)->access(inode, &cache))
+				cmask |= cache.mask;
+		}
+		if (mask & MAY_READ) {
+			cache.mask = MAY_READ;
+			if (!NFS_PROTO(inode)->access(inode, &cache))
+				cmask |= cache.mask;
+		}
+		if (!cmask)
+			return -NFSERR_ACCES;
+
+		cache.mask = cmask;
+	} else if (status)
 		return status;
+
 	nfs_access_add_cache(inode, &cache);
 out:
 	if ((cache.mask & mask) == mask)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/