summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPatrick Williams <iawillia@us.ibm.com>2014-05-16 12:14:01 -0500
committerA. Patrick Williams III <iawillia@us.ibm.com>2014-05-21 10:18:08 -0500
commit287187c4aa025d35b66eb9140e98080b3af05de3 (patch)
treeac2f9eb79f020ac2ea0cf0d52ba140f342492a5c
parentee8e275731ea63b96c5fec143b2303afa4736c30 (diff)
downloadtalos-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.C15
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));
OpenPOWER on IntegriCloud