* [Ocfs2-devel] Deadlock in DLM code still there
@ 2010-05-13 19:43 Jan Kara
2010-05-13 22:25 ` Sunil Mushran
2010-05-14 14:50 ` Wengang Wang
0 siblings, 2 replies; 4+ messages in thread
From: Jan Kara @ 2010-05-13 19:43 UTC (permalink / raw)
To: ocfs2-devel
Hi,
in http://www.mail-archive.com/ocfs2-devel at oss.oracle.com/msg03188.html
(more than an year ago) I've reported a lock inversion between dlm->ast_lock
and res->spinlock. The deadlock seems to be still there in 2.6.34-rc7:
=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.34-rc7-xen #4
-------------------------------------------------------
dlm_thread/2001 is trying to acquire lock:
(&(&dlm->ast_lock)->rlock){+.+...}, at: [<ffffffffa0119785>] dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
but task is already holding lock:
(&(&res->spinlock)->rlock){+.+...}, at: [<ffffffffa010452d>] dlm_thread+0x7cd/0x17f0 [ocfs2_dlm]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&(&res->spinlock)->rlock){+.+...}:
[<ffffffff810746bf>] __lock_acquire+0x109f/0x1720
[<ffffffff81074da9>] lock_acquire+0x69/0x90
[<ffffffff81328c6c>] _raw_spin_lock+0x2c/0x40
[<ffffffff8117e158>] _atomic_dec_and_lock+0x78/0xa0
[<ffffffffa010ebb9>] dlm_lockres_release_ast+0x29/0xb0 [ocfs2_dlm]
[<ffffffffa0104e41>] dlm_thread+0x10e1/0x17f0 [ocfs2_dlm]
[<ffffffff81060e1e>] kthread+0x8e/0xa0
[<ffffffff8100bda4>] kernel_thread_helper+0x4/0x10
-> #0 (&(&dlm->ast_lock)->rlock){+.+...}:
[<ffffffff81074b18>] __lock_acquire+0x14f8/0x1720
[<ffffffff81074da9>] lock_acquire+0x69/0x90
[<ffffffff81328c6c>] _raw_spin_lock+0x2c/0x40
[<ffffffffa0119785>] dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
[<ffffffffa010494f>] dlm_thread+0xbef/0x17f0 [ocfs2_dlm]
[<ffffffff81060e1e>] kthread+0x8e/0xa0
[<ffffffff8100bda4>] kernel_thread_helper+0x4/0x10
other info that might help us debug this:
1 lock held by dlm_thread/2001:
#0: (&(&res->spinlock)->rlock){+.+...}, at: [<ffffffffa010452d>] dlm_thread+0x7cd/0x17f0 [ocfs2_dlm]
stack backtrace:
Pid: 2001, comm: dlm_thread Not tainted 2.6.34-rc7-xen #4
Call Trace:
[<ffffffff810723d0>] print_circular_bug+0xf0/0x100
[<ffffffff81074b18>] __lock_acquire+0x14f8/0x1720
[<ffffffff8100701d>] ? xen_force_evtchn_callback+0xd/0x10
[<ffffffff81074da9>] lock_acquire+0x69/0x90
[<ffffffffa0119785>] ? dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
[<ffffffff81328c6c>] _raw_spin_lock+0x2c/0x40
[<ffffffffa0119785>] ? dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
[<ffffffffa0119785>] dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
[<ffffffffa010494f>] dlm_thread+0xbef/0x17f0 [ocfs2_dlm]
[<ffffffff81070cdd>] ? trace_hardirqs_off+0xd/0x10
[<ffffffff8107335d>] ? trace_hardirqs_on+0xd/0x10
[<ffffffff813293b2>] ? _raw_spin_unlock_irq+0x32/0x40
[<ffffffff81061330>] ? autoremove_wake_function+0x0/0x40
[<ffffffffa0103d60>] ? dlm_thread+0x0/0x17f0 [ocfs2_dlm]
[<ffffffff81060e1e>] kthread+0x8e/0xa0
[<ffffffff8100bda4>] kernel_thread_helper+0x4/0x10
[<ffffffff81329790>] ? restore_args+0x0/0x30
[<ffffffff8100bda0>] ? kernel_thread_helper+0x0/0x10
I'm now regularly hitting this problem so it stops me from verifying
whether there are other possible deadlocks in ocfs2 quota code...
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Ocfs2-devel] Deadlock in DLM code still there
2010-05-13 19:43 [Ocfs2-devel] Deadlock in DLM code still there Jan Kara
@ 2010-05-13 22:25 ` Sunil Mushran
2010-05-14 14:50 ` Wengang Wang
1 sibling, 0 replies; 4+ messages in thread
From: Sunil Mushran @ 2010-05-13 22:25 UTC (permalink / raw)
To: ocfs2-devel
Yes. This is a tricky problem. I'll work on it as soon as
I have completed my current task.
05/13/2010 12:43 PM, Jan Kara wrote:
> Hi,
>
> in http://www.mail-archive.com/ocfs2-devel at oss.oracle.com/msg03188.html
> (more than an year ago) I've reported a lock inversion between dlm->ast_lock
> and res->spinlock. The deadlock seems to be still there in 2.6.34-rc7:
>
> =======================================================
> [ INFO: possible circular locking dependency detected ]
> 2.6.34-rc7-xen #4
> -------------------------------------------------------
> dlm_thread/2001 is trying to acquire lock:
> (&(&dlm->ast_lock)->rlock){+.+...}, at: [<ffffffffa0119785>] dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
>
> but task is already holding lock:
> (&(&res->spinlock)->rlock){+.+...}, at: [<ffffffffa010452d>] dlm_thread+0x7cd/0x17f0 [ocfs2_dlm]
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (&(&res->spinlock)->rlock){+.+...}:
> [<ffffffff810746bf>] __lock_acquire+0x109f/0x1720
> [<ffffffff81074da9>] lock_acquire+0x69/0x90
> [<ffffffff81328c6c>] _raw_spin_lock+0x2c/0x40
> [<ffffffff8117e158>] _atomic_dec_and_lock+0x78/0xa0
> [<ffffffffa010ebb9>] dlm_lockres_release_ast+0x29/0xb0 [ocfs2_dlm]
> [<ffffffffa0104e41>] dlm_thread+0x10e1/0x17f0 [ocfs2_dlm]
> [<ffffffff81060e1e>] kthread+0x8e/0xa0
> [<ffffffff8100bda4>] kernel_thread_helper+0x4/0x10
>
> -> #0 (&(&dlm->ast_lock)->rlock){+.+...}:
> [<ffffffff81074b18>] __lock_acquire+0x14f8/0x1720
> [<ffffffff81074da9>] lock_acquire+0x69/0x90
> [<ffffffff81328c6c>] _raw_spin_lock+0x2c/0x40
> [<ffffffffa0119785>] dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
> [<ffffffffa010494f>] dlm_thread+0xbef/0x17f0 [ocfs2_dlm]
> [<ffffffff81060e1e>] kthread+0x8e/0xa0
> [<ffffffff8100bda4>] kernel_thread_helper+0x4/0x10
>
> other info that might help us debug this:
>
> 1 lock held by dlm_thread/2001:
> #0: (&(&res->spinlock)->rlock){+.+...}, at: [<ffffffffa010452d>] dlm_thread+0x7cd/0x17f0 [ocfs2_dlm]
>
> stack backtrace:
> Pid: 2001, comm: dlm_thread Not tainted 2.6.34-rc7-xen #4
> Call Trace:
> [<ffffffff810723d0>] print_circular_bug+0xf0/0x100
> [<ffffffff81074b18>] __lock_acquire+0x14f8/0x1720
> [<ffffffff8100701d>] ? xen_force_evtchn_callback+0xd/0x10
> [<ffffffff81074da9>] lock_acquire+0x69/0x90
> [<ffffffffa0119785>] ? dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
> [<ffffffff81328c6c>] _raw_spin_lock+0x2c/0x40
> [<ffffffffa0119785>] ? dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
> [<ffffffffa0119785>] dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
> [<ffffffffa010494f>] dlm_thread+0xbef/0x17f0 [ocfs2_dlm]
> [<ffffffff81070cdd>] ? trace_hardirqs_off+0xd/0x10
> [<ffffffff8107335d>] ? trace_hardirqs_on+0xd/0x10
> [<ffffffff813293b2>] ? _raw_spin_unlock_irq+0x32/0x40
> [<ffffffff81061330>] ? autoremove_wake_function+0x0/0x40
> [<ffffffffa0103d60>] ? dlm_thread+0x0/0x17f0 [ocfs2_dlm]
> [<ffffffff81060e1e>] kthread+0x8e/0xa0
> [<ffffffff8100bda4>] kernel_thread_helper+0x4/0x10
> [<ffffffff81329790>] ? restore_args+0x0/0x30
> [<ffffffff8100bda0>] ? kernel_thread_helper+0x0/0x10
>
> I'm now regularly hitting this problem so it stops me from verifying
> whether there are other possible deadlocks in ocfs2 quota code...
>
> Honza
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Ocfs2-devel] Deadlock in DLM code still there
2010-05-13 19:43 [Ocfs2-devel] Deadlock in DLM code still there Jan Kara
2010-05-13 22:25 ` Sunil Mushran
@ 2010-05-14 14:50 ` Wengang Wang
2010-05-14 18:30 ` Sunil Mushran
1 sibling, 1 reply; 4+ messages in thread
From: Wengang Wang @ 2010-05-14 14:50 UTC (permalink / raw)
To: ocfs2-devel
Hi,
On 10-05-13 21:43, Jan Kara wrote:
> Hi,
>
> in http://www.mail-archive.com/ocfs2-devel at oss.oracle.com/msg03188.html
> (more than an year ago) I've reported a lock inversion between dlm->ast_lock
> and res->spinlock. The deadlock seems to be still there in 2.6.34-rc7:
>
> =======================================================
> [ INFO: possible circular locking dependency detected ]
> 2.6.34-rc7-xen #4
> -------------------------------------------------------
> dlm_thread/2001 is trying to acquire lock:
> (&(&dlm->ast_lock)->rlock){+.+...}, at: [<ffffffffa0119785>] dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
>
> but task is already holding lock:
> (&(&res->spinlock)->rlock){+.+...}, at: [<ffffffffa010452d>] dlm_thread+0x7cd/0x17f0 [ocfs2_dlm]
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (&(&res->spinlock)->rlock){+.+...}:
> [<ffffffff810746bf>] __lock_acquire+0x109f/0x1720
> [<ffffffff81074da9>] lock_acquire+0x69/0x90
> [<ffffffff81328c6c>] _raw_spin_lock+0x2c/0x40
> [<ffffffff8117e158>] _atomic_dec_and_lock+0x78/0xa0
> [<ffffffffa010ebb9>] dlm_lockres_release_ast+0x29/0xb0 [ocfs2_dlm]
> [<ffffffffa0104e41>] dlm_thread+0x10e1/0x17f0 [ocfs2_dlm]
> [<ffffffff81060e1e>] kthread+0x8e/0xa0
> [<ffffffff8100bda4>] kernel_thread_helper+0x4/0x10
>
> -> #0 (&(&dlm->ast_lock)->rlock){+.+...}:
> [<ffffffff81074b18>] __lock_acquire+0x14f8/0x1720
> [<ffffffff81074da9>] lock_acquire+0x69/0x90
> [<ffffffff81328c6c>] _raw_spin_lock+0x2c/0x40
> [<ffffffffa0119785>] dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
> [<ffffffffa010494f>] dlm_thread+0xbef/0x17f0 [ocfs2_dlm]
> [<ffffffff81060e1e>] kthread+0x8e/0xa0
> [<ffffffff8100bda4>] kernel_thread_helper+0x4/0x10
The code path is simple:
in dlm_thread()
677 spin_lock(&res->spinlock);
.......
714 /* called while holding lockres lock */
715 dlm_shuffle_lists(dlm, res);
716 res->state &= ~DLM_LOCK_RES_DIRTY;
717 spin_unlock(&res->spinlock);
In dlm_shuffle_lists(), dlm_queue_bast()/dlm_queue_ast() are called.
In the later two functions, dlm->ast_lock is required.
Since we can't release res->spinlock, we have to take dlm->ast_lock
just before taking the res->spinlock(line 677) and release it after
res->spinlock is released. And use __dlm_queue_bast()/__dlm_queue_ast(),
the nolock version, in dlm_shuffle_lists().
There are no too many locks on a lockres, I think there is no performance
harm.
Comments?
Attached the patch(not well tested yet, will be tested if this approach is good).
At least with this patch Jan can continue with his test :P
Regards,
wengang.
-------------- next part --------------
diff --git a/fs/ocfs2/dlm/dlmast.c b/fs/ocfs2/dlm/dlmast.c
index 12d5eb7..992b0ae 100644
--- a/fs/ocfs2/dlm/dlmast.c
+++ b/fs/ocfs2/dlm/dlmast.c
@@ -88,7 +88,7 @@ static int dlm_should_cancel_bast(struct dlm_ctxt *dlm, struct dlm_lock *lock)
return 0;
}
-static void __dlm_queue_ast(struct dlm_ctxt *dlm, struct dlm_lock *lock)
+void __dlm_queue_ast(struct dlm_ctxt *dlm, struct dlm_lock *lock)
{
mlog_entry_void();
@@ -145,7 +145,7 @@ void dlm_queue_ast(struct dlm_ctxt *dlm, struct dlm_lock *lock)
}
-static void __dlm_queue_bast(struct dlm_ctxt *dlm, struct dlm_lock *lock)
+void __dlm_queue_bast(struct dlm_ctxt *dlm, struct dlm_lock *lock)
{
mlog_entry_void();
diff --git a/fs/ocfs2/dlm/dlmcommon.h b/fs/ocfs2/dlm/dlmcommon.h
index 0102be3..453d955 100644
--- a/fs/ocfs2/dlm/dlmcommon.h
+++ b/fs/ocfs2/dlm/dlmcommon.h
@@ -904,6 +904,8 @@ void __dlm_lockres_grab_inflight_ref(struct dlm_ctxt *dlm,
void dlm_queue_ast(struct dlm_ctxt *dlm, struct dlm_lock *lock);
void dlm_queue_bast(struct dlm_ctxt *dlm, struct dlm_lock *lock);
+void __dlm_queue_ast(struct dlm_ctxt *dlm, struct dlm_lock *lock);
+void __dlm_queue_bast(struct dlm_ctxt *dlm, struct dlm_lock *lock);
void dlm_do_local_ast(struct dlm_ctxt *dlm,
struct dlm_lock_resource *res,
struct dlm_lock *lock);
diff --git a/fs/ocfs2/dlm/dlmthread.c b/fs/ocfs2/dlm/dlmthread.c
index 11a6d1f..d4f73ca 100644
--- a/fs/ocfs2/dlm/dlmthread.c
+++ b/fs/ocfs2/dlm/dlmthread.c
@@ -309,6 +309,7 @@ static void dlm_shuffle_lists(struct dlm_ctxt *dlm,
* spinlock, and because we know that it is not migrating/
* recovering/in-progress, it is fine to reserve asts and
* basts right before queueing them all throughout */
+ assert_spin_locked(&dlm->ast_lock);
assert_spin_locked(&res->spinlock);
BUG_ON((res->state & (DLM_LOCK_RES_MIGRATING|
DLM_LOCK_RES_RECOVERING|
@@ -337,7 +338,7 @@ converting:
/* queue the BAST if not already */
if (lock->ml.highest_blocked == LKM_IVMODE) {
__dlm_lockres_reserve_ast(res);
- dlm_queue_bast(dlm, lock);
+ __dlm_queue_bast(dlm, lock);
}
/* update the highest_blocked if needed */
if (lock->ml.highest_blocked < target->ml.convert_type)
@@ -355,7 +356,7 @@ converting:
can_grant = 0;
if (lock->ml.highest_blocked == LKM_IVMODE) {
__dlm_lockres_reserve_ast(res);
- dlm_queue_bast(dlm, lock);
+ __dlm_queue_bast(dlm, lock);
}
if (lock->ml.highest_blocked < target->ml.convert_type)
lock->ml.highest_blocked =
@@ -383,7 +384,7 @@ converting:
spin_unlock(&target->spinlock);
__dlm_lockres_reserve_ast(res);
- dlm_queue_ast(dlm, target);
+ __dlm_queue_ast(dlm, target);
/* go back and check for more */
goto converting;
}
@@ -402,7 +403,7 @@ blocked:
can_grant = 0;
if (lock->ml.highest_blocked == LKM_IVMODE) {
__dlm_lockres_reserve_ast(res);
- dlm_queue_bast(dlm, lock);
+ __dlm_queue_bast(dlm, lock);
}
if (lock->ml.highest_blocked < target->ml.type)
lock->ml.highest_blocked = target->ml.type;
@@ -418,7 +419,7 @@ blocked:
can_grant = 0;
if (lock->ml.highest_blocked == LKM_IVMODE) {
__dlm_lockres_reserve_ast(res);
- dlm_queue_bast(dlm, lock);
+ __dlm_queue_bast(dlm, lock);
}
if (lock->ml.highest_blocked < target->ml.type)
lock->ml.highest_blocked = target->ml.type;
@@ -444,7 +445,7 @@ blocked:
spin_unlock(&target->spinlock);
__dlm_lockres_reserve_ast(res);
- dlm_queue_ast(dlm, target);
+ __dlm_queue_ast(dlm, target);
/* go back and check for more */
goto converting;
}
@@ -674,6 +675,7 @@ static int dlm_thread(void *data)
/* lockres can be re-dirtied/re-added to the
* dirty_list in this gap, but that is ok */
+ spin_lock(&dlm->ast_lock);
spin_lock(&res->spinlock);
if (res->owner != dlm->node_num) {
__dlm_print_one_lock_resource(res);
@@ -694,6 +696,7 @@ static int dlm_thread(void *data)
/* move it to the tail and keep going */
res->state &= ~DLM_LOCK_RES_DIRTY;
spin_unlock(&res->spinlock);
+ spin_unlock(&dlm->ast_lock);
mlog(0, "delaying list shuffling for in-"
"progress lockres %.*s, state=%d\n",
res->lockname.len, res->lockname.name,
@@ -715,6 +718,7 @@ static int dlm_thread(void *data)
dlm_shuffle_lists(dlm, res);
res->state &= ~DLM_LOCK_RES_DIRTY;
spin_unlock(&res->spinlock);
+ spin_unlock(&dlm->ast_lock);
dlm_lockres_calc_usage(dlm, res);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Ocfs2-devel] Deadlock in DLM code still there
2010-05-14 14:50 ` Wengang Wang
@ 2010-05-14 18:30 ` Sunil Mushran
0 siblings, 0 replies; 4+ messages in thread
From: Sunil Mushran @ 2010-05-14 18:30 UTC (permalink / raw)
To: ocfs2-devel
Excellent patch Wengang. I think that should take care of the problem.
Please can you send a formal patch to the list. I'll include it in our test
queue.
Thanks
Sunil
On 05/14/2010 07:50 AM, Wengang Wang wrote:
> Hi,
>
> On 10-05-13 21:43, Jan Kara wrote:
>
>> Hi,
>>
>> in http://www.mail-archive.com/ocfs2-devel at oss.oracle.com/msg03188.html
>> (more than an year ago) I've reported a lock inversion between dlm->ast_lock
>> and res->spinlock. The deadlock seems to be still there in 2.6.34-rc7:
>>
>> =======================================================
>> [ INFO: possible circular locking dependency detected ]
>> 2.6.34-rc7-xen #4
>> -------------------------------------------------------
>> dlm_thread/2001 is trying to acquire lock:
>> (&(&dlm->ast_lock)->rlock){+.+...}, at: [<ffffffffa0119785>] dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
>>
>> but task is already holding lock:
>> (&(&res->spinlock)->rlock){+.+...}, at: [<ffffffffa010452d>] dlm_thread+0x7cd/0x17f0 [ocfs2_dlm]
>>
>> which lock already depends on the new lock.
>>
>>
>> the existing dependency chain (in reverse order) is:
>>
>> -> #1 (&(&res->spinlock)->rlock){+.+...}:
>> [<ffffffff810746bf>] __lock_acquire+0x109f/0x1720
>> [<ffffffff81074da9>] lock_acquire+0x69/0x90
>> [<ffffffff81328c6c>] _raw_spin_lock+0x2c/0x40
>> [<ffffffff8117e158>] _atomic_dec_and_lock+0x78/0xa0
>> [<ffffffffa010ebb9>] dlm_lockres_release_ast+0x29/0xb0 [ocfs2_dlm]
>> [<ffffffffa0104e41>] dlm_thread+0x10e1/0x17f0 [ocfs2_dlm]
>> [<ffffffff81060e1e>] kthread+0x8e/0xa0
>> [<ffffffff8100bda4>] kernel_thread_helper+0x4/0x10
>>
>> -> #0 (&(&dlm->ast_lock)->rlock){+.+...}:
>> [<ffffffff81074b18>] __lock_acquire+0x14f8/0x1720
>> [<ffffffff81074da9>] lock_acquire+0x69/0x90
>> [<ffffffff81328c6c>] _raw_spin_lock+0x2c/0x40
>> [<ffffffffa0119785>] dlm_queue_bast+0x55/0x1e0 [ocfs2_dlm]
>> [<ffffffffa010494f>] dlm_thread+0xbef/0x17f0 [ocfs2_dlm]
>> [<ffffffff81060e1e>] kthread+0x8e/0xa0
>> [<ffffffff8100bda4>] kernel_thread_helper+0x4/0x10
>>
> The code path is simple:
> in dlm_thread()
> 677 spin_lock(&res->spinlock);
> .......
> 714 /* called while holding lockres lock */
> 715 dlm_shuffle_lists(dlm, res);
> 716 res->state&= ~DLM_LOCK_RES_DIRTY;
> 717 spin_unlock(&res->spinlock);
>
> In dlm_shuffle_lists(), dlm_queue_bast()/dlm_queue_ast() are called.
> In the later two functions, dlm->ast_lock is required.
> Since we can't release res->spinlock, we have to take dlm->ast_lock
> just before taking the res->spinlock(line 677) and release it after
> res->spinlock is released. And use __dlm_queue_bast()/__dlm_queue_ast(),
> the nolock version, in dlm_shuffle_lists().
> There are no too many locks on a lockres, I think there is no performance
> harm.
> Comments?
>
> Attached the patch(not well tested yet, will be tested if this approach is good).
> At least with this patch Jan can continue with his test :P
>
> Regards,
> wengang.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-05-14 18:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-13 19:43 [Ocfs2-devel] Deadlock in DLM code still there Jan Kara
2010-05-13 22:25 ` Sunil Mushran
2010-05-14 14:50 ` Wengang Wang
2010-05-14 18:30 ` Sunil Mushran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).