On Thu, 2007-07-26 at 17:13 +0200, Arnd Bergmann wrote:
> Unfortunately, you didn't answer my question. The observed problem is
> that the final kref_put gets called at a time where there are still
> references to the context in nfsi->open_files, and other threads
> therefore do get at them.
Actually, I thought I did: I said we need to grab the lock atomically
with the last put. See below.
> The patch holds the i_lock around the kref_put to prevent others
> from searching the list. Ugly, I know, but it seems that's the
> price you pay for using a kref in such unconventional ways, i.e.
> not counting every reference.
Really ugly. Here's an alternative that is a lot more palatable.
Trond
---------------------
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]>
---
fs/nfs/inode.c | 24 ++++++++----------------
include/linux/nfs_fs.h | 2 +-
2 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index bca6cdc..71a49c3 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -468,7 +468,7 @@ static struct nfs_open_context
*alloc_nfs_open_context(struct vfsmount *mnt, str
ctx->lockowner = current->files;
ctx->error = 0;
ctx->dir_cookie = 0;
- kref_init(&ctx->kref);
+ atomic_set(&ctx->count, 1);
}
return ctx;
}
@@ -476,21 +476,18 @@ static struct nfs_open_context
*alloc_nfs_open_context(struct vfsmount *mnt, str
struct nfs_open_context *get_nfs_open_context(struct nfs_open_context *ctx)
{
if (ctx != NULL)
- kref_get(&ctx->kref);
+ atomic_inc(&ctx->count);
return ctx;
}
-static void nfs_free_open_context(struct kref *kref)
+void put_nfs_open_context(struct nfs_open_context *ctx)
{
- struct nfs_open_context *ctx = container_of(kref,
- struct nfs_open_context, kref);
+ struct inode *inode = ctx->path.dentry->d_inode;
- if (!list_empty(&ctx->list)) {
- struct inode *inode = ctx->path.dentry->d_inode;
- spin_lock(&inode->i_lock);
- list_del(&ctx->list);
- spin_unlock(&inode->i_lock);
- }
+ 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->path, ctx->state, ctx->mode);
if (ctx->cred != NULL)
@@ -500,11 +497,6 @@ static void nfs_free_open_context(struct kref *kref)
kfree(ctx);
}
-void put_nfs_open_context(struct nfs_open_context *ctx)
-{
- kref_put(&ctx->kref, nfs_free_open_context);
-}
-
/*
* Ensure that mmap has a recent RPC credential for use when writing out
* shared pages
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 9ba4aec..157dcb0 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -71,7 +71,7 @@ struct nfs_access_entry {
struct nfs4_state;
struct nfs_open_context {
- struct kref kref;
+ atomic_t count;
struct path path;
struct rpc_cred *cred;
struct nfs4_state *state;
-
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/
