From owner-svn-src-head@freebsd.org Thu Feb 23 17:15:28 2017 Return-Path: Delivered-To: svn-src-head@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 5FE03CEBC45; Thu, 23 Feb 2017 17:15:28 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) (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 07EF9FDF; Thu, 23 Feb 2017 17:15:28 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wm0-x244.google.com with SMTP id r18so947770wmd.3; Thu, 23 Feb 2017 09:15:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=4zbtmG/Ipw49c+6wJzrhngpUIY9vgE5+w4masuPbzF8=; b=Sf7AROvpFQKYPSaY5SggwFQCeG3HfwyUbPXmQ8GZB3PmUg3N8cIFixXZn5uhTiWy4k ovaoEnZRLdPLNQWnarOEg+q/7m8Gw4r4GQxTKbzwbcmBWKayzAdUgao5M1XXoPpr3sEv dTILCtbUe6DZeIiUfgFzoK317ak0DEj0Z//50586zhip6xSiX7GlWzRo7klR9AsCZfnb ZGAdregrFpgkDoEWnlUHgp+FGksOORdI1INcckan4v7Tl7vw0VRkPEqujZU/Mla3C+hV J5TJk3bgC1eHI7tYWmywLOH6xtADqBelTRzRwUZXQC/KBYgdc0ankW8u4o+c8b3oQthk sfOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=4zbtmG/Ipw49c+6wJzrhngpUIY9vgE5+w4masuPbzF8=; b=iUjuY7mrXck/uzY1Jv8Pur6DHKEX5CkC6WsMSSZgVSVC/OWxkRqyoAA5rFU+yMha+N OQ5xt5xUxM4hw7t5coeOF1pCMT0PAd/Lfsp1Kaxsb83JEMIWBekbGX6ef9J2NIB/0sZC 68OckbIYybj+KgJSbw04jei3yUuQw7cAs9mwbEPKUJBW35lWmJAawsRaVFFbBlXSs5AR myQ5DB7GVgjoKwMMvVqEh+ljfbqtPNB8nKWqueC1KYghvrAATOszEXK7wE+Ud7coMEcJ HieJQPq+jeEr4IZp4IfDzJxD/JlZUfSmqGqJKLRw0UJf2YL6gNbFIbdviCFFTyCfheKH fiFw== X-Gm-Message-State: AMke39m8Tcixpu+a6u0uvEfoTY3gwCGNax0s8B/64eWhB0fuX9GCrKXAPTW0NVgekb5Umg== X-Received: by 10.28.19.78 with SMTP id 75mr6122402wmt.108.1487870126199; Thu, 23 Feb 2017 09:15:26 -0800 (PST) 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 r19sm6807669wrr.44.2017.02.23.09.15.25 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 23 Feb 2017 09:15:25 -0800 (PST) Date: Thu, 23 Feb 2017 18:15:23 +0100 From: Mateusz Guzik To: Gleb Smirnoff Cc: Mateusz Guzik , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r313996 - in head/sys: kern sys Message-ID: <20170223171523.GA5744@dft-labs.eu> References: <201702201908.v1KJ8aEE036715@repo.freebsd.org> <20170222230924.GI8899@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170222230924.GI8899@FreeBSD.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Feb 2017 17:15:28 -0000 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-all@freebsd.org mailing list > M> https://lists.freebsd.org/mailman/listinfo/svn-src-all > M> To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" > > -- > Totus tuus, Glebius. > _______________________________________________ > svn-src-all@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/svn-src-all > To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" -- Mateusz Guzik