Re: [NFS] Re: [PATCH][RFC] NFS: Improving the access cache

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Peter Staubach wrote:

2. Use a radix tree per inode.


Using radix trees actual makes things much simpler... Good idea, imo.


It does seem like a good idea, although just using the uid is not sufficient
to identify the correct access cache entry.  The group and groups list must
also be used.  Can the radix tree implementation support this?
We could use (nfsi + uid) as the index... but its not clear what that
would buys us... And the group and group list were never part of the
cache in the first place.... is this something you feel needs to be
added or am I just not understanding....

static int nfs_do_access(struct inode *inode, struct rpc_cred *cred, int mask)
@@ -1674,19 +1706,42 @@ static int nfs_do_access(struct inode *i

    status = nfs_access_get_cached(inode, cred, &cache);
    if (status == 0)
-        goto out;
+        goto out_nowait;
+
+    if (test_and_set_bit(NFS_INO_ACCESS, &NFS_FLAGS(inode))) {
+        status = nfs_wait_on_inode(inode, NFS_INO_ACCESS);
+        if (status < 0)
+            return status;
+
+        nfs_access_get_cached(inode, cred, &cache);
+        goto out_nowait;

You can't just go to out_nowait here, can you?  What happens if something
happened to the file while you were asleep?
point... after the nfs_wait_on_inode() call, nfs_access_get_cached()
will be retried ala...

static int nfs_do_access(struct inode *inode, struct rpc_cred *cred, int mask)
{
    struct nfs_access_entry cache;
    int status;

retry:
    status = nfs_access_get_cached(inode, cred, &cache);
    if (status == 0)
        goto out_nowait;

    if (test_and_set_bit(NFS_INO_ACCESS, &NFS_FLAGS(inode))) {
        status = nfs_wait_on_inode(inode, NFS_INO_ACCESS);
        if (status < 0)
            return status;

        goto retry;
    }



+static inline void nfs_zap_access_cache(struct inode *inode)
+{
+    struct nfs_access_entry *cache;
+
+ cache = radix_tree_delete(&NFS_I(inode)->access_cache, inode->i_uid);

Do you need to be hold access_lock here?
Yes...


+    if (cache) {
+        put_rpccred(cache->cred);
+        kfree(cache);
+    }

What happens if there was more than 1 entry in the cache?
I thought that since nfs_clear_inode() is called for
each and every inode that cache would drain "naturally"
but that turns out to be a bit brain dead... :-\

The problem is there does not seems to be a radix tree interface
that will simply destroy the tree... It appears you need to know
at least the beginning index and since the indexes in this tree are
not symmetrical I'm not sure that would even help...

Anybody.... Is there a way to destroy a radix tree without
known any of the indexes?

steved.

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux