From owner-p4-projects@FreeBSD.ORG Mon Jun 26 22:29:55 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 6088216A4CE; Mon, 26 Jun 2006 22:29:55 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id E272D16A51F; Mon, 26 Jun 2006 22:29:54 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (66-23-211-162.clients.speedfactory.net [66.23.211.162]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6B3E7446F8; Mon, 26 Jun 2006 22:05:20 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.13.4/8.13.4) with ESMTP id k5QM5FvK086648; Mon, 26 Jun 2006 18:05:19 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: Kip Macy Date: Mon, 26 Jun 2006 17:59:41 -0400 User-Agent: KMail/1.9.1 References: <200606262054.k5QKsDq7022302@repoman.freebsd.org> In-Reply-To: <200606262054.k5QKsDq7022302@repoman.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200606261759.41541.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Mon, 26 Jun 2006 18:05:19 -0400 (EDT) X-Virus-Scanned: ClamAV 0.87.1/1564/Mon Jun 26 10:55:16 2006 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,BAYES_00 autolearn=ham version=3.1.0 X-Spam-Checker-Version: SpamAssassin 3.1.0 (2005-09-13) on server.baldwin.cx Cc: Perforce Change Reviews Subject: Re: PERFORCE change 100089 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Jun 2006 22:29:55 -0000 On Monday 26 June 2006 16:54, Kip Macy wrote: > http://perforce.freebsd.org/chv.cgi?CH=100089 > > Change 100089 by kmacy@kmacy_storage:sun4v_work_sleepq on 2006/06/26 20:53:51 > > add profiling for rwlocks > not convinced of correctness as there don't appear to be any contended rwlocks on my workloads Few things use them currently. I have a patch to make the name cache use them if you want it. > Affected files ... > > .. //depot/projects/kmacy_sun4v/src/sys/kern/kern_rwlock.c#4 edit > > Differences ... > > ==== //depot/projects/kmacy_sun4v/src/sys/kern/kern_rwlock.c#4 (text+ko) ==== > > @@ -44,7 +44,7 @@ > #include > #include > #include > - > +#include > #include > > #ifdef DDB > @@ -80,12 +80,19 @@ > #define _rw_assert(rw, what, file, line) > #endif > > +#ifdef SMP > +#define smp_turnstile_valid(obj) (turnstile_lookup((obj)) != NULL) > +#else > +#define smp_turnstile(obj) > +#endif > + > void > rw_init(struct rwlock *rw, const char *name) > { > > rw->rw_lock = RW_UNLOCKED; > > + lock_profile_init(&rw->rw_object, name); > lock_init(&rw->rw_object, &lock_class_rw, name, NULL, LO_WITNESS | > LO_RECURSABLE | LO_UPGRADABLE); > } > @@ -95,6 +102,7 @@ > { > > KASSERT(rw->rw_lock == RW_UNLOCKED, ("rw lock not unlocked")); > + lock_profile_destroy(&rw->rw_object); > lock_destroy(&rw->rw_object); > } > > @@ -109,14 +117,17 @@ > void > _rw_wlock(struct rwlock *rw, const char *file, int line) > { > - > + uint64_t waitstart; > + > MPASS(curthread != NULL); > KASSERT(rw_wowner(rw) != curthread, > ("%s (%s): wlock already held @ %s:%d", __func__, > rw->rw_object.lo_name, file, line)); > WITNESS_CHECKORDER(&rw->rw_object, LOP_NEWORDER | LOP_EXCLUSIVE, file, > line); > + lock_profile_waitstart(&waitstart); > __rw_wlock(rw, curthread, file, line); > + lock_profile_obtain_lock_success(&rw->rw_object, waitstart, file, line); > LOCK_LOG_LOCK("WLOCK", &rw->rw_object, 0, 0, file, line); > WITNESS_LOCK(&rw->rw_object, LOP_EXCLUSIVE, file, line); > } > @@ -129,6 +140,7 @@ > _rw_assert(rw, RA_WLOCKED, file, line); > WITNESS_UNLOCK(&rw->rw_object, LOP_EXCLUSIVE, file, line); > LOCK_LOG_LOCK("WUNLOCK", &rw->rw_object, 0, 0, file, line); > + lock_profile_release_lock(&rw->rw_object); > __rw_wunlock(rw, curthread, file, line); > } > > @@ -139,6 +151,8 @@ > volatile struct thread *owner; > #endif > uintptr_t x; > + uint64_t waitstart; > + int contested; > > KASSERT(rw_wowner(rw) != curthread, > ("%s (%s): wlock already held @ %s:%d", __func__, > @@ -156,6 +170,7 @@ > * be blocked on the writer, and the writer would be blocked > * waiting for the reader to release its original read lock. > */ > + lock_profile_waitstart(&waitstart); > for (;;) { > /* > * Handle the easy case. If no other thread has a write > @@ -169,7 +184,6 @@ > */ > x = rw->rw_lock; > if (x & RW_LOCK_READ) { > - > /* > * The RW_LOCK_READ_WAITERS flag should only be set > * if another thread currently holds a write lock, > @@ -178,6 +192,9 @@ > MPASS((x & RW_LOCK_READ_WAITERS) == 0); > if (atomic_cmpset_acq_ptr(&rw->rw_lock, x, > x + RW_ONE_READER)) { > + if ((x & ((1< + lock_profile_obtain_lock_success(&rw->rw_object, waitstart, file, line); > + > if (LOCK_LOG_TEST(&rw->rw_object, 0)) > CTR4(KTR_LOCK, > "%s: %p succeed %p -> %p", __func__, > @@ -186,6 +203,7 @@ > break; > } > cpu_spinwait(); > + lock_profile_obtain_lock_failed(&rw->rw_object, &contested); > continue; > } > > @@ -234,6 +252,7 @@ > */ > owner = (struct thread *)RW_OWNER(x); > if (TD_IS_RUNNING(owner)) { > + lock_profile_obtain_lock_failed(&rw->rw_object, &contested); > turnstile_release(&rw->rw_object); > if (LOCK_LOG_TEST(&rw->rw_object, 0)) > CTR3(KTR_LOCK, "%s: spinning on %p held by %p", > @@ -279,7 +298,6 @@ > LOCK_LOG_LOCK("RUNLOCK", &rw->rw_object, 0, 0, file, line); > > /* TODO: drop "owner of record" here. */ > - > for (;;) { > /* > * See if there is more than one read lock held. If so, > @@ -297,7 +315,8 @@ > break; > } > continue; > - } > + } else > + lock_profile_release_lock(&rw->rw_object); > > /* > * We should never have read waiters while at least one > @@ -328,7 +347,8 @@ > break; > } > continue; > - } > + } > + > > /* > * There should just be one reader with one or more > @@ -394,6 +414,7 @@ > volatile struct thread *owner; > #endif > uintptr_t v; > + int contested; > > if (LOCK_LOG_TEST(&rw->rw_object, 0)) > CTR5(KTR_LOCK, "%s: %s contested (lock=%p) at %s:%d", __func__, > @@ -434,6 +455,7 @@ > } > turnstile_release(&rw->rw_object); > cpu_spinwait(); > + lock_profile_obtain_lock_failed(&rw->rw_object, &contested); > continue; > } > > @@ -447,6 +469,7 @@ > v | RW_LOCK_WRITE_WAITERS)) { > turnstile_release(&rw->rw_object); > cpu_spinwait(); > + lock_profile_obtain_lock_failed(&rw->rw_object, &contested); > continue; > } > if (LOCK_LOG_TEST(&rw->rw_object, 0)) > @@ -462,6 +485,7 @@ > */ > owner = (struct thread *)RW_OWNER(v); > if (!(v & RW_LOCK_READ) && TD_IS_RUNNING(owner)) { > + lock_profile_obtain_lock_failed(&rw->rw_object, &contested); > turnstile_release(&rw->rw_object); > if (LOCK_LOG_TEST(&rw->rw_object, 0)) > CTR3(KTR_LOCK, "%s: spinning on %p held by %p", > @@ -629,11 +653,7 @@ > v = rw->rw_lock & RW_LOCK_WRITE_WAITERS; > success = atomic_cmpset_acq_ptr(&rw->rw_lock, RW_READERS_LOCK(1) | v, > tid | v); > -#ifdef SMP > - if (success && v && turnstile_lookup(&rw->rw_object) != NULL) > -#else > - if (success && v) > -#endif > + if (success && v && smp_turnstile_valid(&rw->rw_object)) > turnstile_claim(&rw->rw_object); > else > turnstile_release(&rw->rw_object); > -- John Baldwin