lkml.org 
[lkml]   [2007]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [patch] nfs: fix locking in nfs/inode.c in nfs_free_open_context
Date
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 <Trond.Myklebust@netapp.com>
> 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 <Trond.Myklebust@netapp.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2007-07-27 12:43    [W:0.052 / U:50.904 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site