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�����٥