On Thursday 26 July 2007, Trond Myklebust wrote:
> Really ugly. Here's an alternative that is a lot more palatable.

Yes, much better than mine.

> ---------------------
> From: Trond Myklebust <[EMAIL PROTECTED]>
> Date: Thu, 26 Jul 2007 12:06:17 -0400
> NFS: Fix put_nfs_open_context
> 
> We need to grab the inode->i_lock atomically with the last reference put in
> order to remove the open context that is being freed from the
> nfsi->open_files list.
> 
> Fix by converting the kref to a standard atomic counter and then using
> atomic_dec_and_lock()...
> 
> Signed-off-by: Trond Myklebust <[EMAIL PROTECTED]>

Acked-by: Arnd Bergmann <[EMAIL PROTECTED]>

>From the bit of research I did on the bug yesterday, it seems
that the race has been in there for ages, but may have become easier
to hit with the change to kref after 2.6.22. I don't really
understand why we didn't hit it in RHEL5/2.6.18 with the same
test case, but in Fedora 7/2.6.21.

Should a patch like the one below (I said like, you know you
can't trust my patches for your code ;-) be put into 2.6.22.x
and updated distro kernels?

        Arnd <><

--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -482,21 +482,19 @@ struct nfs_open_context *get_nfs_open_context(struct 
nfs_open_context *ctx)
 
 void put_nfs_open_context(struct nfs_open_context *ctx)
 {
-       if (atomic_dec_and_test(&ctx->count)) {
-               if (!list_empty(&ctx->list)) {
-                       struct inode *inode = ctx->dentry->d_inode;
-                       spin_lock(&inode->i_lock);
-                       list_del(&ctx->list);
-                       spin_unlock(&inode->i_lock);
-               }
-               if (ctx->state != NULL)
-                       nfs4_close_state(ctx->state, ctx->mode);
-               if (ctx->cred != NULL)
-                       put_rpccred(ctx->cred);
-               dput(ctx->dentry);
-               mntput(ctx->vfsmnt);
-               kfree(ctx);
-       }
+       struct inode *inode = ctx->dentry->d_inode;
+
+       if (!atomic_dec_and_lock(&ctx->count, &inode->i_lock))
+               return;
+       list_del(&ctx->list);
+       spin_unlock(&inode->i_lock);
+       if (ctx->state != NULL)
+               nfs4_close_state(ctx->state, ctx->mode);
+       if (ctx->cred != NULL)
+               put_rpccred(ctx->cred);
+       dput(ctx->dentry);
+       mntput(ctx->vfsmnt);
+       kfree(ctx);
 }
 
 /*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to