From owner-svn-src-all@freebsd.org Sat Jul 18 01:35:18 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6C0859A4CAE; Sat, 18 Jul 2015 01:35:18 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wg0-x235.google.com (mail-wg0-x235.google.com [IPv6:2a00:1450:400c:c00::235]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 0CC6B1CAA; Sat, 18 Jul 2015 01:35:18 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by wgav7 with SMTP id v7so27915230wga.2; Fri, 17 Jul 2015 18:35:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=lnz+TMzIAN5XZvkhuBJDkK8y/mWcVJ5xp007QdrsGl8=; b=S3dbn1N/QMAKpnTDrkOXQ0d0IrlWkd0ZlrQa8fN53/2cGjVAPjLrjJ+qTquWcpCoth fYQe7ev1oDEraVQ8IG5aAoucrM/VgWmaxlrEkMJerYgPQxXcP8Yzre1gipNrkpatFWoc WVAjIA0pt0wS6NI32WfJ0yH02xgz+r3tZQGRq04yGFqhKVgQc8QHl9c85VhbGtGMR4Lz 4nY9XrG8JLpb0yw/vhaWq1BIvEccvHL2JNepS08I4p8sv2KE/xf0Be98vDHuok6/YGeC sXrZg20YjUql5qh4/uf1y5b/L1oxqy2GTIvDLxeXwRdq4J+jYyoTigifNAd6CdUidHwu eGeg== X-Received: by 10.194.238.168 with SMTP id vl8mr35436415wjc.128.1437183316398; Fri, 17 Jul 2015 18:35:16 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by smtp.gmail.com with ESMTPSA id dl10sm20543807wjb.42.2015.07.17.18.35.13 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 17 Jul 2015 18:35:14 -0700 (PDT) Date: Sat, 18 Jul 2015 03:35:11 +0200 From: Mateusz Guzik To: Mark Johnston Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r285664 - in head/sys: kern sys Message-ID: <20150718013511.GA5792@dft-labs.eu> References: <201507180057.t6I0vVhS076519@repo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <201507180057.t6I0vVhS076519@repo.freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 18 Jul 2015 01:35:18 -0000 On Sat, Jul 18, 2015 at 12:57:31AM +0000, Mark Johnston wrote: > Author: markj > Date: Sat Jul 18 00:57:30 2015 > New Revision: 285664 > URL: https://svnweb.freebsd.org/changeset/base/285664 > > Log: > Pass the lock object to lockstat_nsecs() and return immediately if > LO_NOPROFILE is set. Some timecounter handlers acquire a spin mutex, and > we don't want to recurse if lockstat probes are enabled. > Why do we even make the call? Have you tried replacing this with an inline with __predict_false on both conditions? > PR: 201642 > Reviewed by: avg > MFC after: 3 days > > Modified: > head/sys/kern/kern_lockstat.c > head/sys/kern/kern_mutex.c > head/sys/kern/kern_rwlock.c > head/sys/kern/kern_sx.c > head/sys/sys/lockstat.h > > Modified: head/sys/kern/kern_lockstat.c > ============================================================================== > --- head/sys/kern/kern_lockstat.c Sat Jul 18 00:22:00 2015 (r285663) > +++ head/sys/kern/kern_lockstat.c Sat Jul 18 00:57:30 2015 (r285664) > @@ -34,9 +34,10 @@ > > #ifdef KDTRACE_HOOKS > > -#include > #include > +#include > #include > +#include > > /* > * The following must match the type definition of dtrace_probe. It is > @@ -48,13 +49,15 @@ void (*lockstat_probe_func)(uint32_t, ui > int lockstat_enabled = 0; > > uint64_t > -lockstat_nsecs(void) > +lockstat_nsecs(struct lock_object *lo) > { > struct bintime bt; > uint64_t ns; > > if (!lockstat_enabled) > return (0); > + if ((lo->lo_flags & LO_NOPROFILE) != 0) > + return (0); > > binuptime(&bt); > ns = bt.sec * (uint64_t)1000000000; > > Modified: head/sys/kern/kern_mutex.c > ============================================================================== > --- head/sys/kern/kern_mutex.c Sat Jul 18 00:22:00 2015 (r285663) > +++ head/sys/kern/kern_mutex.c Sat Jul 18 00:57:30 2015 (r285664) > @@ -416,7 +416,7 @@ __mtx_lock_sleep(volatile uintptr_t *c, > "_mtx_lock_sleep: %s contested (lock=%p) at %s:%d", > m->lock_object.lo_name, (void *)m->mtx_lock, file, line); > #ifdef KDTRACE_HOOKS > - all_time -= lockstat_nsecs(); > + all_time -= lockstat_nsecs(&m->lock_object); > #endif > > while (!_mtx_obtain_lock(m, tid)) { > @@ -513,16 +513,16 @@ __mtx_lock_sleep(volatile uintptr_t *c, > * Block on the turnstile. > */ > #ifdef KDTRACE_HOOKS > - sleep_time -= lockstat_nsecs(); > + sleep_time -= lockstat_nsecs(&m->lock_object); > #endif > turnstile_wait(ts, mtx_owner(m), TS_EXCLUSIVE_QUEUE); > #ifdef KDTRACE_HOOKS > - sleep_time += lockstat_nsecs(); > + sleep_time += lockstat_nsecs(&m->lock_object); > sleep_cnt++; > #endif > } > #ifdef KDTRACE_HOOKS > - all_time += lockstat_nsecs(); > + all_time += lockstat_nsecs(&m->lock_object); > #endif > #ifdef KTR > if (cont_logged) { > @@ -600,7 +600,7 @@ _mtx_lock_spin_cookie(volatile uintptr_t > #endif > lock_profile_obtain_lock_failed(&m->lock_object, &contested, &waittime); > #ifdef KDTRACE_HOOKS > - spin_time -= lockstat_nsecs(); > + spin_time -= lockstat_nsecs(&m->lock_object); > #endif > while (!_mtx_obtain_lock(m, tid)) { > > @@ -620,7 +620,7 @@ _mtx_lock_spin_cookie(volatile uintptr_t > spinlock_enter(); > } > #ifdef KDTRACE_HOOKS > - spin_time += lockstat_nsecs(); > + spin_time += lockstat_nsecs(&m->lock_object); > #endif > > if (LOCK_LOG_TEST(&m->lock_object, opts)) > @@ -630,7 +630,8 @@ _mtx_lock_spin_cookie(volatile uintptr_t > > LOCKSTAT_PROFILE_OBTAIN_LOCK_SUCCESS(LS_MTX_SPIN_LOCK_ACQUIRE, m, > contested, waittime, (file), (line)); > - LOCKSTAT_RECORD1(LS_MTX_SPIN_LOCK_SPIN, m, spin_time); > + if (spin_time != 0) > + LOCKSTAT_RECORD1(LS_MTX_SPIN_LOCK_SPIN, m, spin_time); > } > #endif /* SMP */ > > @@ -655,7 +656,7 @@ thread_lock_flags_(struct thread *td, in > return; > > #ifdef KDTRACE_HOOKS > - spin_time -= lockstat_nsecs(); > + spin_time -= lockstat_nsecs(&td->td_lock->lock_object); > #endif > for (;;) { > retry: > @@ -703,7 +704,7 @@ retry: > __mtx_unlock_spin(m); /* does spinlock_exit() */ > } > #ifdef KDTRACE_HOOKS > - spin_time += lockstat_nsecs(); > + spin_time += lockstat_nsecs(&m->lock_object); > #endif > if (m->mtx_recurse == 0) > LOCKSTAT_PROFILE_OBTAIN_LOCK_SUCCESS(LS_MTX_SPIN_LOCK_ACQUIRE, > > Modified: head/sys/kern/kern_rwlock.c > ============================================================================== > --- head/sys/kern/kern_rwlock.c Sat Jul 18 00:22:00 2015 (r285663) > +++ head/sys/kern/kern_rwlock.c Sat Jul 18 00:57:30 2015 (r285664) > @@ -378,7 +378,7 @@ __rw_rlock(volatile uintptr_t *c, const > WITNESS_CHECKORDER(&rw->lock_object, LOP_NEWORDER, file, line, NULL); > > #ifdef KDTRACE_HOOKS > - all_time -= lockstat_nsecs(); > + all_time -= lockstat_nsecs(&rw->lock_object); > state = rw->rw_lock; > #endif > for (;;) { > @@ -532,11 +532,11 @@ __rw_rlock(volatile uintptr_t *c, const > CTR2(KTR_LOCK, "%s: %p blocking on turnstile", __func__, > rw); > #ifdef KDTRACE_HOOKS > - sleep_time -= lockstat_nsecs(); > + sleep_time -= lockstat_nsecs(&rw->lock_object); > #endif > turnstile_wait(ts, rw_owner(rw), TS_SHARED_QUEUE); > #ifdef KDTRACE_HOOKS > - sleep_time += lockstat_nsecs(); > + sleep_time += lockstat_nsecs(&rw->lock_object); > sleep_cnt++; > #endif > if (LOCK_LOG_TEST(&rw->lock_object, 0)) > @@ -544,7 +544,7 @@ __rw_rlock(volatile uintptr_t *c, const > __func__, rw); > } > #ifdef KDTRACE_HOOKS > - all_time += lockstat_nsecs(); > + all_time += lockstat_nsecs(&rw->lock_object); > if (sleep_time) > LOCKSTAT_RECORD4(LS_RW_RLOCK_BLOCK, rw, sleep_time, > LOCKSTAT_READER, (state & RW_LOCK_READ) == 0, > @@ -767,7 +767,7 @@ __rw_wlock_hard(volatile uintptr_t *c, u > rw->lock_object.lo_name, (void *)rw->rw_lock, file, line); > > #ifdef KDTRACE_HOOKS > - all_time -= lockstat_nsecs(); > + all_time -= lockstat_nsecs(&rw->lock_object); > state = rw->rw_lock; > #endif > while (!_rw_write_lock(rw, tid)) { > @@ -893,11 +893,11 @@ __rw_wlock_hard(volatile uintptr_t *c, u > CTR2(KTR_LOCK, "%s: %p blocking on turnstile", __func__, > rw); > #ifdef KDTRACE_HOOKS > - sleep_time -= lockstat_nsecs(); > + sleep_time -= lockstat_nsecs(&rw->lock_object); > #endif > turnstile_wait(ts, rw_owner(rw), TS_EXCLUSIVE_QUEUE); > #ifdef KDTRACE_HOOKS > - sleep_time += lockstat_nsecs(); > + sleep_time += lockstat_nsecs(&rw->lock_object); > sleep_cnt++; > #endif > if (LOCK_LOG_TEST(&rw->lock_object, 0)) > @@ -908,7 +908,7 @@ __rw_wlock_hard(volatile uintptr_t *c, u > #endif > } > #ifdef KDTRACE_HOOKS > - all_time += lockstat_nsecs(); > + all_time += lockstat_nsecs(&rw->lock_object); > if (sleep_time) > LOCKSTAT_RECORD4(LS_RW_WLOCK_BLOCK, rw, sleep_time, > LOCKSTAT_WRITER, (state & RW_LOCK_READ) == 0, > > Modified: head/sys/kern/kern_sx.c > ============================================================================== > --- head/sys/kern/kern_sx.c Sat Jul 18 00:22:00 2015 (r285663) > +++ head/sys/kern/kern_sx.c Sat Jul 18 00:57:30 2015 (r285664) > @@ -541,7 +541,7 @@ _sx_xlock_hard(struct sx *sx, uintptr_t > sx->lock_object.lo_name, (void *)sx->sx_lock, file, line); > > #ifdef KDTRACE_HOOKS > - all_time -= lockstat_nsecs(); > + all_time -= lockstat_nsecs(&sx->lock_object); > state = sx->sx_lock; > #endif > while (!atomic_cmpset_acq_ptr(&sx->sx_lock, SX_LOCK_UNLOCKED, tid)) { > @@ -691,7 +691,7 @@ _sx_xlock_hard(struct sx *sx, uintptr_t > __func__, sx); > > #ifdef KDTRACE_HOOKS > - sleep_time -= lockstat_nsecs(); > + sleep_time -= lockstat_nsecs(&sx->lock_object); > #endif > GIANT_SAVE(); > sleepq_add(&sx->lock_object, NULL, sx->lock_object.lo_name, > @@ -702,7 +702,7 @@ _sx_xlock_hard(struct sx *sx, uintptr_t > else > error = sleepq_wait_sig(&sx->lock_object, 0); > #ifdef KDTRACE_HOOKS > - sleep_time += lockstat_nsecs(); > + sleep_time += lockstat_nsecs(&sx->lock_object); > sleep_cnt++; > #endif > if (error) { > @@ -717,7 +717,7 @@ _sx_xlock_hard(struct sx *sx, uintptr_t > __func__, sx); > } > #ifdef KDTRACE_HOOKS > - all_time += lockstat_nsecs(); > + all_time += lockstat_nsecs(&sx->lock_object); > if (sleep_time) > LOCKSTAT_RECORD4(LS_SX_XLOCK_BLOCK, sx, sleep_time, > LOCKSTAT_WRITER, (state & SX_LOCK_SHARED) == 0, > @@ -828,7 +828,7 @@ _sx_slock_hard(struct sx *sx, int opts, > > #ifdef KDTRACE_HOOKS > state = sx->sx_lock; > - all_time -= lockstat_nsecs(); > + all_time -= lockstat_nsecs(&sx->lock_object); > #endif > > /* > @@ -955,7 +955,7 @@ _sx_slock_hard(struct sx *sx, int opts, > __func__, sx); > > #ifdef KDTRACE_HOOKS > - sleep_time -= lockstat_nsecs(); > + sleep_time -= lockstat_nsecs(&sx->lock_object); > #endif > GIANT_SAVE(); > sleepq_add(&sx->lock_object, NULL, sx->lock_object.lo_name, > @@ -966,7 +966,7 @@ _sx_slock_hard(struct sx *sx, int opts, > else > error = sleepq_wait_sig(&sx->lock_object, 0); > #ifdef KDTRACE_HOOKS > - sleep_time += lockstat_nsecs(); > + sleep_time += lockstat_nsecs(&sx->lock_object); > sleep_cnt++; > #endif > if (error) { > @@ -981,7 +981,7 @@ _sx_slock_hard(struct sx *sx, int opts, > __func__, sx); > } > #ifdef KDTRACE_HOOKS > - all_time += lockstat_nsecs(); > + all_time += lockstat_nsecs(&sx->lock_object); > if (sleep_time) > LOCKSTAT_RECORD4(LS_SX_SLOCK_BLOCK, sx, sleep_time, > LOCKSTAT_READER, (state & SX_LOCK_SHARED) == 0, > > Modified: head/sys/sys/lockstat.h > ============================================================================== > --- head/sys/sys/lockstat.h Sat Jul 18 00:22:00 2015 (r285663) > +++ head/sys/sys/lockstat.h Sat Jul 18 00:57:30 2015 (r285664) > @@ -149,11 +149,12 @@ > * The following must match the type definition of dtrace_probe. It is > * defined this way to avoid having to rely on CDDL code. > */ > +struct lock_object; > extern uint32_t lockstat_probemap[LS_NPROBES]; > typedef void (*lockstat_probe_func_t)(uint32_t, uintptr_t arg0, uintptr_t arg1, > uintptr_t arg2, uintptr_t arg3, uintptr_t arg4); > extern lockstat_probe_func_t lockstat_probe_func; > -extern uint64_t lockstat_nsecs(void); > +extern uint64_t lockstat_nsecs(struct lock_object *); > extern int lockstat_enabled; > > #ifdef KDTRACE_HOOKS > -- Mateusz Guzik