From owner-freebsd-current@freebsd.org Thu Nov 5 14:26:42 2015 Return-Path: Delivered-To: freebsd-current@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 DC0D3A267F2 for ; Thu, 5 Nov 2015 14:26:41 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 8313A14DE for ; Thu, 5 Nov 2015 14:26:41 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id tA5EQSFH009956 (version=TLSv1 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Thu, 5 Nov 2015 16:26:29 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua tA5EQSFH009956 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id tA5EQSpd009955; Thu, 5 Nov 2015 16:26:28 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 5 Nov 2015 16:26:28 +0200 From: Konstantin Belousov To: Mateusz Guzik , freebsd-current@freebsd.org Subject: Re: [PATCH] microoptimize by trying to avoid locking a locked mutex Message-ID: <20151105142628.GJ2257@kib.kiev.ua> References: <20151104233218.GA27709@dft-labs.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151104233218.GA27709@dft-labs.eu> User-Agent: Mutt/1.5.24 (2015-08-30) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Nov 2015 14:26:42 -0000 On Thu, Nov 05, 2015 at 12:32:18AM +0100, Mateusz Guzik wrote: > mtx_lock will unconditionally try to grab the lock and if that fails, > will call __mtx_lock_sleep which will immediately try to do the same > atomic op again. > > So, the obvious microoptimization is to check the state in > __mtx_lock_sleep and avoid the operation if the lock is not free. > > This gives me ~40% speedup in a microbenchmark of 40 find processes > traversing tmpfs and contending on mount mtx (only used as an easy > benchmark, I have WIP patches to get rid of it). > > Second part of the patch is optional and just checks the state of the > lock prior to doing any atomic operations, but it gives a very modest > speed up when applied on top of the __mtx_lock_sleep change. As such, > I'm not going to defend this part. Shouldn't the same consideration applied to all spinning loops, i.e. also to the spin/thread mutexes, and to the spinning parts of sx and lockmgr ? > > x vanilla > + patched > +--------------------------------------------------------------------------------------------------------------------------------------+ > | + | > |+ ++ ++ ++ + x x x xx xx x x| > | |_____AM____| |____________A_M__________| | > +--------------------------------------------------------------------------------------------------------------------------------------+ > N Min Max Median Avg Stddev > x 9 13.845 16.148 15.271 15.133889 0.75997096 > + 9 8.363 9.56 9.126 9.0643333 0.34198501 > Difference at 95.0% confidence > -6.06956 +/- 0.588917 > -40.1057% +/- 3.89138% > (Student's t, pooled s = 0.589283) > > x patched > + patched2 > +--------------------------------------------------------------------------------------------------------------------------------------+ > | + | > |+ * + + + + x x + + x x x x x x| > | |____________________________A_____M______|_______________|______A___M__________________| | > +--------------------------------------------------------------------------------------------------------------------------------------+ > N Min Max Median Avg Stddev > x 9 8.363 9.56 9.126 9.0643333 0.34198501 > + 9 7.563 9.038 8.611 8.5256667 0.43365885 > Difference at 95.0% confidence > -0.538667 +/- 0.390278 > -5.94271% +/- 4.30565% > (Student's t, pooled s = 0.390521) > > diff --git a/sys/kern/kern_mutex.c b/sys/kern/kern_mutex.c > index bec8f6b..092aaae 100644 > --- a/sys/kern/kern_mutex.c > +++ b/sys/kern/kern_mutex.c > @@ -419,7 +419,10 @@ __mtx_lock_sleep(volatile uintptr_t *c, uintptr_t tid, int opts, > all_time -= lockstat_nsecs(&m->lock_object); > #endif > > - while (!_mtx_obtain_lock(m, tid)) { > + for (;;) { > + v = m->mtx_lock; > + if (v == MTX_UNOWNED && _mtx_obtain_lock(m, tid)) > + break; > #ifdef KDTRACE_HOOKS > spin_cnt++; > #endif > @@ -428,7 +431,6 @@ __mtx_lock_sleep(volatile uintptr_t *c, uintptr_t tid, int opts, > * If the owner is running on another CPU, spin until the > * owner stops running or the state of the lock changes. > */ > - v = m->mtx_lock; > if (v != MTX_UNOWNED) { You could restructure the code to only do one comparision with MTX_UNOWNED, effectively using 'else' instead of the if() on the previous line. > owner = (struct thread *)(v & ~MTX_FLAGMASK); > if (TD_IS_RUNNING(owner)) { > diff --git a/sys/sys/mutex.h b/sys/sys/mutex.h > index a9ec072..4208d5f 100644 > --- a/sys/sys/mutex.h > +++ b/sys/sys/mutex.h > @@ -184,12 +184,11 @@ void thread_lock_flags_(struct thread *, int, const char *, int); > /* Lock a normal mutex. */ > #define __mtx_lock(mp, tid, opts, file, line) do { \ > uintptr_t _tid = (uintptr_t)(tid); \ > - \ > - if (!_mtx_obtain_lock((mp), _tid)) \ > - _mtx_lock_sleep((mp), _tid, (opts), (file), (line)); \ > - else \ > + if (((mp)->mtx_lock == MTX_UNOWNED && _mtx_obtain_lock((mp), _tid))) \ > LOCKSTAT_PROFILE_OBTAIN_LOCK_SUCCESS(adaptive__acquire, \ > mp, 0, 0, file, line); \ > + else \ > + _mtx_lock_sleep((mp), _tid, (opts), (file), (line)); \ > } while (0) > > /* > -- > Mateusz Guzik > _______________________________________________ > freebsd-current@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"