Commit b0d4f817 authored by Sunil Mushran's avatar Sunil Mushran Committed by Mark Fasheh

ocfs2/dlm: Fix race in adding/removing lockres' to/from the tracking list

This patch adds a new lock, dlm->tracking_lock, to protect adding/removing
lockres' to/from the dlm->tracking_list. We were previously using dlm->spinlock
for the same, but that proved inadequate as we could be freeing a lockres from
a context that did not hold that lock. As the new lock only protects this list,
we can explicitly take it when removing the lockres from the tracking list.

This bug was exposed when testing multiple processes concurrently flock() the
same file.
Signed-off-by: default avatarSunil Mushran <sunil.mushran@oracle.com>
Signed-off-by: default avatarMark Fasheh <mfasheh@suse.com>
parent d4f7e650
...@@ -140,6 +140,7 @@ struct dlm_ctxt ...@@ -140,6 +140,7 @@ struct dlm_ctxt
unsigned int purge_count; unsigned int purge_count;
spinlock_t spinlock; spinlock_t spinlock;
spinlock_t ast_lock; spinlock_t ast_lock;
spinlock_t track_lock;
char *name; char *name;
u8 node_num; u8 node_num;
u32 key; u32 key;
...@@ -316,6 +317,8 @@ struct dlm_lock_resource ...@@ -316,6 +317,8 @@ struct dlm_lock_resource
* put on a list for the dlm thread to run. */ * put on a list for the dlm thread to run. */
unsigned long last_used; unsigned long last_used;
struct dlm_ctxt *dlm;
unsigned migration_pending:1; unsigned migration_pending:1;
atomic_t asts_reserved; atomic_t asts_reserved;
spinlock_t spinlock; spinlock_t spinlock;
......
...@@ -630,43 +630,38 @@ static void *lockres_seq_start(struct seq_file *m, loff_t *pos) ...@@ -630,43 +630,38 @@ static void *lockres_seq_start(struct seq_file *m, loff_t *pos)
{ {
struct debug_lockres *dl = m->private; struct debug_lockres *dl = m->private;
struct dlm_ctxt *dlm = dl->dl_ctxt; struct dlm_ctxt *dlm = dl->dl_ctxt;
struct dlm_lock_resource *oldres = dl->dl_res;
struct dlm_lock_resource *res = NULL; struct dlm_lock_resource *res = NULL;
struct list_head *track_list;
spin_lock(&dlm->spinlock); spin_lock(&dlm->track_lock);
if (oldres)
track_list = &oldres->tracking;
else
track_list = &dlm->tracking_list;
if (dl->dl_res) { list_for_each_entry(res, track_list, tracking) {
list_for_each_entry(res, &dl->dl_res->tracking, tracking) { if (&res->tracking == &dlm->tracking_list)
if (dl->dl_res) { res = NULL;
dlm_lockres_put(dl->dl_res); else
dl->dl_res = NULL;
}
if (&res->tracking == &dlm->tracking_list) {
mlog(0, "End of list found, %p\n", res);
dl = NULL;
break;
}
dlm_lockres_get(res); dlm_lockres_get(res);
dl->dl_res = res; break;
break;
}
} else {
if (!list_empty(&dlm->tracking_list)) {
list_for_each_entry(res, &dlm->tracking_list, tracking)
break;
dlm_lockres_get(res);
dl->dl_res = res;
} else
dl = NULL;
} }
spin_unlock(&dlm->track_lock);
if (dl) { if (oldres)
spin_lock(&dl->dl_res->spinlock); dlm_lockres_put(oldres);
dump_lockres(dl->dl_res, dl->dl_buf, dl->dl_len - 1);
spin_unlock(&dl->dl_res->spinlock);
}
spin_unlock(&dlm->spinlock); dl->dl_res = res;
if (res) {
spin_lock(&res->spinlock);
dump_lockres(res, dl->dl_buf, dl->dl_len - 1);
spin_unlock(&res->spinlock);
} else
dl = NULL;
/* passed to seq_show */
return dl; return dl;
} }
......
...@@ -1550,6 +1550,7 @@ static struct dlm_ctxt *dlm_alloc_ctxt(const char *domain, ...@@ -1550,6 +1550,7 @@ static struct dlm_ctxt *dlm_alloc_ctxt(const char *domain,
spin_lock_init(&dlm->spinlock); spin_lock_init(&dlm->spinlock);
spin_lock_init(&dlm->master_lock); spin_lock_init(&dlm->master_lock);
spin_lock_init(&dlm->ast_lock); spin_lock_init(&dlm->ast_lock);
spin_lock_init(&dlm->track_lock);
INIT_LIST_HEAD(&dlm->list); INIT_LIST_HEAD(&dlm->list);
INIT_LIST_HEAD(&dlm->dirty_list); INIT_LIST_HEAD(&dlm->dirty_list);
INIT_LIST_HEAD(&dlm->reco.resources); INIT_LIST_HEAD(&dlm->reco.resources);
......
...@@ -505,8 +505,10 @@ void dlm_change_lockres_owner(struct dlm_ctxt *dlm, ...@@ -505,8 +505,10 @@ void dlm_change_lockres_owner(struct dlm_ctxt *dlm,
static void dlm_lockres_release(struct kref *kref) static void dlm_lockres_release(struct kref *kref)
{ {
struct dlm_lock_resource *res; struct dlm_lock_resource *res;
struct dlm_ctxt *dlm;
res = container_of(kref, struct dlm_lock_resource, refs); res = container_of(kref, struct dlm_lock_resource, refs);
dlm = res->dlm;
/* This should not happen -- all lockres' have a name /* This should not happen -- all lockres' have a name
* associated with them at init time. */ * associated with them at init time. */
...@@ -515,6 +517,7 @@ static void dlm_lockres_release(struct kref *kref) ...@@ -515,6 +517,7 @@ static void dlm_lockres_release(struct kref *kref)
mlog(0, "destroying lockres %.*s\n", res->lockname.len, mlog(0, "destroying lockres %.*s\n", res->lockname.len,
res->lockname.name); res->lockname.name);
spin_lock(&dlm->track_lock);
if (!list_empty(&res->tracking)) if (!list_empty(&res->tracking))
list_del_init(&res->tracking); list_del_init(&res->tracking);
else { else {
...@@ -522,6 +525,9 @@ static void dlm_lockres_release(struct kref *kref) ...@@ -522,6 +525,9 @@ static void dlm_lockres_release(struct kref *kref)
res->lockname.len, res->lockname.name); res->lockname.len, res->lockname.name);
dlm_print_one_lock_resource(res); dlm_print_one_lock_resource(res);
} }
spin_unlock(&dlm->track_lock);
dlm_put(dlm);
if (!hlist_unhashed(&res->hash_node) || if (!hlist_unhashed(&res->hash_node) ||
!list_empty(&res->granted) || !list_empty(&res->granted) ||
...@@ -595,6 +601,10 @@ static void dlm_init_lockres(struct dlm_ctxt *dlm, ...@@ -595,6 +601,10 @@ static void dlm_init_lockres(struct dlm_ctxt *dlm,
res->migration_pending = 0; res->migration_pending = 0;
res->inflight_locks = 0; res->inflight_locks = 0;
/* put in dlm_lockres_release */
dlm_grab(dlm);
res->dlm = dlm;
kref_init(&res->refs); kref_init(&res->refs);
/* just for consistency */ /* just for consistency */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment