On Wed, Feb 22, 2017 at 03:09:24PM -0800, Gleb Smirnoff wrote: > Mateusz, > > why do you __predict_false() the recursion scenario? I'm afraid > that performance loss for mispredictions could outweight the > gain due to predictions. AFAIK, mutex recursion is still a pretty > common event in the kernel. >
Do you mean spin mutexes or blocking ones sa well? For blocking mutexes, recursion almost never happens and I'm confident in the prediction to not do it. It may be there is a lock somewere which recurses a lot and its singlethreaded operation is harmed by the prediction. For such locks a separate wrappers shouldl be introduced which expect recursion to happen. For spin mutexes the above is almost correct. Most calls do the slow path to spin. There is an important lock which sometimes recurses (sched lock) but it is unclear if it got hurt by the change. Note that lock clean up is a work in progress. For spin mutexes I'm pondering revamping them a little - the inline path is extremelly long and already constains a function call to spinlock_enter/exit. So the plan would be to create a primitive with an inlined spinlock code to keep one call and reduce inline code. Finally, while mfcability of the work prevents riping out recursion support from the code, an additional set of primitives which expect recursion to happen can be introduced and used in places which do recurse. TL;DR I think the changes are a net win to spinlocks even if they recurse and will be a bigger win with coming cleanups. On the other hand if you profiled a regression with the above, I'm happy to unpredict the change. > On Mon, Feb 20, 2017 at 07:08:36PM +0000, Mateusz Guzik wrote: > M> Author: mjg > M> Date: Mon Feb 20 19:08:36 2017 > M> New Revision: 313996 > M> URL: https://svnweb.freebsd.org/changeset/base/313996 > M> > M> Log: > M> mtx: fix spin mutexes interaction with failed fcmpset > M> > M> While doing so move recursion support down to the fallback routine. > M> > M> Modified: > M> head/sys/kern/kern_mutex.c > M> head/sys/sys/mutex.h > M> > M> Modified: head/sys/kern/kern_mutex.c > M> > ============================================================================== > M> --- head/sys/kern/kern_mutex.c Mon Feb 20 17:33:25 2017 > (r313995) > M> +++ head/sys/kern/kern_mutex.c Mon Feb 20 19:08:36 2017 > (r313996) > M> @@ -696,6 +696,14 @@ _mtx_lock_spin_cookie(volatile uintptr_t > M> lock_delay_arg_init(&lda, &mtx_spin_delay); > M> m = mtxlock2mtx(c); > M> > M> + if (__predict_false(v == MTX_UNOWNED)) > M> + v = MTX_READ_VALUE(m); > M> + > M> + if (__predict_false(v == tid)) { > M> + m->mtx_recurse++; > M> + return; > M> + } > M> + > M> if (LOCK_LOG_TEST(&m->lock_object, opts)) > M> CTR1(KTR_LOCK, "_mtx_lock_spin: %p spinning", m); > M> KTR_STATE1(KTR_SCHED, "thread", sched_tdname((struct thread *)tid), > M> > M> Modified: head/sys/sys/mutex.h > M> > ============================================================================== > M> --- head/sys/sys/mutex.h Mon Feb 20 17:33:25 2017 (r313995) > M> +++ head/sys/sys/mutex.h Mon Feb 20 19:08:36 2017 (r313996) > M> @@ -223,12 +223,9 @@ void thread_lock_flags_(struct thread *, > M> uintptr_t _v = MTX_UNOWNED; \ > M> \ > M> spinlock_enter(); \ > M> - if (!_mtx_obtain_lock_fetch((mp), &_v, _tid)) { \ > M> - if (_v == _tid) \ > M> - (mp)->mtx_recurse++; \ > M> - else \ > M> - _mtx_lock_spin((mp), _v, _tid, (opts), (file), (line));\ > M> - } else \ > M> + if (!_mtx_obtain_lock_fetch((mp), &_v, _tid)) \ > M> + _mtx_lock_spin((mp), _v, _tid, (opts), (file), (line)); \ > M> + else \ > M> LOCKSTAT_PROFILE_OBTAIN_LOCK_SUCCESS(spin__acquire, \ > M> mp, 0, 0, file, line); \ > M> } while (0) > M> _______________________________________________ > M> svn-src-...@freebsd.org mailing list > M> https://lists.freebsd.org/mailman/listinfo/svn-src-all > M> To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org" > > -- > Totus tuus, Glebius. > _______________________________________________ > svn-src-...@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/svn-src-all > To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org" -- Mateusz Guzik <mjguzik gmail.com> _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"