| Message ID | 1547841679-26655-1-git-send-email-arbab@linux.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | core/lock: Stop drop_my_locks() from always causing abort | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | master/apply_patch Successfully applied |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
On 01/19/2019 01:31 AM, Reza Arbab wrote: > The loop in drop_my_locks() looks like this: > > while((l = list_pop(&this_cpu()->locks_held, struct lock, list)) != NULL) { > if (warn) > prlog(PR_ERR, " %s\n", l->owner); > unlock(l); > } > > Both list_pop() and unlock() call list_del(). This means that on the > last iteration of the loop, the list will be empty when we get to > unlock_check(), causing this: > > LOCK ERROR: Releasing lock we don't hold depth @0x30493d20 (state: 0x0000000000000001) > [13836.000173140,0] Aborting! > CPU 0000 Backtrace: > S: 0000000031c03930 R: 000000003001d840 ._abort+0x60 > S: 0000000031c039c0 R: 000000003001a0c4 .lock_error+0x64 > S: 0000000031c03a50 R: 0000000030019c70 .unlock+0x54 > S: 0000000031c03af0 R: 000000003001a040 .drop_my_locks+0xf4 > > To fix this, change list_pop() to list_top(). > > Signed-off-by: Reza Arbab <arbab@linux.ibm.com> Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> @Stewart, I think we should pick this for Stable branch. #6.0+ -Vasant
Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: > On 01/19/2019 01:31 AM, Reza Arbab wrote: >> The loop in drop_my_locks() looks like this: >> >> while((l = list_pop(&this_cpu()->locks_held, struct lock, list)) != NULL) { >> if (warn) >> prlog(PR_ERR, " %s\n", l->owner); >> unlock(l); >> } >> >> Both list_pop() and unlock() call list_del(). This means that on the >> last iteration of the loop, the list will be empty when we get to >> unlock_check(), causing this: >> >> LOCK ERROR: Releasing lock we don't hold depth @0x30493d20 (state: 0x0000000000000001) >> [13836.000173140,0] Aborting! >> CPU 0000 Backtrace: >> S: 0000000031c03930 R: 000000003001d840 ._abort+0x60 >> S: 0000000031c039c0 R: 000000003001a0c4 .lock_error+0x64 >> S: 0000000031c03a50 R: 0000000030019c70 .unlock+0x54 >> S: 0000000031c03af0 R: 000000003001a040 .drop_my_locks+0xf4 >> >> To fix this, change list_pop() to list_top(). >> >> Signed-off-by: Reza Arbab <arbab@linux.ibm.com> > > Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > > > @Stewart, > I think we should pick this for Stable branch. #6.0+ Ack. Merged to master as of 9ef153f6f013b224db8e9b78764ef6cf89c152fa and cherry picking back into stable
diff --git a/core/lock.c b/core/lock.c index d3f881a..fbf0b21 100644 --- a/core/lock.c +++ b/core/lock.c @@ -321,7 +321,7 @@ void drop_my_locks(bool warn) struct lock *l; disable_fast_reboot("Lock corruption"); - while((l = list_pop(&this_cpu()->locks_held, struct lock, list)) != NULL) { + while((l = list_top(&this_cpu()->locks_held, struct lock, list)) != NULL) { if (warn) prlog(PR_ERR, " %s\n", l->owner); unlock(l);
The loop in drop_my_locks() looks like this: while((l = list_pop(&this_cpu()->locks_held, struct lock, list)) != NULL) { if (warn) prlog(PR_ERR, " %s\n", l->owner); unlock(l); } Both list_pop() and unlock() call list_del(). This means that on the last iteration of the loop, the list will be empty when we get to unlock_check(), causing this: LOCK ERROR: Releasing lock we don't hold depth @0x30493d20 (state: 0x0000000000000001) [13836.000173140,0] Aborting! CPU 0000 Backtrace: S: 0000000031c03930 R: 000000003001d840 ._abort+0x60 S: 0000000031c039c0 R: 000000003001a0c4 .lock_error+0x64 S: 0000000031c03a50 R: 0000000030019c70 .unlock+0x54 S: 0000000031c03af0 R: 000000003001a040 .drop_my_locks+0xf4 To fix this, change list_pop() to list_top(). Signed-off-by: Reza Arbab <arbab@linux.ibm.com> --- core/lock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)