diff mbox

[3.8.y.z,extended,stable] Patch "SUNRPC: fix races on PipeFS UMOUNT notifications" has been added to staging queue

Message ID 1374015236-27153-1-git-send-email-kamal@canonical.com
State New
Headers show

Commit Message

Kamal Mostafa July 16, 2013, 10:53 p.m. UTC
This is a note to let you know that I have just added a patch titled

    SUNRPC: fix races on PipeFS UMOUNT notifications

to the linux-3.8.y-queue branch of the 3.8.y.z extended stable tree 
which can be found at:

 http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.8.y-queue

This patch is scheduled to be released in version 3.8.13.5.

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.8.y.z tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Kamal

------

From 26655857295527252a3d6606b4cb248580b7843d Mon Sep 17 00:00:00 2001
From: Stanislav Kinsbursky <skinsbursky@parallels.com>
Date: Wed, 26 Jun 2013 10:15:14 +0400
Subject: SUNRPC: fix races on PipeFS UMOUNT notifications

commit adb6fa7ffe9031857ec14b8aab75c9ab65556cbc upstream.

CPU#0                                   CPU#1
-----------------------------           -----------------------------
rpc_kill_sb
sn->pipefs_sb = NULL                    rpc_release_client
(UMOUNT_EVENT)                          rpc_free_auth
rpc_pipefs_event
rpc_get_client_for_event
!atomic_inc_not_zero(cl_count)
<skip the client>
                                        atomic_inc(cl_count)
                                        rpc_free_client
                                        rpc_clnt_remove_pipedir
                                        <skip client dir removing>

To fix this, this patch does the following:

1) Calls RPC_PIPEFS_UMOUNT notification with sn->pipefs_sb_lock being held.
2) Removes SUNRPC client from the list AFTER pipes destroying.
3) Doesn't hold RPC client on notification: if client in the list, then it
can't be destroyed while sn->pipefs_sb_lock in hold by notification caller.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
---
 net/sunrpc/clnt.c     | 5 +----
 net/sunrpc/rpc_pipe.c | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

--
1.8.1.2

Comments

Trond Myklebust July 16, 2013, 11:16 p.m. UTC | #1
On Tue, 2013-07-16 at 15:53 -0700, Kamal Mostafa wrote:
> This is a note to let you know that I have just added a patch titled
> 
>     SUNRPC: fix races on PipeFS UMOUNT notifications
> 
> to the linux-3.8.y-queue branch of the 3.8.y.z extended stable tree 
> which can be found at:
> 
>  http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.8.y-queue
> 
> This patch is scheduled to be released in version 3.8.13.5.
> 
> If you, or anyone else, feels it should not be added to this tree, please 
> reply to this email.
> 
> For more information about the 3.8.y.z tree, see
> https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable
> 
> Thanks.
> -Kamal
> 
> ------
> 
> From 26655857295527252a3d6606b4cb248580b7843d Mon Sep 17 00:00:00 2001
> From: Stanislav Kinsbursky <skinsbursky@parallels.com>
> Date: Wed, 26 Jun 2013 10:15:14 +0400
> Subject: SUNRPC: fix races on PipeFS UMOUNT notifications
> 
> commit adb6fa7ffe9031857ec14b8aab75c9ab65556cbc upstream.

Hi,

Can we please hold off on this patch and the other PipeFS followups for
now? It isn't completely clear that people are hitting these problems in
the wild, and the patch itself introduces bugs which still aren't 100%
fixed upstream (I have 1 more patch queued to deal with the aftermath).

Let's just wait until we have definite proof that people are Oopsing,
and we've addressed all the issues in the upstream kernel.

Thanks,
  Trond
Kamal Mostafa July 16, 2013, 11:40 p.m. UTC | #2
On Tue, 2013-07-16 at 23:16 +0000, Myklebust, Trond wrote:
> On Tue, 2013-07-16 at 15:53 -0700, Kamal Mostafa wrote:
> > This is a note to let you know that I have just added a patch titled
> > 
> >     SUNRPC: fix races on PipeFS UMOUNT notifications
> > 
> > to the linux-3.8.y-queue branch of the 3.8.y.z extended stable tree 
> > which can be found at:
> > 
> >  http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.8.y-queue
> > 
> > This patch is scheduled to be released in version 3.8.13.5.
> > 
> > If you, or anyone else, feels it should not be added to this tree, please 
> > reply to this email.
> > 
> > For more information about the 3.8.y.z tree, see
> > https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable
> > 
> > Thanks.
> > -Kamal
> > 
> > ------
> > 
> > From 26655857295527252a3d6606b4cb248580b7843d Mon Sep 17 00:00:00 2001
> > From: Stanislav Kinsbursky <skinsbursky@parallels.com>
> > Date: Wed, 26 Jun 2013 10:15:14 +0400
> > Subject: SUNRPC: fix races on PipeFS UMOUNT notifications
> > 
> > commit adb6fa7ffe9031857ec14b8aab75c9ab65556cbc upstream.
> 
> Hi,
> 
> Can we please hold off on this patch and the other PipeFS followups for
> now? It isn't completely clear that people are hitting these problems in
> the wild, and the patch itself introduces bugs which still aren't 100%
> fixed upstream (I have 1 more patch queued to deal with the aftermath).
> 
> Let's just wait until we have definite proof that people are Oopsing,
> and we've addressed all the issues in the upstream kernel.
> 
> Thanks,
>   Trond
> 

No problem.  I've dropped the SUNRPC commits from the linux-3.8.y-queue.

Thanks Trond,

 -Kamal
diff mbox

Patch

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index f87fe8e..a4207ae 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -235,8 +235,6 @@  static struct rpc_clnt *rpc_get_client_for_event(struct net *net, int event)
 			continue;
 		if (rpc_clnt_skip_event(clnt, event))
 			continue;
-		if (atomic_inc_not_zero(&clnt->cl_count) == 0)
-			continue;
 		spin_unlock(&sn->rpc_client_lock);
 		return clnt;
 	}
@@ -253,7 +251,6 @@  static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,

 	while ((clnt = rpc_get_client_for_event(sb->s_fs_info, event))) {
 		error = __rpc_pipefs_event(clnt, event, sb);
-		rpc_release_client(clnt);
 		if (error)
 			break;
 	}
@@ -639,8 +636,8 @@  rpc_free_client(struct rpc_clnt *clnt)
 			rcu_dereference(clnt->cl_xprt)->servername);
 	if (clnt->cl_parent != clnt)
 		rpc_release_client(clnt->cl_parent);
-	rpc_unregister_client(clnt);
 	rpc_clnt_remove_pipedir(clnt);
+	rpc_unregister_client(clnt);
 	rpc_free_iostats(clnt->cl_metrics);
 	kfree(clnt->cl_principal);
 	clnt->cl_metrics = NULL;
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 6af77d1..87a616a 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -1160,12 +1160,12 @@  static void rpc_kill_sb(struct super_block *sb)
 		goto out;
 	}
 	sn->pipefs_sb = NULL;
-	mutex_unlock(&sn->pipefs_sb_lock);
 	dprintk("RPC:       sending pipefs UMOUNT notification for net %p%s\n",
 		net, NET_NAME(net));
 	blocking_notifier_call_chain(&rpc_pipefs_notifier_list,
 					   RPC_PIPEFS_UMOUNT,
 					   sb);
+	mutex_unlock(&sn->pipefs_sb_lock);
 	put_net(net);
 out:
 	kill_litter_super(sb);