diff options
author | Patrick Williams <iawillia@us.ibm.com> | 2014-05-16 12:14:01 -0500 |
---|---|---|
committer | A. Patrick Williams III <iawillia@us.ibm.com> | 2014-05-21 10:18:08 -0500 |
commit | 287187c4aa025d35b66eb9140e98080b3af05de3 (patch) | |
tree | ac2f9eb79f020ac2ea0cf0d52ba140f342492a5c | |
parent | ee8e275731ea63b96c5fec143b2303afa4736c30 (diff) | |
download | talos-hostboot-287187c4aa025d35b66eb9140e98080b3af05de3.tar.gz talos-hostboot-287187c4aa025d35b66eb9140e98080b3af05de3.zip |
Trace race condition causes deadlock.
In some cases where the trace daemon is coalescing trace at the
exact instant that a thread is creating a trace entry we can end
up in a deadlock. The daemon is in the process of moving a trace
entry from one page to another and updates some pointers in the
trace linked list. The daemon then pauses for all of the trace
producers to finish their transactions before updating the component
buffer's pointers. At this point, the producer can use data that
is in a half-updated state and deadlock. The solution is to have
the producer cache state data during the life of its transaction.
Change-Id: Ic7d031abb03f24e47cf4903400ef5f0e7520ee68
CQ: SW261995
Backport: release-fips811
Reviewed-on: http://gfw160.aus.stglabs.ibm.com:8080/gerrit/11149
Tested-by: Jenkins Server
Reviewed-by: Andrew J. Geissler <andrewg@us.ibm.com>
Reviewed-by: Douglas R. Gilbert <dgilbert@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>
Reviewed-by: A. Patrick Williams III <iawillia@us.ibm.com>
-rw-r--r-- | src/usr/trace/buffer.C | 15 |
1 files changed, 11 insertions, 4 deletions
diff --git a/src/usr/trace/buffer.C b/src/usr/trace/buffer.C index 425df2ac7..a1b5dbd8a 100644 --- a/src/usr/trace/buffer.C +++ b/src/usr/trace/buffer.C @@ -360,16 +360,23 @@ namespace TRACE // Read the component from the entry itself (added as part of claiming). ComponentDesc* l_comp = i_entry->comp; + Entry* l_savedNext = NULL; + // Lockless loop to update component linked-list. do { // Update our entry's "next" pointer. - i_entry->next = l_comp->iv_first; + // Note: Our next pointer could change out from under us by the + // daemon's replaceEntry function, but the component iv_first + // cannot change by the daemon until we _producerExit, therefore + // we need to save the original next pointer for the atomic update + // of l_comp->iv_first below. + l_savedNext = i_entry->next = l_comp->iv_first; // If there is an entry, update its "prev" pointer to this entry. - if (i_entry->next) + if (l_savedNext) { - if (!__sync_bool_compare_and_swap(&i_entry->next->prev, + if (!__sync_bool_compare_and_swap(&l_savedNext->prev, NULL, i_entry)) { @@ -403,7 +410,7 @@ namespace TRACE // it point at this entry. // while (!__sync_bool_compare_and_swap(&l_comp->iv_first, - i_entry->next, + l_savedNext, i_entry)); |