Commit b274b48f3ef6e43e3831e8793c697a9573a607af
1 parent
e9cc6c234b
Exists in
master
and in
7 other branches
NFSv4: Fix circular locking dependency in nfs4_kill_renewd
Erez Zadok reports: ======================================================= [ INFO: possible circular locking dependency detected ] 2.6.24-rc6-unionfs2 #80 ------------------------------------------------------- umount.nfs4/4017 is trying to acquire lock: (&(&clp->cl_renewd)->work){--..}, at: [<c0223e53>] __cancel_work_timer+0x83/0x17f but task is already holding lock: (&clp->cl_sem){----}, at: [<f8879897>] nfs4_kill_renewd+0x17/0x29 [nfs] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&clp->cl_sem){----}: [<c0230699>] __lock_acquire+0x9cc/0xb95 [<c0230c39>] lock_acquire+0x5f/0x78 [<c0397cb8>] down_read+0x3a/0x4c [<f88798e6>] nfs4_renew_state+0x1c/0x1b8 [nfs] [<c0223821>] run_workqueue+0xd9/0x1ac [<c0224220>] worker_thread+0x7a/0x86 [<c0226b49>] kthread+0x3b/0x62 [<c02033a3>] kernel_thread_helper+0x7/0x10 [<ffffffff>] 0xffffffff -> #0 (&(&clp->cl_renewd)->work){--..}: [<c0230589>] __lock_acquire+0x8bc/0xb95 [<c0230c39>] lock_acquire+0x5f/0x78 [<c0223e87>] __cancel_work_timer+0xb7/0x17f [<c0223f5a>] cancel_delayed_work_sync+0xb/0xd [<f887989e>] nfs4_kill_renewd+0x1e/0x29 [nfs] [<f885a8f6>] nfs_free_client+0x37/0x9e [nfs] [<f885ab20>] nfs_put_client+0x5d/0x62 [nfs] [<f885ab9a>] nfs_free_server+0x75/0xae [nfs] [<f8862672>] nfs4_kill_super+0x27/0x2b [nfs] [<c0258aab>] deactivate_super+0x3f/0x51 [<c0269668>] mntput_no_expire+0x42/0x67 [<c025d0e4>] path_release_on_umount+0x15/0x18 [<c0269d30>] sys_umount+0x1a3/0x1cb [<c0269d71>] sys_oldumount+0x19/0x1b [<c02026ca>] sysenter_past_esp+0x5f/0xa5 [<ffffffff>] 0xffffffff Looking at the code, it would seem that taking the clp->cl_sem in nfs4_kill_renewd is completely redundant, since we're already guaranteed to have exclusive access to the nfs_client (we're shutting down). Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Showing 1 changed file with 0 additions and 2 deletions Inline Diff
fs/nfs/nfs4renewd.c
1 | /* | 1 | /* |
2 | * fs/nfs/nfs4renewd.c | 2 | * fs/nfs/nfs4renewd.c |
3 | * | 3 | * |
4 | * Copyright (c) 2002 The Regents of the University of Michigan. | 4 | * Copyright (c) 2002 The Regents of the University of Michigan. |
5 | * All rights reserved. | 5 | * All rights reserved. |
6 | * | 6 | * |
7 | * Kendrick Smith <kmsmith@umich.edu> | 7 | * Kendrick Smith <kmsmith@umich.edu> |
8 | * | 8 | * |
9 | * Redistribution and use in source and binary forms, with or without | 9 | * Redistribution and use in source and binary forms, with or without |
10 | * modification, are permitted provided that the following conditions | 10 | * modification, are permitted provided that the following conditions |
11 | * are met: | 11 | * are met: |
12 | * | 12 | * |
13 | * 1. Redistributions of source code must retain the above copyright | 13 | * 1. Redistributions of source code must retain the above copyright |
14 | * notice, this list of conditions and the following disclaimer. | 14 | * notice, this list of conditions and the following disclaimer. |
15 | * 2. Redistributions in binary form must reproduce the above copyright | 15 | * 2. Redistributions in binary form must reproduce the above copyright |
16 | * notice, this list of conditions and the following disclaimer in the | 16 | * notice, this list of conditions and the following disclaimer in the |
17 | * documentation and/or other materials provided with the distribution. | 17 | * documentation and/or other materials provided with the distribution. |
18 | * 3. Neither the name of the University nor the names of its | 18 | * 3. Neither the name of the University nor the names of its |
19 | * contributors may be used to endorse or promote products derived | 19 | * contributors may be used to endorse or promote products derived |
20 | * from this software without specific prior written permission. | 20 | * from this software without specific prior written permission. |
21 | * | 21 | * |
22 | * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED | 22 | * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED |
23 | * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF | 23 | * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF |
24 | * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE | 24 | * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE |
25 | * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE | 25 | * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE |
26 | * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR | 26 | * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR |
27 | * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF | 27 | * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF |
28 | * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR | 28 | * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR |
29 | * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF | 29 | * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF |
30 | * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING | 30 | * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING |
31 | * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS | 31 | * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS |
32 | * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | 32 | * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. |
33 | * | 33 | * |
34 | * Implementation of the NFSv4 "renew daemon", which wakes up periodically to | 34 | * Implementation of the NFSv4 "renew daemon", which wakes up periodically to |
35 | * send a RENEW, to keep state alive on the server. The daemon is implemented | 35 | * send a RENEW, to keep state alive on the server. The daemon is implemented |
36 | * as an rpc_task, not a real kernel thread, so it always runs in rpciod's | 36 | * as an rpc_task, not a real kernel thread, so it always runs in rpciod's |
37 | * context. There is one renewd per nfs_server. | 37 | * context. There is one renewd per nfs_server. |
38 | * | 38 | * |
39 | * TODO: If the send queue gets backlogged (e.g., if the server goes down), | 39 | * TODO: If the send queue gets backlogged (e.g., if the server goes down), |
40 | * we will keep filling the queue with periodic RENEW requests. We need a | 40 | * we will keep filling the queue with periodic RENEW requests. We need a |
41 | * mechanism for ensuring that if renewd successfully sends off a request, | 41 | * mechanism for ensuring that if renewd successfully sends off a request, |
42 | * then it only wakes up when the request is finished. Maybe use the | 42 | * then it only wakes up when the request is finished. Maybe use the |
43 | * child task framework of the RPC layer? | 43 | * child task framework of the RPC layer? |
44 | */ | 44 | */ |
45 | 45 | ||
46 | #include <linux/mm.h> | 46 | #include <linux/mm.h> |
47 | #include <linux/pagemap.h> | 47 | #include <linux/pagemap.h> |
48 | #include <linux/sunrpc/sched.h> | 48 | #include <linux/sunrpc/sched.h> |
49 | #include <linux/sunrpc/clnt.h> | 49 | #include <linux/sunrpc/clnt.h> |
50 | 50 | ||
51 | #include <linux/nfs.h> | 51 | #include <linux/nfs.h> |
52 | #include <linux/nfs4.h> | 52 | #include <linux/nfs4.h> |
53 | #include <linux/nfs_fs.h> | 53 | #include <linux/nfs_fs.h> |
54 | #include "nfs4_fs.h" | 54 | #include "nfs4_fs.h" |
55 | #include "delegation.h" | 55 | #include "delegation.h" |
56 | 56 | ||
57 | #define NFSDBG_FACILITY NFSDBG_PROC | 57 | #define NFSDBG_FACILITY NFSDBG_PROC |
58 | 58 | ||
59 | void | 59 | void |
60 | nfs4_renew_state(struct work_struct *work) | 60 | nfs4_renew_state(struct work_struct *work) |
61 | { | 61 | { |
62 | struct nfs_client *clp = | 62 | struct nfs_client *clp = |
63 | container_of(work, struct nfs_client, cl_renewd.work); | 63 | container_of(work, struct nfs_client, cl_renewd.work); |
64 | struct rpc_cred *cred; | 64 | struct rpc_cred *cred; |
65 | long lease, timeout; | 65 | long lease, timeout; |
66 | unsigned long last, now; | 66 | unsigned long last, now; |
67 | 67 | ||
68 | down_read(&clp->cl_sem); | 68 | down_read(&clp->cl_sem); |
69 | dprintk("%s: start\n", __FUNCTION__); | 69 | dprintk("%s: start\n", __FUNCTION__); |
70 | /* Are there any active superblocks? */ | 70 | /* Are there any active superblocks? */ |
71 | if (list_empty(&clp->cl_superblocks)) | 71 | if (list_empty(&clp->cl_superblocks)) |
72 | goto out; | 72 | goto out; |
73 | spin_lock(&clp->cl_lock); | 73 | spin_lock(&clp->cl_lock); |
74 | lease = clp->cl_lease_time; | 74 | lease = clp->cl_lease_time; |
75 | last = clp->cl_last_renewal; | 75 | last = clp->cl_last_renewal; |
76 | now = jiffies; | 76 | now = jiffies; |
77 | timeout = (2 * lease) / 3 + (long)last - (long)now; | 77 | timeout = (2 * lease) / 3 + (long)last - (long)now; |
78 | /* Are we close to a lease timeout? */ | 78 | /* Are we close to a lease timeout? */ |
79 | if (time_after(now, last + lease/3)) { | 79 | if (time_after(now, last + lease/3)) { |
80 | cred = nfs4_get_renew_cred(clp); | 80 | cred = nfs4_get_renew_cred(clp); |
81 | if (cred == NULL) { | 81 | if (cred == NULL) { |
82 | set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state); | 82 | set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state); |
83 | spin_unlock(&clp->cl_lock); | 83 | spin_unlock(&clp->cl_lock); |
84 | nfs_expire_all_delegations(clp); | 84 | nfs_expire_all_delegations(clp); |
85 | goto out; | 85 | goto out; |
86 | } | 86 | } |
87 | spin_unlock(&clp->cl_lock); | 87 | spin_unlock(&clp->cl_lock); |
88 | /* Queue an asynchronous RENEW. */ | 88 | /* Queue an asynchronous RENEW. */ |
89 | nfs4_proc_async_renew(clp, cred); | 89 | nfs4_proc_async_renew(clp, cred); |
90 | put_rpccred(cred); | 90 | put_rpccred(cred); |
91 | timeout = (2 * lease) / 3; | 91 | timeout = (2 * lease) / 3; |
92 | spin_lock(&clp->cl_lock); | 92 | spin_lock(&clp->cl_lock); |
93 | } else | 93 | } else |
94 | dprintk("%s: failed to call renewd. Reason: lease not expired \n", | 94 | dprintk("%s: failed to call renewd. Reason: lease not expired \n", |
95 | __FUNCTION__); | 95 | __FUNCTION__); |
96 | if (timeout < 5 * HZ) /* safeguard */ | 96 | if (timeout < 5 * HZ) /* safeguard */ |
97 | timeout = 5 * HZ; | 97 | timeout = 5 * HZ; |
98 | dprintk("%s: requeueing work. Lease period = %ld\n", | 98 | dprintk("%s: requeueing work. Lease period = %ld\n", |
99 | __FUNCTION__, (timeout + HZ - 1) / HZ); | 99 | __FUNCTION__, (timeout + HZ - 1) / HZ); |
100 | cancel_delayed_work(&clp->cl_renewd); | 100 | cancel_delayed_work(&clp->cl_renewd); |
101 | schedule_delayed_work(&clp->cl_renewd, timeout); | 101 | schedule_delayed_work(&clp->cl_renewd, timeout); |
102 | spin_unlock(&clp->cl_lock); | 102 | spin_unlock(&clp->cl_lock); |
103 | out: | 103 | out: |
104 | up_read(&clp->cl_sem); | 104 | up_read(&clp->cl_sem); |
105 | dprintk("%s: done\n", __FUNCTION__); | 105 | dprintk("%s: done\n", __FUNCTION__); |
106 | } | 106 | } |
107 | 107 | ||
108 | /* Must be called with clp->cl_sem locked for writes */ | 108 | /* Must be called with clp->cl_sem locked for writes */ |
109 | void | 109 | void |
110 | nfs4_schedule_state_renewal(struct nfs_client *clp) | 110 | nfs4_schedule_state_renewal(struct nfs_client *clp) |
111 | { | 111 | { |
112 | long timeout; | 112 | long timeout; |
113 | 113 | ||
114 | spin_lock(&clp->cl_lock); | 114 | spin_lock(&clp->cl_lock); |
115 | timeout = (2 * clp->cl_lease_time) / 3 + (long)clp->cl_last_renewal | 115 | timeout = (2 * clp->cl_lease_time) / 3 + (long)clp->cl_last_renewal |
116 | - (long)jiffies; | 116 | - (long)jiffies; |
117 | if (timeout < 5 * HZ) | 117 | if (timeout < 5 * HZ) |
118 | timeout = 5 * HZ; | 118 | timeout = 5 * HZ; |
119 | dprintk("%s: requeueing work. Lease period = %ld\n", | 119 | dprintk("%s: requeueing work. Lease period = %ld\n", |
120 | __FUNCTION__, (timeout + HZ - 1) / HZ); | 120 | __FUNCTION__, (timeout + HZ - 1) / HZ); |
121 | cancel_delayed_work(&clp->cl_renewd); | 121 | cancel_delayed_work(&clp->cl_renewd); |
122 | schedule_delayed_work(&clp->cl_renewd, timeout); | 122 | schedule_delayed_work(&clp->cl_renewd, timeout); |
123 | set_bit(NFS_CS_RENEWD, &clp->cl_res_state); | 123 | set_bit(NFS_CS_RENEWD, &clp->cl_res_state); |
124 | spin_unlock(&clp->cl_lock); | 124 | spin_unlock(&clp->cl_lock); |
125 | } | 125 | } |
126 | 126 | ||
127 | void | 127 | void |
128 | nfs4_renewd_prepare_shutdown(struct nfs_server *server) | 128 | nfs4_renewd_prepare_shutdown(struct nfs_server *server) |
129 | { | 129 | { |
130 | cancel_delayed_work(&server->nfs_client->cl_renewd); | 130 | cancel_delayed_work(&server->nfs_client->cl_renewd); |
131 | } | 131 | } |
132 | 132 | ||
133 | void | 133 | void |
134 | nfs4_kill_renewd(struct nfs_client *clp) | 134 | nfs4_kill_renewd(struct nfs_client *clp) |
135 | { | 135 | { |
136 | down_read(&clp->cl_sem); | ||
137 | cancel_delayed_work_sync(&clp->cl_renewd); | 136 | cancel_delayed_work_sync(&clp->cl_renewd); |
138 | up_read(&clp->cl_sem); | ||
139 | } | 137 | } |
140 | 138 | ||
141 | /* | 139 | /* |
142 | * Local variables: | 140 | * Local variables: |
143 | * c-basic-offset: 8 | 141 | * c-basic-offset: 8 |
144 | * End: | 142 | * End: |
145 | */ | 143 | */ |
146 | 144 |