NFSv3/v4: be more efficient when doing ACCESS RPC calls. Always ask

   for the full set of permissions.
Signed-off-by: Trond Myklebust <trond.myklebust@fys.uio.no>

fs/nfs/dir.c

@@ -1498,10 +1498,56 @@ static int nfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 	return error;
 }
 
-int
-nfs_permission(struct inode *inode, int mask, struct nameidata *nd)
+static int nfs_access_get_cached(struct inode *inode, struct rpc_cred *cred, struct nfs_access_entry *res)
+{
+	struct nfs_access_entry *cache = &NFS_I(inode)->cache_access;
+
+	if (cache->cred != cred
+			|| time_after(jiffies, cache->jiffies + NFS_ATTRTIMEO(inode))
+			|| (NFS_FLAGS(inode) & NFS_INO_INVALID_ATTR))
+		return -ENOENT;
+	memcpy(res, cache, sizeof(*res));
+	return 0;
+}
+
+static void nfs_access_add_cache(struct inode *inode, struct nfs_access_entry *set)
+{
+	struct nfs_access_entry *cache = &NFS_I(inode)->cache_access;
+
+	if (cache->cred != set->cred) {
+		if (cache->cred)
+			put_rpccred(cache->cred);
+		cache->cred = get_rpccred(set->cred);
+	}
+	cache->jiffies = set->jiffies;
+	cache->mask = set->mask;
+}
+
+static int nfs_do_access(struct inode *inode, struct rpc_cred *cred, int mask)
+{
+	struct nfs_access_entry cache;
+	int status;
+
+	status = nfs_access_get_cached(inode, cred, &cache);
+	if (status == 0)
+		goto out;
+
+	/* Be clever: ask server to check for all possible rights */
+	cache.mask = MAY_EXEC | MAY_WRITE | MAY_READ;
+	cache.cred = cred;
+	cache.jiffies = jiffies;
+	status = NFS_PROTO(inode)->access(inode, &cache);
+	if (status != 0)
+		return status;
+	nfs_access_add_cache(inode, &cache);
+out:
+	if ((cache.mask & mask) == mask)
+		return 0;
+	return -EACCES;
+}
+
+int nfs_permission(struct inode *inode, int mask, struct nameidata *nd)
 {
-	struct nfs_access_cache *cache = &NFS_I(inode)->cache_access;
 	struct rpc_cred *cred;
 	int mode = inode->i_mode;
 	int res;
@@ -1542,24 +1588,7 @@ nfs_permission(struct inode *inode, int mask, struct nameidata *nd)
 		goto out_notsup;
 
 	cred = rpcauth_lookupcred(NFS_CLIENT(inode)->cl_auth, 0);
-	if (cache->cred == cred
-	    && time_before(jiffies, cache->jiffies + NFS_ATTRTIMEO(inode))
-	    && !(NFS_FLAGS(inode) & NFS_INO_INVALID_ATTR)) {
-		if (!(res = cache->err)) {
-			/* Is the mask a subset of an accepted mask? */
-			if ((cache->mask & mask) == mask)
-				goto out;
-		} else {
-			/* ...or is it a superset of a rejected mask? */
-			if ((cache->mask & mask) == cache->mask)
-				goto out;
-		}
-	}
-
-	res = NFS_PROTO(inode)->access(inode, cred, mask);
-	if (!res || res == -EACCES)
-		goto add_cache;
-out:
+	res = nfs_do_access(inode, cred, mask);
 	put_rpccred(cred);
 	unlock_kernel();
 	return res;
@@ -1568,15 +1597,6 @@ nfs_permission(struct inode *inode, int mask, struct nameidata *nd)
 	res = vfs_permission(inode, mask);
 	unlock_kernel();
 	return res;
-add_cache:
-	cache->jiffies = jiffies;
-	if (cache->cred)
-		put_rpccred(cache->cred);
-	cache->cred = cred;
-	cache->mask = mask;
-	cache->err = res;
-	unlock_kernel();
-	return res;
 }
 
 /*

fs/nfs/nfs3proc.c

@@ -164,8 +164,7 @@ nfs3_proc_lookup(struct inode *dir, struct qstr *name,
 	return status;
 }
 
-static int
-nfs3_proc_access(struct inode *inode, struct rpc_cred *cred, int mode)
+static int nfs3_proc_access(struct inode *inode, struct nfs_access_entry *entry)
 {
 	struct nfs_fattr	fattr;
 	struct nfs3_accessargs	arg = {
@@ -178,8 +177,9 @@ nfs3_proc_access(struct inode *inode, struct rpc_cred *cred, int mode)
 		.rpc_proc	= &nfs3_procedures[NFS3PROC_ACCESS],
 		.rpc_argp	= &arg,
 		.rpc_resp	= &res,
-		.rpc_cred	= cred
+		.rpc_cred	= entry->cred
 	};
+	int mode = entry->mask;
 	int status;
 
 	dprintk("NFS call  access\n");
@@ -200,10 +200,16 @@ nfs3_proc_access(struct inode *inode, struct rpc_cred *cred, int mode)
 	}
 	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)
-		status = -EACCES;
+	if (status == 0) {
+		entry->mask = 0;
+		if (res.access & NFS3_ACCESS_READ)
+			entry->mask |= MAY_READ;
+		if (res.access & (NFS3_ACCESS_MODIFY | NFS3_ACCESS_EXTEND | NFS3_ACCESS_DELETE))
+			entry->mask |= MAY_WRITE;
+		if (res.access & (NFS3_ACCESS_LOOKUP|NFS3_ACCESS_EXECUTE))
+			entry->mask |= MAY_EXEC;
+	}
+	dprintk("NFS reply access, status = %d\n", status);
 	return status;
 }
 

fs/nfs/nfs4proc.c

@@ -734,9 +734,8 @@ static int nfs4_proc_lookup(struct inode *dir, struct qstr *name,
 	return nfs4_map_errors(status);
 }
 
-static int nfs4_proc_access(struct inode *inode, struct rpc_cred *cred, int mode)
+static int nfs4_proc_access(struct inode *inode, struct nfs_access_entry *entry)
 {
-	int			status;
 	struct nfs4_accessargs args = {
 		.fh = NFS_FH(inode),
 	};
@@ -745,8 +744,10 @@ static int nfs4_proc_access(struct inode *inode, struct rpc_cred *cred, int mode
 		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_ACCESS],
 		.rpc_argp = &args,
 		.rpc_resp = &res,
-		.rpc_cred = cred,
+		.rpc_cred = entry->cred,
 	};
+	int mode = entry->mask;
+	int status;
 
 	/*
 	 * Determine which access bits we want to ask for...
@@ -758,8 +759,7 @@ static int nfs4_proc_access(struct inode *inode, struct rpc_cred *cred, int mode
 			args.access |= NFS4_ACCESS_MODIFY | NFS4_ACCESS_EXTEND | NFS4_ACCESS_DELETE;
 		if (mode & MAY_EXEC)
 			args.access |= NFS4_ACCESS_LOOKUP;
-	}
-	else {
+	} else {
 		if (mode & MAY_WRITE)
 			args.access |= NFS4_ACCESS_MODIFY | NFS4_ACCESS_EXTEND;
 		if (mode & MAY_EXEC)
@@ -767,11 +767,13 @@ static int nfs4_proc_access(struct inode *inode, struct rpc_cred *cred, int mode
 	}
 	status = rpc_call_sync(NFS_CLIENT(inode), &msg, 0);
 	if (!status) {
-		if (args.access != res.supported) {
-			printk(KERN_NOTICE "NFS: server didn't support all access bits!\n");
-			status = -ENOTSUPP;
-		} else if ((args.access & res.access) != args.access)
-			status = -EACCES;
+		entry->mask = 0;
+		if (res.access & NFS4_ACCESS_READ)
+			entry->mask |= MAY_READ;
+		if (res.access & (NFS4_ACCESS_MODIFY | NFS4_ACCESS_EXTEND | NFS4_ACCESS_DELETE))
+			entry->mask |= MAY_WRITE;
+		if (res.access & (NFS4_ACCESS_LOOKUP|NFS4_ACCESS_EXECUTE))
+			entry->mask |= MAY_EXEC;
 	}
 	return nfs4_map_errors(status);
 }

include/linux/nfs_fs.h

@@ -75,13 +75,12 @@
 #ifdef __KERNEL__
 
 /*
- * NFSv3 Access mode cache
+ * NFSv3/v4 Access mode cache entry
  */
-struct nfs_access_cache {
+struct nfs_access_entry {
 	unsigned long		jiffies;
 	struct rpc_cred *	cred;
 	int			mask;
-	int			err;
 };
 
 /*
@@ -137,7 +136,7 @@ struct nfs_inode {
 	 */
 	atomic_t		data_updates;
 
-	struct nfs_access_cache	cache_access;
+	struct nfs_access_entry	cache_access;
 
 	/*
 	 * This is the cookie verifier used for NFSv3 readdir

include/linux/nfs_xdr.h

@@ -657,6 +657,8 @@ struct nfs_write_data {
 	void (*complete) (struct nfs_write_data *, int);
 };
 
+struct nfs_access_entry;
+
 /*
  * RPC procedure vector for NFSv2/NFSv3 demuxing
  */
@@ -672,7 +674,7 @@ struct nfs_rpc_ops {
 			    struct iattr *);
 	int	(*lookup)  (struct inode *, struct qstr *,
 			    struct nfs_fh *, struct nfs_fattr *);
-	int	(*access)  (struct inode *, struct rpc_cred *, int);
+	int	(*access)  (struct inode *, struct nfs_access_entry *);
 	int	(*readlink)(struct inode *, struct page *);
 	int	(*read)    (struct nfs_read_data *, struct file *);
 	int	(*write)   (struct nfs_write_data *, struct file *);