--- 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;
}
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 <[email protected]>
SuSE Labs, SuSE Linux AG <http://www.suse.de/>
Andreas Gruenbacher wrote:
>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].
>
Good point!
>[....] 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.
>
Yes... your patch does seem to work with "normal" cases.... which is
probably the
best we can hope for...
>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.
>
>
True... its too bad the cred cache doesn't use uids and gids....
>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.
>
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
--- 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;
}
On Sun, 2003-08-31 at 22:33, Steve Dickson wrote:
> 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
Individually you mean? That's not at all the case in general.
> [...] even with Andreas patch's there are at least two rpcs for one access call....
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.
> 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?
You have again attached your first, buggy patch.
Cheers,
--
Andreas Gruenbacher <[email protected]>
SuSE Labs, SuSE Linux AG <http://www.suse.de/>
--- linux-2.4.21/fs/nfs/dir.c.orig 2003-08-31 13:04:30.000000000 -0400
+++ linux-2.4.21/fs/nfs/dir.c 2003-08-31 13:07:49.000000000 -0400
@@ -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);
--- linux-2.4.21/fs/nfs/nfs3proc.c.orig 2003-08-30 16:30:57.000000000 -0400
+++ linux-2.4.21/fs/nfs/nfs3proc.c 2003-08-31 12:58:11.000000000 -0400
@@ -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;
}
>>>>> " " == Steve Dickson <[email protected]> writes:
>>> Thoughts?
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
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%