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
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.
13 This bug was exposed when testing multiple processes concurrently flock() the
16 Signed-off-by: Sunil Mushran <sunil.mushran@oracle.com>
17 Signed-off-by: Mark Fasheh <mfasheh@suse.com>
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(-)
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;
33 + spinlock_t track_lock;
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;
41 + struct dlm_ctxt *dlm;
43 unsigned migration_pending:1;
44 atomic_t asts_reserved;
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)
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;
58 - spin_lock(&dlm->spinlock);
59 + spin_lock(&dlm->track_lock);
61 + track_list = &oldres->tracking;
63 + track_list = &dlm->tracking_list;
66 - list_for_each_entry(res, &dl->dl_res->tracking, tracking) {
68 - dlm_lockres_put(dl->dl_res);
71 - if (&res->tracking == &dlm->tracking_list) {
72 - mlog(0, "End of list found, %p\n", res);
76 + list_for_each_entry(res, track_list, tracking) {
77 + if (&res->tracking == &dlm->tracking_list)
85 - if (!list_empty(&dlm->tracking_list)) {
86 - list_for_each_entry(res, &dlm->tracking_list, tracking)
88 - dlm_lockres_get(res);
94 + spin_unlock(&dlm->track_lock);
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);
102 + dlm_lockres_put(oldres);
104 - spin_unlock(&dlm->spinlock);
108 + spin_lock(&res->spinlock);
109 + dump_lockres(res, dl->dl_buf, dl->dl_len - 1);
110 + spin_unlock(&res->spinlock);
114 + /* passed to seq_show */
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)
137 struct dlm_lock_resource *res;
138 + struct dlm_ctxt *dlm;
140 res = container_of(kref, struct dlm_lock_resource, refs);
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,
149 + spin_lock(&dlm->track_lock);
150 if (!list_empty(&res->tracking))
151 list_del_init(&res->tracking);
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);
157 + spin_unlock(&dlm->track_lock);
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;
167 + /* put in dlm_lockres_release */
171 kref_init(&res->refs);
173 /* just for consistency */