From patchwork Tue Dec 14 11:22:48 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Henderson X-Patchwork-Id: 75483 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 9A4B41007D5 for ; Tue, 14 Dec 2010 22:22:54 +1100 (EST) Received: (qmail 9764 invoked by alias); 14 Dec 2010 11:22:50 -0000 Received: (qmail 9754 invoked by uid 22791); 14 Dec 2010 11:22:48 -0000 X-SWARE-Spam-Status: No, hits=-6.0 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, TW_RW, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 14 Dec 2010 11:22:43 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id oBEBMfBT016403 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 14 Dec 2010 06:22:41 -0500 Received: from stone.twiddle.home (vpn-8-37.rdu.redhat.com [10.11.8.37]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id oBEBMdic029567; Tue, 14 Dec 2010 06:22:40 -0500 Message-ID: <4D075388.1010807@redhat.com> Date: Tue, 14 Dec 2010 03:22:48 -0800 From: Richard Henderson User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc14 Thunderbird/3.1.7 MIME-Version: 1.0 To: GCC Patches CC: torvald@se.inf.tu-dresden.de Subject: [trans-mem] Reduce contention in find_clone X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org We can do this by synchronizing on a lock we already have acquired, rather than invent one specific to this data structure. Idea courtesy of Torvald Riegel. r~ * clone.cc (table_lock): Remove. (find_clone): Don't take it. (ExcludeTransaction): New helper class. (_ITM_registerTMCloneTable): Use it. (_ITM_deregisterTMCloneTable): Likewise. Index: clone.cc =================================================================== --- clone.cc (revision 167789) +++ clone.cc (working copy) @@ -1,4 +1,4 @@ -/* Copyright (C) 2009 Free Software Foundation, Inc. +/* Copyright (C) 2009, 2010 Free Software Foundation, Inc. Contributed by Richard Henderson . This file is part of the GNU Transactional Memory Library (libitm). @@ -26,8 +26,6 @@ using namespace GTM; -static gtm_rwlock table_lock; - struct clone_entry { void *orig, *clone; @@ -46,10 +44,7 @@ find_clone (void *ptr) { clone_table *table; - void *ret = NULL; - table_lock.read_lock (); - for (table = all_tables; table ; table = table->next) { clone_entry *t = table->table; @@ -68,10 +63,7 @@ else if (ptr > t[i].orig) lo = i + 1; else - { - ret = t[i].clone; - goto found; - } + return t[i].clone; } /* Given the quick test above, if we don't find the entry in @@ -79,9 +71,7 @@ break; } - found: - table_lock.read_unlock (); - return ret; + return NULL; } @@ -120,11 +110,43 @@ return 0; } +namespace { + +// Within find_clone, we know that we are inside a transaction. Because +// of that, we have already synchronized with serial_lock. By taking the +// serial_lock for write, we exclude all transactions while we make this +// change to the clone tables, without having to synchronize on a separate +// lock. Do be careful not to attempt a recursive write lock. + +class ExcludeTransaction +{ + bool do_lock; + + public: + ExcludeTransaction() + { + gtm_transaction *tx = gtm_tx(); + do_lock = !(tx && (tx->state & gtm_transaction::STATE_SERIAL)); + + if (do_lock) + gtm_transaction::serial_lock.write_lock (); + } + + ~ExcludeTransaction() + { + if (do_lock) + gtm_transaction::serial_lock.write_unlock (); + } +}; + +} // end anon namespace + + void _ITM_registerTMCloneTable (void *xent, size_t size) { clone_entry *ent = static_cast(xent); - clone_table *old, *table; + clone_table *table; table = (clone_table *) xmalloc (sizeof (clone_table)); table->table = ent; @@ -132,31 +154,31 @@ qsort (ent, size, sizeof (clone_entry), clone_entry_compare); - old = all_tables; - do - { - table->next = old; - old = __sync_val_compare_and_swap (&all_tables, old, table); - } - while (old != table); + // Hold the serial_lock while we update the ALL_TABLES datastructure. + { + ExcludeTransaction exclude; + table->next = all_tables; + all_tables = table; + } } void _ITM_deregisterTMCloneTable (void *xent) { clone_entry *ent = static_cast(xent); - clone_table **pprev = &all_tables; clone_table *tab; - table_lock.write_lock (); + // Hold the serial_lock while we update the ALL_TABLES datastructure. + { + ExcludeTransaction exclude; + clone_table **pprev; - for (pprev = &all_tables; - tab = *pprev, tab->table != ent; - pprev = &tab->next) - continue; - *pprev = tab->next; + for (pprev = &all_tables; + tab = *pprev, tab->table != ent; + pprev = &tab->next) + continue; + *pprev = tab->next; + } - table_lock.write_unlock (); - free (tab); }