On Fri, 2012-02-24 at 22:14 +0400, Stanislav Kinsbursky wrote:
> 23.02.2012 21:48, Fred Isaman пишет:
> > This needs to be looked at closely by someone more familiar with the
> > pipe code.
> >
> > It fixes an issue with the current nfs_for_next branch which causes a
> > chain of oopses on umount every time if sufficient CONFIG_* debug
> > options are set.
> >
> > A git-bisect shows that the problem was introduced by
> > commit c239d83b SUNRPC: split SUNPRC PipeFS dentry and private pipe data creation
> >
>
>
>
> Fred, thanks for the config.
> The problem is caused by destroying pipe data on NFS client umount after
> unlinking pipe dentry. This is valid approach, but it looks like idmap daemon
> holds dentry by eventfd.
> This is a race between idmap daemon release of this dentry and releasing of pipe
> data...
> I need some time to find out how to fix this properly.
>
How about something like the following (still untested) patch?
Cheers
Trond
----------------------------------------------------------------------------------------
>From 54baa44ba01559b9bb337d6877458dd6e12d36aa Mon Sep 17 00:00:00 2001
From: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
Date: Sun, 26 Feb 2012 18:25:56 -0500
Subject: [PATCH] SUNRPC: Ensure that the rpc_pipe lifetime matches that of
the dentry
It is expected that daemons may hold onto the rpc_pipefs dentries
even if the listeners have disappeared. Currently, we end up Oopsing
in this case, because the rpc_pipe is destroyed before the dentry.
This patch adds refcounting to the rpc_pipe, and ensures that it can
be safely shared, being destroyed only when the last dentry that
references it is destroyed.
Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
---
fs/nfs/blocklayout/blocklayout.c | 8 ++------
fs/nfs/idmap.c | 11 +++++------
include/linux/sunrpc/rpc_pipe_fs.h | 10 +++++++++-
net/sunrpc/auth_gss/auth_gss.c | 19 +++++++++----------
net/sunrpc/rpc_pipe.c | 25 ++++++++++++++++++++++---
5 files changed, 47 insertions(+), 26 deletions(-)
diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index 783ebd5..2639d53 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -1063,6 +1063,7 @@ static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
switch (event) {
case RPC_PIPEFS_MOUNT:
+ rpc_get_pipe_data(nn->bl_device_pipe);
dentry = nfs4blocklayout_register_sb(sb, nn->bl_device_pipe);
if (IS_ERR(dentry)) {
ret = PTR_ERR(dentry);
@@ -1096,7 +1097,6 @@ static struct dentry *nfs4blocklayout_register_net(struct net *net,
if (!pipefs_sb)
return NULL;
dentry = nfs4blocklayout_register_sb(pipefs_sb, pipe);
- rpc_put_sb_net(net);
return dentry;
}
@@ -1108,7 +1108,6 @@ static void nfs4blocklayout_unregister_net(struct net *net,
pipefs_sb = rpc_get_sb_net(net);
if (pipefs_sb) {
nfs4blocklayout_unregister_sb(pipefs_sb, pipe);
- rpc_put_sb_net(net);
}
}
@@ -1121,10 +1120,8 @@ static int nfs4blocklayout_net_init(struct net *net)
if (IS_ERR(nn->bl_device_pipe))
return PTR_ERR(nn->bl_device_pipe);
dentry = nfs4blocklayout_register_net(net, nn->bl_device_pipe);
- if (IS_ERR(dentry)) {
- rpc_destroy_pipe_data(nn->bl_device_pipe);
+ if (IS_ERR(dentry))
return PTR_ERR(dentry);
- }
nn->bl_device_pipe->dentry = dentry;
return 0;
}
@@ -1134,7 +1131,6 @@ static void nfs4blocklayout_net_exit(struct net *net)
struct nfs_net *nn = net_generic(net, nfs_net_id);
nfs4blocklayout_unregister_net(net, nn->bl_device_pipe);
- rpc_destroy_pipe_data(nn->bl_device_pipe);
nn->bl_device_pipe = NULL;
}
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index b5c6d8e..234c0a4 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -463,7 +463,6 @@ nfs_idmap_new(struct nfs_client *clp)
}
error = nfs_idmap_register(clp, idmap, pipe);
if (error) {
- rpc_destroy_pipe_data(pipe);
kfree(idmap);
return error;
}
@@ -508,7 +507,6 @@ nfs_idmap_delete(struct nfs_client *clp)
if (!idmap)
return;
nfs_idmap_unregister(clp, idmap->idmap_pipe);
- rpc_destroy_pipe_data(idmap->idmap_pipe);
clp->cl_idmap = NULL;
idmap_free_hashtable(&idmap->idmap_user_hash);
idmap_free_hashtable(&idmap->idmap_group_hash);
@@ -518,6 +516,7 @@ nfs_idmap_delete(struct nfs_client *clp)
static int __rpc_pipefs_event(struct nfs_client *clp, unsigned long event,
struct super_block *sb)
{
+ struct rpc_pipe *pipe = clp->cl_idmap->idmap_pipe;
int err = 0;
switch (event) {
@@ -525,14 +524,14 @@ static int __rpc_pipefs_event(struct nfs_client *clp, unsigned long event,
BUG_ON(clp->cl_rpcclient->cl_dentry == NULL);
err = __nfs_idmap_register(clp->cl_rpcclient->cl_dentry,
clp->cl_idmap,
- clp->cl_idmap->idmap_pipe);
+ rpc_get_pipe_data(pipe));
break;
case RPC_PIPEFS_UMOUNT:
- if (clp->cl_idmap->idmap_pipe) {
+ if (pipe) {
struct dentry *parent;
- parent = clp->cl_idmap->idmap_pipe->dentry->d_parent;
- __nfs_idmap_unregister(clp->cl_idmap->idmap_pipe);
+ parent = pipe->dentry->d_parent;
+ __nfs_idmap_unregister(pipe);
/*
* Note: This is a dirty hack. SUNRPC hook has been
* called already but simple_rmdir() call for the
diff --git a/include/linux/sunrpc/rpc_pipe_fs.h b/include/linux/sunrpc/rpc_pipe_fs.h
index 426ce6e..0cfaef0 100644
--- a/include/linux/sunrpc/rpc_pipe_fs.h
+++ b/include/linux/sunrpc/rpc_pipe_fs.h
@@ -35,6 +35,7 @@ struct rpc_pipe {
const struct rpc_pipe_ops *ops;
spinlock_t lock;
struct dentry *dentry;
+ atomic_t count;
};
struct rpc_inode {
@@ -86,12 +87,19 @@ extern void rpc_remove_cache_dir(struct dentry *);
extern int rpc_rmdir(struct dentry *dentry);
struct rpc_pipe *rpc_mkpipe_data(const struct rpc_pipe_ops *ops, int flags);
-void rpc_destroy_pipe_data(struct rpc_pipe *pipe);
extern struct dentry *rpc_mkpipe_dentry(struct dentry *, const char *, void *,
struct rpc_pipe *);
extern int rpc_unlink(struct dentry *);
extern int register_rpc_pipefs(void);
extern void unregister_rpc_pipefs(void);
+static inline struct rpc_pipe *rpc_get_pipe_data(struct rpc_pipe *pipe)
+{
+ atomic_inc(&pipe->count);
+ return pipe;
+}
+
+extern void rpc_put_pipe_data(struct rpc_pipe *pipe);
+
#endif
#endif
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index cb2e564..041d111 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -780,8 +780,11 @@ static int gss_pipes_dentries_create(struct rpc_auth *auth)
gss_auth->pipe[1]->dentry = rpc_mkpipe_dentry(clnt->cl_dentry,
"gssd",
clnt, gss_auth->pipe[1]);
- if (IS_ERR(gss_auth->pipe[1]->dentry))
- return PTR_ERR(gss_auth->pipe[1]->dentry);
+ if (IS_ERR(gss_auth->pipe[1]->dentry)) {
+ err = PTR_ERR(gss_auth->pipe[1]->dentry);
+ rpc_put_pipe_data(gss_auth->pipe[0]);
+ goto err;
+ }
gss_auth->pipe[0]->dentry = rpc_mkpipe_dentry(clnt->cl_dentry,
gss_auth->mech->gm_name,
clnt, gss_auth->pipe[0]);
@@ -793,6 +796,7 @@ static int gss_pipes_dentries_create(struct rpc_auth *auth)
err_unlink_pipe_1:
rpc_unlink(gss_auth->pipe[1]->dentry);
+err:
return err;
}
@@ -879,11 +883,12 @@ gss_create(struct rpc_clnt *clnt, rpc_authflavor_t flavor)
RPC_PIPE_WAIT_FOR_OPEN);
if (IS_ERR(gss_auth->pipe[0])) {
err = PTR_ERR(gss_auth->pipe[0]);
- goto err_destroy_pipe_1;
+ rpc_put_pipe_data(gss_auth->pipe[1]);
+ goto err_put_mech;
}
err = gss_pipes_dentries_create_net(clnt, auth);
if (err)
- goto err_destroy_pipe_0;
+ goto err_put_mech;
err = rpcauth_init_credcache(auth);
if (err)
goto err_unlink_pipes;
@@ -891,10 +896,6 @@ gss_create(struct rpc_clnt *clnt, rpc_authflavor_t flavor)
return auth;
err_unlink_pipes:
gss_pipes_dentries_destroy_net(clnt, auth);
-err_destroy_pipe_0:
- rpc_destroy_pipe_data(gss_auth->pipe[0]);
-err_destroy_pipe_1:
- rpc_destroy_pipe_data(gss_auth->pipe[1]);
err_put_mech:
gss_mech_put(gss_auth->mech);
err_free:
@@ -908,8 +909,6 @@ static void
gss_free(struct gss_auth *gss_auth)
{
gss_pipes_dentries_destroy_net(gss_auth->client, &gss_auth->rpc_auth);
- rpc_destroy_pipe_data(gss_auth->pipe[0]);
- rpc_destroy_pipe_data(gss_auth->pipe[1]);
gss_mech_put(gss_auth->mech);
kfree(gss_auth);
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 6873c9b..c7aad88 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -450,8 +450,19 @@ static int rpc_delete_dentry(const struct dentry *dentry)
return 1;
}
+static void rpc_dentry_iput(struct dentry *dentry, struct inode *inode)
+{
+ struct rpc_pipe *pipe = RPC_I(inode)->pipe;
+
+ RPC_I(inode)->pipe = NULL;
+ if (pipe != NULL)
+ rpc_put_pipe_data(pipe);
+ iput(inode);
+}
+
static const struct dentry_operations rpc_dentry_operations = {
.d_delete = rpc_delete_dentry,
+ .d_iput = rpc_dentry_iput,
};
static struct inode *
@@ -543,13 +554,20 @@ init_pipe(struct rpc_pipe *pipe)
pipe->ops = NULL;
spin_lock_init(&pipe->lock);
pipe->dentry = NULL;
+ atomic_set(&pipe->count, 1);
+}
+
+static void rpc_destroy_pipe_data(struct rpc_pipe *pipe)
+{
+ kfree(pipe);
}
-void rpc_destroy_pipe_data(struct rpc_pipe *pipe)
+void rpc_put_pipe_data(struct rpc_pipe *pipe)
{
- kfree(pipe);
+ if (atomic_dec_and_test(&pipe->count))
+ rpc_destroy_pipe_data(pipe);
}
-EXPORT_SYMBOL_GPL(rpc_destroy_pipe_data);
+EXPORT_SYMBOL_GPL(rpc_put_pipe_data);
struct rpc_pipe *rpc_mkpipe_data(const struct rpc_pipe_ops *ops, int flags)
{
@@ -842,6 +860,7 @@ out:
mutex_unlock(&dir->i_mutex);
return dentry;
out_err:
+ rpc_put_pipe_data(pipe);
dentry = ERR_PTR(err);
printk(KERN_WARNING "%s: %s() failed to create pipe %s/%s (errno = %d)\n",
__FILE__, __func__, parent->d_name.name, name,
--
1.7.7.6
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
