]>
Commit | Line | Data |
---|---|---|
00e5a55c BS |
1 | From: Sunil Mushran <sunil.mushran@oracle.com> |
2 | Date: Tue, 16 Dec 2008 15:49:22 -0800 | |
3 | Subject: ocfs2/dlm: Fix race in adding/removing lockres' to/from the tracking list | |
4 | Patch-mainline: 2.6.30 | |
5 | References: bnc#408304 | |
6 | ||
7 | This patch adds a new lock, dlm->tracking_lock, to protect adding/removing | |
8 | lockres' to/from the dlm->tracking_list. We were previously using dlm->spinlock | |
9 | for the same, but that proved inadequate as we could be freeing a lockres from | |
10 | a context that did not hold that lock. As the new lock only protects this list, | |
11 | we can explicitly take it when removing the lockres from the tracking list. | |
12 | ||
13 | This bug was exposed when testing multiple processes concurrently flock() the | |
14 | same file. | |
15 | ||
16 | Signed-off-by: Sunil Mushran <sunil.mushran@oracle.com> | |
17 | Signed-off-by: Mark Fasheh <mfasheh@suse.com> | |
18 | --- | |
19 | fs/ocfs2/dlm/dlmcommon.h | 3 ++ | |
20 | fs/ocfs2/dlm/dlmdebug.c | 53 ++++++++++++++++++++------------------------- | |
21 | fs/ocfs2/dlm/dlmdomain.c | 1 + | |
22 | fs/ocfs2/dlm/dlmmaster.c | 10 ++++++++ | |
23 | 4 files changed, 38 insertions(+), 29 deletions(-) | |
24 | ||
25 | diff --git a/fs/ocfs2/dlm/dlmcommon.h b/fs/ocfs2/dlm/dlmcommon.h | |
26 | index d5a86fb..bb53714 100644 | |
27 | --- a/fs/ocfs2/dlm/dlmcommon.h | |
28 | +++ b/fs/ocfs2/dlm/dlmcommon.h | |
29 | @@ -140,6 +140,7 @@ struct dlm_ctxt | |
30 | unsigned int purge_count; | |
31 | spinlock_t spinlock; | |
32 | spinlock_t ast_lock; | |
33 | + spinlock_t track_lock; | |
34 | char *name; | |
35 | u8 node_num; | |
36 | u32 key; | |
37 | @@ -316,6 +317,8 @@ struct dlm_lock_resource | |
38 | * put on a list for the dlm thread to run. */ | |
39 | unsigned long last_used; | |
40 | ||
41 | + struct dlm_ctxt *dlm; | |
42 | + | |
43 | unsigned migration_pending:1; | |
44 | atomic_t asts_reserved; | |
45 | spinlock_t spinlock; | |
46 | diff --git a/fs/ocfs2/dlm/dlmdebug.c b/fs/ocfs2/dlm/dlmdebug.c | |
47 | index 1b81dcb..b32f60a 100644 | |
48 | --- a/fs/ocfs2/dlm/dlmdebug.c | |
49 | +++ b/fs/ocfs2/dlm/dlmdebug.c | |
50 | @@ -630,43 +630,38 @@ static void *lockres_seq_start(struct seq_file *m, loff_t *pos) | |
51 | { | |
52 | struct debug_lockres *dl = m->private; | |
53 | struct dlm_ctxt *dlm = dl->dl_ctxt; | |
54 | + struct dlm_lock_resource *oldres = dl->dl_res; | |
55 | struct dlm_lock_resource *res = NULL; | |
56 | + struct list_head *track_list; | |
57 | ||
58 | - spin_lock(&dlm->spinlock); | |
59 | + spin_lock(&dlm->track_lock); | |
60 | + if (oldres) | |
61 | + track_list = &oldres->tracking; | |
62 | + else | |
63 | + track_list = &dlm->tracking_list; | |
64 | ||
65 | - if (dl->dl_res) { | |
66 | - list_for_each_entry(res, &dl->dl_res->tracking, tracking) { | |
67 | - if (dl->dl_res) { | |
68 | - dlm_lockres_put(dl->dl_res); | |
69 | - dl->dl_res = NULL; | |
70 | - } | |
71 | - if (&res->tracking == &dlm->tracking_list) { | |
72 | - mlog(0, "End of list found, %p\n", res); | |
73 | - dl = NULL; | |
74 | - break; | |
75 | - } | |
76 | + list_for_each_entry(res, track_list, tracking) { | |
77 | + if (&res->tracking == &dlm->tracking_list) | |
78 | + res = NULL; | |
79 | + else | |
80 | dlm_lockres_get(res); | |
81 | - dl->dl_res = res; | |
82 | - break; | |
83 | - } | |
84 | - } else { | |
85 | - if (!list_empty(&dlm->tracking_list)) { | |
86 | - list_for_each_entry(res, &dlm->tracking_list, tracking) | |
87 | - break; | |
88 | - dlm_lockres_get(res); | |
89 | - dl->dl_res = res; | |
90 | - } else | |
91 | - dl = NULL; | |
92 | + break; | |
93 | } | |
94 | + spin_unlock(&dlm->track_lock); | |
95 | ||
96 | - if (dl) { | |
97 | - spin_lock(&dl->dl_res->spinlock); | |
98 | - dump_lockres(dl->dl_res, dl->dl_buf, dl->dl_len - 1); | |
99 | - spin_unlock(&dl->dl_res->spinlock); | |
100 | - } | |
101 | + if (oldres) | |
102 | + dlm_lockres_put(oldres); | |
103 | ||
104 | - spin_unlock(&dlm->spinlock); | |
105 | + dl->dl_res = res; | |
106 | + | |
107 | + if (res) { | |
108 | + spin_lock(&res->spinlock); | |
109 | + dump_lockres(res, dl->dl_buf, dl->dl_len - 1); | |
110 | + spin_unlock(&res->spinlock); | |
111 | + } else | |
112 | + dl = NULL; | |
113 | ||
114 | + /* passed to seq_show */ | |
115 | return dl; | |
116 | } | |
117 | ||
118 | diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c | |
119 | index 63f8125..d8d578f 100644 | |
120 | --- a/fs/ocfs2/dlm/dlmdomain.c | |
121 | +++ b/fs/ocfs2/dlm/dlmdomain.c | |
122 | @@ -1550,6 +1550,7 @@ static struct dlm_ctxt *dlm_alloc_ctxt(const char *domain, | |
123 | spin_lock_init(&dlm->spinlock); | |
124 | spin_lock_init(&dlm->master_lock); | |
125 | spin_lock_init(&dlm->ast_lock); | |
126 | + spin_lock_init(&dlm->track_lock); | |
127 | INIT_LIST_HEAD(&dlm->list); | |
128 | INIT_LIST_HEAD(&dlm->dirty_list); | |
129 | INIT_LIST_HEAD(&dlm->reco.resources); | |
130 | diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c | |
131 | index 92fd1d7..cbf3abe 100644 | |
132 | --- a/fs/ocfs2/dlm/dlmmaster.c | |
133 | +++ b/fs/ocfs2/dlm/dlmmaster.c | |
134 | @@ -505,8 +505,10 @@ void dlm_change_lockres_owner(struct dlm_ctxt *dlm, | |
135 | static void dlm_lockres_release(struct kref *kref) | |
136 | { | |
137 | struct dlm_lock_resource *res; | |
138 | + struct dlm_ctxt *dlm; | |
139 | ||
140 | res = container_of(kref, struct dlm_lock_resource, refs); | |
141 | + dlm = res->dlm; | |
142 | ||
143 | /* This should not happen -- all lockres' have a name | |
144 | * associated with them at init time. */ | |
145 | @@ -515,6 +517,7 @@ static void dlm_lockres_release(struct kref *kref) | |
146 | mlog(0, "destroying lockres %.*s\n", res->lockname.len, | |
147 | res->lockname.name); | |
148 | ||
149 | + spin_lock(&dlm->track_lock); | |
150 | if (!list_empty(&res->tracking)) | |
151 | list_del_init(&res->tracking); | |
152 | else { | |
153 | @@ -522,6 +525,9 @@ static void dlm_lockres_release(struct kref *kref) | |
154 | res->lockname.len, res->lockname.name); | |
155 | dlm_print_one_lock_resource(res); | |
156 | } | |
157 | + spin_unlock(&dlm->track_lock); | |
158 | + | |
159 | + dlm_put(dlm); | |
160 | ||
161 | if (!hlist_unhashed(&res->hash_node) || | |
162 | !list_empty(&res->granted) || | |
163 | @@ -595,6 +601,10 @@ static void dlm_init_lockres(struct dlm_ctxt *dlm, | |
164 | res->migration_pending = 0; | |
165 | res->inflight_locks = 0; | |
166 | ||
167 | + /* put in dlm_lockres_release */ | |
168 | + dlm_grab(dlm); | |
169 | + res->dlm = dlm; | |
170 | + | |
171 | kref_init(&res->refs); | |
172 | ||
173 | /* just for consistency */ | |
174 | -- | |
175 | 1.5.6 | |
176 |